unittest -> pytest #425
unittest -> pytest #425
Conversation
|
Glad to see the effort, does pytest support the Test class as well? |
|
Thanks. Pytest uses functions instead of test classes. Fixtures in pytest allow for setup/teardown of test resources, and can be composed to handle more complex situations. I am most interested in using parametrized tests so that we can loop over a large number of cases to get better test coverage. |
|
|
||
| def test_rnn_state_passing(self): | ||
| K.clear_session() |
There was a problem hiding this comment.
This is being run for all tests now in the runner fixture. It helps avoid difficult to reproduce errors that occur due to resource leaking in the tests.
|
This PR is ready for review. All of the main tests are converted to using pytest. There are a number of additional goals above, but given the diff already, I think they are better left for other PRs. This includes migrating the nightly tests, which still use unittest. I would highly recommend using the "Hide whitespace changes" to review this PR, since the majority of the changes are indentation. |
| @@ -0,0 +1,32 @@ | |||
| ############################################################################### | |||
There was a problem hiding this comment.
what's the meaning of 'conftest'?
There was a problem hiding this comment.
conftest.py is the way that pytest shares fixtures across the entire test suite.
https://docs.pytest.org/en/latest/fixture.html#conftest-py-sharing-fixture-functions
tests/test_cgan.py
Outdated
| if __name__ == "__main__": | ||
| unittest.main() | ||
| @pytest.mark.skipif(keras2onnx.proto.tfcompat.is_tf2 and is_tf_keras, reason="Tensorflow 1.x only tests.") | ||
| @pytest.mark.skipif(is_tf_keras and StrictVersion(tf.__version__) < StrictVersion("1.14.0"), |
There was a problem hiding this comment.
StrictVersion(tensorflow.version.split('-')[0]) >= StrictVersi...
otherwise, the whole test cases will failed on tensorflow nightly build.
There was a problem hiding this comment.
I'm confused by what you mean here. The current conditional is the same as that in line 133, so the previous behavior is reproduced. Was the original incorrect?
There was a problem hiding this comment.
The previous code is also ‘imprecise’, but the issue was hidden because this condition wasn't tested since the first is_tf2 test is already false. And it looks the pytest.mark will test both. If you install a tensorflow nightly build. you will see the failure.
There was a problem hiding this comment.
Ok, that makes sense. I've made the change.
| self.assertTrue(opset_comparison) | ||
|
|
||
|
|
||
| if __name__ == '__main__': |
There was a problem hiding this comment.
we sometimes run this single test file with python cmd line instead of pytest, which helps on debugging. Is there alternative way in the pytest as well?
There was a problem hiding this comment.
You can select a specific file by running pytest against that file.
pytest tests/test_misc.py -vv
You can also filter tests for specific strings of interest (e.g. LSTM):
pytest tests -k LSTM -vv
https://docs.pytest.org/en/latest/usage.html#specifying-tests-selecting-tests
I use |
I saw there were few PEP8 suggestions in my editor tool. But it's not mandatory now. Feel free to ignore it if it is not straightforward in your development. |
|
Thanks a lot for your contribution! |
This PR migrates the testing from
unittesttopytest. This allows for the use of parametrized tests and fixtures, leading to more robust test coverage and easier test-code maintenance. This was motivated by my recent PRs on Bidirectional and Masking features, where I saw that many of the tests rely on nested for-loops, which are hard to filter (using-k) and maintain as they expand.Goals:
actualpredictions and better integrate into pytest/pdbThis PR is still a work in progress, and will be updated. I'm interested in any suggestions or comments.