Conversation
test/smoke_test/smoke_test.py
Outdated
| parser.add_argument("--streamreader", action="store_true") | ||
| parser.add_argument("--no-streamreader", dest="streamreader", action="store_false") |
There was a problem hiding this comment.
I think this can be achieved w/ just one argument
There was a problem hiding this comment.
yes, can we just have one argument --no-streamreader so that it defaults to test streamreader.
Also, I think it's more correct to say --no-ffmpeg instead of streamreader as there is streamwriter as well.
|
This is very sensitive change, and I am not in favor of this, because the reason this test was added is to makes sure that FFmpeg installation (a version different from the one used for build) is properly integrated, which constitutes the most basic smoke test. If this option is abused, the binary being tested will be in a bad shape. |
This is to be used when calling from https://pytorch.org/get-started/locally/ and release validation workflows in https://github.com/pytorch/builder. Which do not install ffmpeg currently. |
test/smoke_test/smoke_test.py
Outdated
| def main() -> None: | ||
| parser = argparse.ArgumentParser() | ||
| parser.add_argument("--streamreader", action="store_true") | ||
| parser.add_argument("--no-streamreader", dest="streamreader", action="store_false") |
There was a problem hiding this comment.
Also, can we have some warning and note on this option?
The use of this argument should restricted to minimal.
|
@atalman has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
Hey @atalman. |
Summary: Toggle on/off ffmpeg test if needed By default it ON, hence should not affect any current tests. To toggle ON no change required. To toggle OFF use: ``` smoke_test.py --no-ffmpeg ``` To be used when calling from builder currently. Since we do not install ffmpeg currently. Pull Request resolved: pytorch#2901 Reviewed By: carolineechen, mthrok Differential Revision: D41874976 Pulled By: atalman fbshipit-source-id: c57b19f37c63a1f476f93a5211550e980e67d9c7
Summary: Toggle on/off ffmpeg test if needed By default it ON, hence should not affect any current tests. To toggle ON no change required. To toggle OFF use: ``` smoke_test.py --no-ffmpeg ``` To be used when calling from builder currently. Since we do not install ffmpeg currently. Pull Request resolved: pytorch#2901 Reviewed By: carolineechen, mthrok Differential Revision: D41874976 Pulled By: atalman fbshipit-source-id: c57b19f37c63a1f476f93a5211550e980e67d9c7
Toggle on/off ffmpeg test if needed
By default it ON, hence should not affect any current tests.
To toggle ON no change required.
To toggle OFF use:
To be used when calling from builder currently. Since we do not install ffmpeg currently.