-
Notifications
You must be signed in to change notification settings - Fork 3.8k
[microTVM] Refactor pytest fixtures #12207
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
Changes from all commits
0b6e33a
2cfd878
b416bd3
d9c774c
989facd
d5c0ba5
1c51152
4493112
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,111 @@ | ||
| # Licensed to the Apache Software Foundation (ASF) under one | ||
| # or more contributor license agreements. See the NOTICE file | ||
| # distributed with this work for additional information | ||
| # regarding copyright ownership. The ASF licenses this file | ||
| # to you under the Apache License, Version 2.0 (the | ||
| # "License"); you may not use this file except in compliance | ||
| # with the License. You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, | ||
| # software distributed under the License is distributed on an | ||
| # "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| # KIND, either express or implied. See the License for the | ||
| # specific language governing permissions and limitations | ||
| # under the License. | ||
|
|
||
| # pylint: disable=invalid-name,redefined-outer-name | ||
| """ microTVM testing fixtures used to deduce testing argument | ||
| values from testing parameters """ | ||
|
|
||
| import pathlib | ||
| import os | ||
| import datetime | ||
| import pytest | ||
|
|
||
| from tvm.contrib.utils import tempdir | ||
|
|
||
| from .utils import get_supported_boards | ||
|
|
||
|
|
||
| def pytest_addoption(parser): | ||
| """Adds more pytest arguments""" | ||
| parser.addoption( | ||
| "--board", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Love this change! It has been a long time in the workds.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should also document explicitly the difference between
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added more details
areusch marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| required=True, | ||
| choices=list(get_supported_boards("zephyr").keys()) | ||
| + list(get_supported_boards("arduino").keys()), | ||
| help=( | ||
| "microTVM boards for tests. Board refers to instances" | ||
| "of microcontrollers/emulators defined in a platform." | ||
| ), | ||
| ) | ||
| parser.addoption( | ||
| "--test-build-only", | ||
| action="store_true", | ||
| help="Only run tests that don't require physical hardware.", | ||
| ) | ||
| parser.addoption( | ||
mehrdadh marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| "--microtvm-debug", | ||
| action="store_true", | ||
| default=False, | ||
| help=( | ||
| "If set true, it will keep the project directory for debugging." | ||
| "Also, it will enable debug level logging in project generation." | ||
| ), | ||
| ) | ||
|
|
||
|
|
||
| @pytest.fixture(scope="session") | ||
| def board(request): | ||
| return request.config.getoption("--board") | ||
|
|
||
|
|
||
| @pytest.fixture(scope="session") | ||
| def microtvm_debug(request): | ||
| return request.config.getoption("--microtvm-debug") | ||
|
|
||
|
|
||
| def pytest_collection_modifyitems(config, items): | ||
| if config.getoption("--test-build-only"): | ||
| skip_hardware_tests = pytest.mark.skip(reason="--test-build-only was passed") | ||
| for item in items: | ||
| if "requires_hardware" in item.keywords: | ||
| item.add_marker(skip_hardware_tests) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def workspace_dir(request, board, microtvm_debug): | ||
| """Creates workspace directory for each test.""" | ||
| parent_dir = pathlib.Path(os.path.dirname(request.module.__file__)) | ||
| board_workspace = ( | ||
| parent_dir / f"workspace_{board}" / datetime.datetime.now().strftime("%Y-%m-%dT%H-%M-%S") | ||
areusch marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ) | ||
| board_workspace_base = str(board_workspace) | ||
| number = 1 | ||
| while board_workspace.exists(): | ||
| board_workspace = pathlib.Path(board_workspace_base + f"-{number}") | ||
| number += 1 | ||
|
|
||
| if not os.path.exists(board_workspace.parent): | ||
| os.makedirs(board_workspace.parent) | ||
|
|
||
| keep_for_debug = microtvm_debug if microtvm_debug else None | ||
| test_temp_dir = tempdir(custom_path=board_workspace, keep_for_debug=keep_for_debug) | ||
| return test_temp_dir | ||
|
|
||
|
|
||
| @pytest.fixture(autouse=True) | ||
| def skip_by_board(request, board): | ||
| """Skip test if board is in the list.""" | ||
| if request.node.get_closest_marker("skip_boards"): | ||
| if board in request.node.get_closest_marker("skip_boards").args[0]: | ||
| pytest.skip("skipped on this board: {}".format(board)) | ||
|
|
||
|
|
||
| def pytest_configure(config): | ||
| config.addinivalue_line( | ||
| "markers", | ||
| "skip_boards(board): skip test for the given board", | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| # test workspaces | ||
| */workspace_*/ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,35 +15,19 @@ | |
| # specific language governing permissions and limitations | ||
| # under the License. | ||
|
|
||
| import pytest | ||
| pytest_plugins = [ | ||
| "tvm.micro.testing.pytest_plugin", | ||
| ] | ||
|
|
||
| from test_utils import ARDUINO_BOARDS | ||
| import pytest | ||
|
|
||
|
|
||
| def pytest_addoption(parser): | ||
| parser.addoption( | ||
| "--arduino-board", | ||
| nargs="+", | ||
| required=True, | ||
| choices=ARDUINO_BOARDS.keys(), | ||
| help="Arduino board for tests.", | ||
| ) | ||
| parser.addoption( | ||
| "--arduino-cli-cmd", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it worth abstracting this parameter to "build tool path"? I could be convinced either way.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this way is more clear, specially when used in tvmc
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yea you’re right, build tool is just too abstract. |
||
| default="arduino-cli", | ||
| help="Path to `arduino-cli` command for flashing device.", | ||
| ) | ||
| parser.addoption( | ||
| "--test-build-only", | ||
| action="store_true", | ||
| help="Only run tests that don't require physical hardware.", | ||
| ) | ||
| parser.addoption( | ||
| "--tvm-debug", | ||
| action="store_true", | ||
| default=False, | ||
| help="If given, enable a debug session while the test is running. Before running the test, in a separate shell, you should run: <python -m tvm.exec.microtvm_debug_shell>", | ||
| ) | ||
|
|
||
|
|
||
| def pytest_configure(config): | ||
|
|
@@ -52,27 +36,6 @@ def pytest_configure(config): | |
| ) | ||
|
|
||
|
|
||
| def pytest_collection_modifyitems(config, items): | ||
| if config.getoption("--test-build-only"): | ||
| skip_hardware_tests = pytest.mark.skip(reason="--test-build-only was passed") | ||
| for item in items: | ||
| if "requires_hardware" in item.keywords: | ||
| item.add_marker(skip_hardware_tests) | ||
|
|
||
|
|
||
| # We might do project generation differently for different boards in the future | ||
| # (to take advantage of multiple cores / external memory / etc.), so all tests | ||
| # are parameterized by board | ||
| def pytest_generate_tests(metafunc): | ||
| board = metafunc.config.getoption("arduino_board") | ||
| metafunc.parametrize("board", board, scope="session") | ||
|
|
||
|
|
||
| @pytest.fixture(scope="session") | ||
| def arduino_cli_cmd(request): | ||
| return request.config.getoption("--arduino-cli-cmd") | ||
|
|
||
|
|
||
| @pytest.fixture(scope="session") | ||
| def tvm_debug(request): | ||
| return request.config.getoption("--tvm-debug") | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,23 +36,24 @@ | |
| 6. Use serial connection to ensure model behaves correctly | ||
| """ | ||
|
|
||
|
|
||
| # Since these tests are sequential, we'll use the same project/workspace | ||
| # directory for all tests in this file | ||
| @pytest.fixture(scope="module") | ||
| def workspace_dir(request, board): | ||
| def workflow_workspace_dir(request, board): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Like this change! |
||
| return test_utils.make_workspace_dir("arduino_workflow", board) | ||
|
|
||
|
|
||
| @pytest.fixture(scope="module") | ||
| def project_dir(workspace_dir): | ||
| return workspace_dir / "project" | ||
| def project_dir(workflow_workspace_dir): | ||
| return workflow_workspace_dir / "project" | ||
|
|
||
|
|
||
| # We MUST pass workspace_dir, not project_dir, or the workspace will be dereferenced too soon | ||
| @pytest.fixture(scope="module") | ||
| def project(board, arduino_cli_cmd, tvm_debug, workspace_dir): | ||
| return test_utils.make_kws_project(board, arduino_cli_cmd, tvm_debug, workspace_dir) | ||
| def project(board, arduino_cli_cmd, microtvm_debug, workflow_workspace_dir): | ||
| return test_utils.make_kws_project( | ||
| board, arduino_cli_cmd, microtvm_debug, workflow_workspace_dir | ||
| ) | ||
|
|
||
|
|
||
| def _get_directory_elements(directory): | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we changing this to return
predicted_labels? There are a few cases where we'll want to overrideevaluate_model_accuracy(say, with the MLPerf Tiny model for anomaly detection) but there won't be a 1:1 correspondence of samples to predicted_labels (because anomaly detection uses an area of the curve metric). I'd prefer to keep it as is.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made this change since we were using the predicted labels outside of this function for validity check in some tests. If you think it's out of the scope of this function, I can replicated the whole function in the other repo. thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I'd forgotten we need the labels for some hardware in the loop tests. That seems fine - for anomaly detection and other AOC metrics using the same "template", we could use confidence values (or even just
None) in place ofpredicted_labels. This LGTM now.