- Notifications
You must be signed in to change notification settings - Fork441
Switch to pytest and add optional Python 3.8 test#380
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
coveralls commentedMar 18, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
coveralls commentedMar 18, 2020
1 similar comment
coveralls commentedMar 18, 2020
For some reason the Without the |
a03ea58 to53202aaCompareRebased changes on top of master to get some of the fixes from#366 the eased precision on some of the tests. |
| @@ -118,7 +130,7 @@ install: | |||
| # command to run tests | |||
| script: | |||
| - 'if [ $SLYCOT != "" ]; then python -c "import slycot"; fi' | |||
| - coverage runsetup.py test | |||
| - coverage run-m pytest --disable-warnings control/tests | |||
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.
is--disable-warnings needed? I would rather have them printed and give every contributor the opportunity to fix problems early before e.g.DeprecationWarnings turn into errors after a Python or some library update.
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.
There are 18,209 warning messages and unfortunately pytest prints error messages first => I found it hard to locate the errors. Most of the warnings seem to be around the use ofnumpy.matrix, which I figure we will get rid of starting in v0.9.0.
control/tests/freqresp_test.py Outdated
| def suite(): | ||
| return unittest.TestLoader().loadTestsFromTestCase(TestTimeresp) | ||
| return unittest.TestLoader().loadTestsFromTestCase(TestFreqresp) | ||
| if __name__ == '__main__': | ||
| unittest.main() |
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.
This is code from the unittest framework. Pytest does not use it. IMHO, let's removesuite() and the__main__ sections from all test files.
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.
I have removed thesuite() functions, which were often not properly set up and were only used intests/run_all.py, which I don't think we were using anyway.
I left in the__main__ sections, so that you could (if you wanted) call a test script directly (rather than having to usepytest).
This PR switches the unit testing to use
pytestfor Travis CI instead of the deprecatedsetup.py test. It also adds an optional test for Python 3.8.