-
Notifications
You must be signed in to change notification settings - Fork 664
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[CI] Run unit test with non-editable installation #845
Conversation
44abacd
to
25cca95
Compare
05160c5
to
91dd92b
Compare
91dd92b
to
30aa240
Compare
@@ -6,5 +6,5 @@ eval "$(./conda/Scripts/conda.exe 'shell.bash' 'hook')" | |||
conda activate ./env | |||
|
|||
python -m torch.utils.collect_env | |||
pytest --cov=torchaudio --junitxml=test-results/junit.xml -v --durations 20 test | |||
flake8 torchaudio test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
flake8 is now tested separately, hence why this is removed here, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's correct.
fi | ||
cd test | ||
pytest "${args[@]}" "${common_args[@]}" torchaudio_unittest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could the following command work without moving files from test/
to test/torchaudio_unittest/
?
pytest "${args[@]}" "${common_args[@]}" test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An alternative would be remove source torchaudio
directory.
- do
rm torchaudio
before running test - ugly - Move the source code to
src
. See this. - though this is clean and standard way, it involves the reorg of whole source code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks for pointing these out! I'm ok with the approach suggested here -- though neither pytorch nor the other libraries do it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for figuring out the source of the issue :) What would be an alternative to moving files from test/
to test/torchaudio_unittest/
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks |
Update 'Was this Helpful?' tracking
We have been running unit test with editable installation. (i.e.
python setup.py develop
), with which we missed issues like #842.This PR makes installation in CI non-editable, and change test directory structure so that the source code will not shadow the installed version of
torchaudio
. With simplepytest test
,pytest
modifies and prepend checked out repository, which shadows the installed version.To remedy this, the whole test suites has been moved from
./test
to./test/torchaudio_unittest
. This adds nice module structure to our test code and we can do absolute import in each test module, which makes it possible again to run test withpython -m unittest torchaudio_unittest/XXX.py
This change does not affect the regular development process (
python setup.py develop
&&pytest test
)