Replace tests comparing functionality to sox with hard coded results. #4017
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/audio/4017
Note: Links to docs will display an error until the docs builds have been completed. ❗ 1 Active SEVsThere are 1 currently active SEVs. If your PR is affected, please view them below: ❌ 18 New FailuresAs of commit 7094873 with merge base 6519052 ( NEW FAILURES - The following jobs have failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
| if os.path.exists(path): | ||
| shutil.copyfile(path, output_file) | ||
| return | ||
|
|
There was a problem hiding this comment.
IIUC the end state we want is for if os.path.exists(path): to be True, right? If it's false it means we forgot to check-in a file? If that's the case, let's not have the if part and always do
shutil.copyfile(path, output_file)
return
unconditionally.
I agree with your previous point that the file generation should still exist for reference, but for safety it might be best to comment it out, so that in never gets executed.
In the current state, as I understand it, if sox is available and the file doesn't exist for whatever reason, then we are effectively checking against sox's output instead of checking against a file which should exist - so we're not testing what we think we're testing.
There was a problem hiding this comment.
Your understanding is correct! I was planning to address your point by just removing the sox dependency from the dependencies installed on CI, so that if the file was missing we'd see an error about importing the library. But I can certainly comment the generation part out if you think that's clearer.
To eliminate the sox dependency, this PR pre-computes the result of calls in the test suite that compare functionality to sox.