[microTVM] Replace static fixtures with parameterization #12530
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mehrdadh
reviewed
Aug 22, 2022
|
|
||
| def pytest_addoption(parser): | ||
| """Adds more pytest arguments""" | ||
| parser.addoption( |
Member
There was a problem hiding this comment.
doesn't this force every test to have --platform as an argument? In that case we don't want this for tests that are specific to Zephyr/Arduino
Member
Author
There was a problem hiding this comment.
Nope! Only tests that include the platform fixture will require the --platform argument, and likewise for board. This is done by the if argument in metafunc.fixturenames line.
Member
There was a problem hiding this comment.
cool, thanks for clarification!
mehrdadh
approved these changes
Aug 22, 2022
xinetzone
pushed a commit
to daobook/tvm
that referenced
this pull request
Nov 25, 2022
* Replace microTVM static fixtures with parameterization * [microTVM] Only perform parameterization when fixture is present * Reformat with black * Fix Cortex-M tests * Add docstring to pytest_generate_tests * Remove trailing space from docstring
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#12207 went a long way to improving microTVM code reuse, but introduced an unintentional? change. Previously, the full
pytestnames of microTVM tests included the value ofboard, prepended to any paremeterization. For example,test_rpc_large_arraywould have the name:This behavior was useful, as it meant that exported Junit XML files would correctly record which board the test was run on. However, after #12207 the names of tests stopped including this information:
This prevented Junit from differentiating between the tests when they were run on different boards, and instead just overwrote results on previous boards. Additionally, we never made this distinction for
commontests that were parameterized byplatform, so tests liketests.micro.common.test_autotune.test_kws_autotune_workflowwould have one of the platforms overwrite the other (with whichever one finished last being the one that was ultimately reported).This PR uses the
pytest_generate_testshook to fix this issue. Now, theboardparameter is included in the test name as before, and theplatformparameter will be included for any tests that use that fixture (e.g. common tests).cc @alanmacd @gromero @mehrdadh