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.

Sign up for GitHub

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

Jump to bottom

[MicroTVM][PyTest] Explicitly skip MicroTVM unittests. #9335

Merged
merged 2 commits into from
Nov 10, 2021
Merged

[MicroTVM][PyTest] Explicitly skip MicroTVM unittests. #9335

merged 2 commits into from
Nov 10, 2021

Conversation

Lunderberg
Copy link
Contributor

Refactor unit tests so they will show as skipped if USE_MICRO=OFF.

Mousius
Mousius approved these changes Oct 21, 2021
View reviewed changes
Copy link
Member

@Mousius Mousius left a comment

Choose a reason for hiding this comment

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

LGTM with a few suggestions but otherwise always happy to see pytest being given more information about the tests 😸

tests/python/unittest/test_micro_project_api.py Outdated

class BaseTestHandler(project_api.server.ProjectAPIHandler):
class BaseTestHandler(project_api.server.ProjectAPIHandler):
Copy link
Member

Choose a reason for hiding this comment

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

Unsure about the name shadowing here, but this is neat.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you. Most of the time I would try to move the possibly-missing imports into the test itself, but here it wasn't possible without moving the entire class definition as well. Good point on the name shadowing, and I've updated the inner class definition to be "BaseTestHandler_Impl" instead.

tests/python/unittest/test_micro_transport.py Outdated
@@ -26,8 +26,7 @@
import tvm.testing


@tvm.testing.requires_micro
class TransportLoggerTests(unittest.TestCase):
def test_transport_class():
Copy link
Member

Choose a reason for hiding this comment

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

This would be nice as a fixture as above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I hadn't done so initially because this file was written with unittest instead of pytest. I've refactored this entire file to use pytest, and to split out the different tests of functionality into different test cases, with the class definition now included in a fixture.

@Lunderberg Lunderberg force-pushed the pytest_skipif_micro branch 2 times, most recently from 90384ea to 9211c71 Compare October 22, 2021 15:49
@Lunderberg
[MicroTVM][PyTest] Explicitly skip MicroTVM unittests.
73efc2f 
Refactor unit tests so they will show as skipped if `USE_MICRO=OFF`.
@Lunderberg
Updates following PR review.
8ce1e74 
- Updated to avoid name shadowing of BaseTestHandler

- Updated test_micro_transport to use fixture for setup.  Ended up
  needing to refactor to use pytest instead of unittest, split up test
  functionality during refactor.
areusch
areusch approved these changes Nov 10, 2021
View reviewed changes
Copy link
Contributor

@areusch areusch left a comment

Choose a reason for hiding this comment

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

thanks @Lunderberg , this is a neat trick!

