[Hexagon]Use requires_hexagon instead of requires_hexagon_toolchain if running on hexagon target#11294
Conversation
| def test_speedup(hexagon_session, capsys): | ||
| @tvm.testing.requires_hexagon | ||
| def test_speedup(hexagon_session: Session, capsys): | ||
| if hexagon_session is None: |
There was a problem hiding this comment.
Since hexagon_session will be non-null whenever ANDROID_SERIAL_NUMBER is set, and tvm.testing.requires_hexagon requires that ANDROID_SERIAL_NUMBER is set, should we also use this to remove the if hexagon_session is None checks?
There was a problem hiding this comment.
make sense, removed the if condition.
Lunderberg
left a comment
Lunderberg
left a comment
There was a problem hiding this comment.
LGTM! One question for clean-ups made possible by this change, which could go in this or a future PR.
Lunderberg
left a comment
Lunderberg
left a comment
There was a problem hiding this comment.
Though, double-checking the test results themselves, it looks like the tests are being skipped in CI. (example link). We should investigate why these are being skipped.
|
At first glance, nothing is obviously standing out as the cause of the skipped CI tests. The |
mehrdadh
force-pushed
the
hexagon/fix_require_hexagon
branch
from
34d32e6 to
d229814
Compare
mehrdadh
left a comment
mehrdadh
left a comment
There was a problem hiding this comment.
@Lunderberg thanks for the review. The issue was the is_enabled condition which I forgot to change correctly. Not it is fixed:
is_enabled = tvm.support.libinfo()["USE_HEXAGON"].lower() in ["on", "true", "1"]
| def test_speedup(hexagon_session, capsys): | ||
| @tvm.testing.requires_hexagon | ||
| def test_speedup(hexagon_session: Session, capsys): | ||
| if hexagon_session is None: |
There was a problem hiding this comment.
make sense, removed the if condition.
|
testing on hardware CI to see how many test are failing there. |
|
Hardware test passes and I don't see any skip for Hexagon target. Only one test failed: We should investigate that in a follow up PR. |
…f running on hexagon target (apache#11294) * refactor requires_hexagon_toolchain * trigger * lint
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.
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.
* [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
* [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
2 participants
We use
requires_hexagontest fixture when we want to run on hexagon target.requires_hexagon_toolchainis used only for cases where we want to build it but not running it.