-
Notifications
You must be signed in to change notification settings - Fork 117
Make unittests compatible with pytest #231
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
Conversation
vkarak
left a comment
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.
It seems that not all the tests are run on for both the generic and the CSCS settings (it could be simply the counting of the tests, but we need to investigate):
- CSCS settings on
master:
Ran 402 tests in 85.897s
OK (SKIP=8)
- CSCS settings with
pytest:
Ran 387 tests in 101.403s
OK (SKIP=8)
- Generic settings on master:
Ran 387 tests in 77.355s
OK (SKIP=21)
- Generic settings with
pytest
Ran 387 tests in 117.708s
OK (SKIP=27)
| - pip install -r requirements.txt | ||
|
|
||
| script: | ||
| ./test_reframe.py |
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.
We will need to be able to run the the tests with ./test_reframe.py passing any options to pytest. We can achieve this by calling pytest.main() instead of nose.main inside the test_reframe.py.
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.
Yes, I know but first we must add also pytest to corresponding Python bare module.
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.
In any case we should add it to the requirements.txt, so that anybody can run the unit tests.
|
@vkarak I have tried with all the possible settings and I always get 387 tests with either pytest or nose. |
|
@teojgo I've checked again the current status of this branch and the master and both run the same number of unit tests. So, it's fine from this point of view. |
requirements.txt
Outdated
| @@ -1 +1,3 @@ | |||
| nose==1.3.7 | |||
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.
We should remove nose now.
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.
Ok, I have also a module for python bare ready.
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.
As soon as this is fixed, this PR is fine for me. We could though postpone its merging until the Python-bare module is fixed. The only thing is that we should perhaps add some additional pytest plugins in Python-bare, although we don't use them now, so as not to have to update this module every time.
|
The corresponding module which adds support for pytest is here: eth-cscs/production#568 |
638684b to
c2dfccb
Compare
|
Moved to the next sprint; it needs a fix from the big PR for new style checks and the unit tests refactoring. |
* Change exception name `TestLoadError` to `LoadTestError` so that pytest doesn't treat it as a test. * Stop inheriting from `unittest.TestCase` in abstract classes used in test files. * Update documentation and requirements. * Use pytest inside the `test_reframe.py` script.
c2dfccb to
39b8da4
Compare
reframe/core/exceptions.py
Outdated
|
|
||
|
|
||
| class TestLoadError(ReframeError): | ||
| class LoadTestError(ReframeError): |
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 suggest naming this RegressionTestLoadError.
reframe/frontend/loader.py
Outdated
| 'syntax is not allowed' % module.__file__) | ||
| raise RegressionTestLoadError('%s: mixing old and new regression ' | ||
| 'test syntax is not allowed' | ||
| % module.__file__) |
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.
We split lines after the operators, but never mind.
unittest.TestCaseinheritance to the actual testclasses.
Closes #111.
Related to #134