Use pytest for test running #297
Conversation
|
I should note also: the advanced indexing PR was merged while I was still working on this. There was a small conflict for a few tests but I believe everything is carried over as it should be. But more eyes especially on that would be advised. |
|
@magnatelee @manopapad For now I am going to fix conflicts as they arise with rebase / force-push to keep the commit history clean. Let me know when you are starting a review and I will switch to merges if needed |
bryevdv
force-pushed
the
bryanv/pytest_integration
branch
from
a30a50b
to
bbda5a9
Compare
for more information, see https://pre-commit.ci
|
moved/renamed/updated the recently merged fft tests in 4b81936 @mferreravila please take a look at the changes cc @magnatelee |
|
@bryevdv I left some comments. Looks good to me otherwise. One thing I noticed though is that the pytest driver seems to be hiding standard error messages that only show up with the |
|
@mferreravila 👍 For now I left all the print statements, but I parameterized them and moved to the "check" helper function, which is more convenient with parameterized tests in pytest |
|
@magnatelee see #297 (comment) for some comments. There's definitely lots of options. E.g. a test wrapper (like |
@magnatelee actually I may not have fully appreciated this comment. Are you saying that there can be runtime error messages with tests still passing? If a runtime error causes a test to fail (or fail to run) then it should be captured and displayed at the end of the pytest report with or without Or, do you mean when running with
|
|
@magnatelee here is an extremely quick and dirty diff that restores stdout from tests when Would you like me to include it in this PR for now? I'd plan to fix it better in a PR dedicated to Edit: could also consider |
|
@bryevdv I wasn't talking about
This doesn't seem to be true. The pytest report only printed out any error messages from Python and masked those from the C++ runtime. I was able to see them only when I passed
Yes, I think we need both |
|
@mfoerste4 please confirm the merge conflict resolution in daa5ef9 |
|
Noting that 44771ea sets the default pytest capture mode to |
LGTM |
This PR convert the existing test modules to use
pytestfor test discovery and running. Tests were also updated for minor cleanup and to utilizepytestfeatures (e.g.parameterizeand fixtures) effectively. The end result is to afford testing options that are more familiar and ergonomic to "standard" python devs.Overview
Cleanup
A few commits perform some minor cleanup on the existing tests:
returnstatements throughoutFile movement
To make it easier to use
pytestfor test selection, the existing tests were moved and renamed:3f3c5cb — move files to
integrationsubdirectory9f4f410 — prefix all test module filenames with
test_Moving the files to a subdirectory allows this entire group of tests to be easily run by executing
pytestwith this directory path. Thetest_prefix is to conform withpytestexpectations for default test discovery.Pytest updates
test_prefix. By default,pytestwill interpret anything starting withtestorTestas test to run.pytestscaffolding to each test module and the least change to get running. For most tests this was just a couple of lines of boilerplate change. But a few tests required slightly more extensive updates.Notes
test.pymodule runs exactly as before. The only change was to update it for the new path to the test files(see note about verbose output, though)legate.coreissue that currently required fixtures to avoid cunumeric array re-use in testsparametrizeanywhere it made sense, so that more fine-grained reporting output becomes available:Further notes will be inline.
Operation
Running
test.pyThe existing way to run tests with
test.pyis unchanged:This still generates the existing report:
(Note: Currentlytest.py -vonly shows stdout fromtest.pyand not from tests. This will be straightforward to restore but I would like to do that in a follow-on PR dedicated totest.py. Test stdout can be observed with either of the methods below with the standard-sflag. cc @magnatelee)Executing individual tests
Individual tests can still be executed by running the module with
legateincluding by passing in runtime command line options:However, now the output is a standard
pytestreport:It is also now possible to pass standard
pytestsoptions, e.g.-vfor a verbose report:Using
pytestfor test discoveryAlthough it is not yet possible to simply run
pytest <dir>in the typical way, it is possible to achieve the same operation with a little more explicit invocation:Note that all the standard
pytestoptions, e.g. incluing-kand-mfor test filtering, can be used.The above will run all the tests under
tests/integration, and generate a standard combinedpytestreport:The full report output text can be seen here:
There are some warning that show up in the output at the end, mostly about implicit casting. I am not sure if these are expected or not (I expect they would be)
Future work
Things that are not in this PR but are planned for follow-on PRs:
pytestto run the examplesRestoring previous-vbehaviour totest.pylegate -m pytest