@areusch areusch merged commit 0fd83eb into apache:main Nov 10, 2021
@Lunderberg Lunderberg deleted the pytest_skipif_micro branch November 10, 2021 03:40
AndrewZhaoLuo added a commit to AndrewZhaoLuo/tvm that referenced this pull request Nov 12, 2021
@AndrewZhaoLuo
Merge branch 'main' into aluo/qnn/conv2d-transpose-fq2i
da7a16b 
* main: (119 commits)
  [Topi][Op][PyTorch][Vitas] Fix inconsistent kernel layout conventions for conv2d_transpose (apache#9336)
  Fix repository URL in ubuntu_install_rocm.sh (apache#9425)
  Add LLVM-13 installation to Docker setup (apache#9498)
  [Relay] Use target_host determined at Relay level instead of recalculating it (apache#9499)
  Arm(R) Ethos(TM)-U NPU BinaryElementwise operators support (apache#9442)
  [COMMUNITY] Junru's and Wuwei's PGP key for ASF release (apache#9488)
  Add default for split op (apache#9489)
  [HOTFIX][TARGET] Change LOG in compilation config to DLOG (apache#9486)
  Fixed some warnings about lambda's closures that are bigger than necessary (apache#9481)
  [Support] Add libinfo into the runtime build (apache#9310)
  Change Call with TIRCallAttrs to call_lowered op (apache#9312)
  [ETHOSN] Streamline Ethos(TM)-N cross-compile rpc usage (apache#9477)
  [CMSIS-NN] Assert correct amount of CMSIS-NN artifacts in MLF (apache#9480)
  [MicroTVM][PyTest] Explicitly skip MicroTVM unittests. (apache#9335)
  [microNPU] Replace ICHECK with diagnostic context in type inference (apache#9470)
  Better host handling in CompilationConfig & debug printing (apache#9460)
  [AOT][Tests] Use pre-built libraries in Reference System tests (apache#9271)
  [TIR] Add type hint for TIR  (apache#9432)
  [TVMC] Add test for quantized pytorch model (apache#9467)
  [CMSIS-NN] Convert CMSIS-NN to use Target Hooks (apache#9397)
  ...
mehrdadh pushed a commit to mehrdadh/tvm that referenced this pull request Dec 1, 2021
@Lunderberg @mehrdadh
[MicroTVM][PyTest] Explicitly skip MicroTVM unittests. (apache#9335)
1982101 
* [MicroTVM][PyTest] Explicitly skip MicroTVM unittests.

Refactor unit tests so they will show as skipped if `USE_MICRO=OFF`.

* Updates following PR review.

- Updated to avoid name shadowing of BaseTestHandler

- Updated test_micro_transport to use fixture for setup.  Ended up
  needing to refactor to use pytest instead of unittest, split up test
  functionality during refactor.
mehrdadh pushed a commit to mehrdadh/tvm that referenced this pull request Dec 1, 2021
@Lunderberg @mehrdadh
[MicroTVM][PyTest] Explicitly skip MicroTVM unittests. (apache#9335)
9b2ba91 
* [MicroTVM][PyTest] Explicitly skip MicroTVM unittests.

Refactor unit tests so they will show as skipped if `USE_MICRO=OFF`.

* Updates following PR review.

- Updated to avoid name shadowing of BaseTestHandler

- Updated test_micro_transport to use fixture for setup.  Ended up
  needing to refactor to use pytest instead of unittest, split up test
  functionality during refactor.
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 7, 2022
@Lunderberg @ylc
[MicroTVM][PyTest] Explicitly skip MicroTVM unittests. (apache#9335)
a074c93 
* [MicroTVM][PyTest] Explicitly skip MicroTVM unittests.

Refactor unit tests so they will show as skipped if `USE_MICRO=OFF`.

* Updates following PR review.

- Updated to avoid name shadowing of BaseTestHandler

- Updated test_micro_transport to use fixture for setup.  Ended up
  needing to refactor to use pytest instead of unittest, split up test
  functionality during refactor.
yangulei pushed a commit to yangulei/tvm that referenced this pull request Jan 11, 2022
@Lunderberg @yangulei
[MicroTVM][PyTest] Explicitly skip MicroTVM unittests. (apache#9335)
62d61d2 
* [MicroTVM][PyTest] Explicitly skip MicroTVM unittests.

Refactor unit tests so they will show as skipped if `USE_MICRO=OFF`.

* Updates following PR review.

- Updated to avoid name shadowing of BaseTestHandler

- Updated test_micro_transport to use fixture for setup.  Ended up
  needing to refactor to use pytest instead of unittest, split up test
  functionality during refactor.
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 13, 2022
@Lunderberg @ylc
[MicroTVM][PyTest] Explicitly skip MicroTVM unittests. (apache#9335)
ae519dd 
* [MicroTVM][PyTest] Explicitly skip MicroTVM unittests.

Refactor unit tests so they will show as skipped if `USE_MICRO=OFF`.

* Updates following PR review.

- Updated to avoid name shadowing of BaseTestHandler

- Updated test_micro_transport to use fixture for setup.  Ended up
  needing to refactor to use pytest instead of unittest, split up test
  functionality during refactor.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@areusch areusch areusch approved these changes

@Mousius Mousius Mousius approved these changes

@comaniac comaniac Awaiting requested review from comaniac

@jroesch jroesch Awaiting requested review from jroesch

@junrushao junrushao Awaiting requested review from junrushao

@merrymercy merrymercy Awaiting requested review from merrymercy

@tqchen tqchen Awaiting requested review from tqchen

@yzhliu yzhliu Awaiting requested review from yzhliu

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Lunderberg @areusch @Mousius