Skip to content
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

[CI] Refactor of tvm.testing.requires_* annotations #11313

Merged
merged 9 commits into from
Jun 3, 2022

Conversation

Lunderberg
Copy link
Contributor

@Lunderberg Lunderberg commented May 13, 2022

This PR has two main goals.

  • Improve the pytest messages when unit tests are skipped. When investigating the bug in [Hexagon]Use requires_hexagon instead of requires_hexagon_toolchain if running on hexagon target #11294, reporting the reason why a test is skipped (disabled in config.cmake, disabled in TVM_TEST_TARGETS, or no device available).
  • Distinguish between tests that only require compile-time support from tests that require a physical device. The existing @tvm.testing.requires_* decorators maintain the same semantics, skipping a test unless both compile-time and run-time support are present. This adds an optional argument to indicate that the test does not require a physical device (e.g. to require cuda library support without requiring a cuda-capable GPU: @requires_cuda(support_required="compile-only")

cc @Mousius @areusch @driazati

@Lunderberg
Copy link
Contributor Author

@mehrdadh I expect this to conflict with these changes in PR#11294. Between the two, your PR takes priority, as it is solving an immediate issue. If all goes well, I don't expect the conflict to be difficult to resolve, as the corresponding change after this refactor would be to replace target_kind_hardware="hexagon" with run_time_check=lambda: "ANDROID_SERIAL_NUMBER" in os.environ in the definition of requires_hexagon, and to replace requires_hexagon_toolchain with requires_hexagon(support_required="compile-only").

Previously, the same message was given regardless of why a test
couldn't be run.  This has been split up into separate checks for TVM
cmake options in `config.cmake`, enabled targets in `TVM_TEST_TARGETS`
environment variable, and checks for available hardware.
@Lunderberg Lunderberg force-pushed the requires_feature_refactor branch from 4a5087f to 7bd8211 Compare May 18, 2022 16:30
@Lunderberg Lunderberg marked this pull request as ready for review May 31, 2022 14:32
Copy link
Member

@mehrdadh mehrdadh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the refactor! LGTM!
cc @areusch and @driazati to take a look as well

@mehrdadh mehrdadh merged commit b885362 into apache:main Jun 3, 2022
@Lunderberg Lunderberg deleted the requires_feature_refactor branch June 3, 2022 21:05
Lunderberg added a commit to Lunderberg/tvm that referenced this pull request Jun 7, 2022
These were enabled in apache#11294, then
erroneously disabled in apache#11313.
This applies the same fix as in
apache#11294, checking the
`ANDROID_SERIAL_NUMBER` to determine if Hexagon tests can execute at
runtime, but using the refactored `pytest.skipif` messages introduced
in apache#11313.
Lunderberg added a commit to Lunderberg/tvm that referenced this pull request Jun 7, 2022
These were enabled in apache#11294, then
erroneously disabled in apache#11313.
This applies the same fix as in
apache#11294, checking the
`ANDROID_SERIAL_NUMBER` to determine if Hexagon tests can execute at
runtime, but using the refactored `pytest.skipif` messages introduced
in apache#11313.
kparzysz-quic pushed a commit that referenced this pull request Jun 7, 2022
* [Hexagon][CI] Re-enable Hexagon tests in CI

These were enabled in #11294, then
erroneously disabled in #11313.
This applies the same fix as in
#11294, checking the
`ANDROID_SERIAL_NUMBER` to determine if Hexagon tests can execute at
runtime, but using the refactored `pytest.skipif` messages introduced
in #11313.

* Fixed circular dependency, but feels somewhat ugly
Kathryn-cat pushed a commit to Kathryn-cat/tvm that referenced this pull request Jun 10, 2022
* [Hexagon][CI] Re-enable Hexagon tests in CI

These were enabled in apache#11294, then
erroneously disabled in apache#11313.
This applies the same fix as in
apache#11294, checking the
`ANDROID_SERIAL_NUMBER` to determine if Hexagon tests can execute at
runtime, but using the refactored `pytest.skipif` messages introduced
in apache#11313.

* Fixed circular dependency, but feels somewhat ugly
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants