-
Notifications
You must be signed in to change notification settings - Fork 684
[Backend Tester] Migrate to pytest #14456
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
Conversation
Stack from ghstack (oldest at bottom): |
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/14456
Note: Links to docs will display an error until the docs builds have been completed. ❗ 1 Active SEVsThere are 1 currently active SEVs. If your PR is affected, please view them below: ❌ 5 New Failures, 2 Cancelled Jobs, 2 Unrelated FailuresAs of commit 92eac20 with merge base bef9555 ( NEW FAILURES - The following jobs have failed:
CANCELLED JOBS - The following jobs were cancelled. Please retry:
FLAKY - The following job failed but was likely due to flakiness present on trunk:
BROKEN TRUNK - The following job failed but was present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
return _ALL_TEST_FLOWS | ||
|
||
|
||
def dtype_to_str(dtype: torch.dtype) -> str: |
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.
This utility function is used for generating the display name for parameterized tests.
|
||
test_kwargs = copy.copy(params) or {} | ||
test_kwargs["flow"] = flow | ||
def wrap_test(original_func, test_type): |
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.
This is shim code to make the existing op tests work with the new pytest setup. It will be removed once I update all of the op tests. Note that this is not a functional change, just code cleanup. It's using the proper pytest fixtures and parameterization with this PR, just through this shim instead of directly.
.ci/scripts/test_backend_linux.sh
Outdated
|
||
EXIT_CODE=0 | ||
python -m executorch.backends.test.suite.runner $SUITE --flow $FLOW --report "$REPORT_FILE" || EXIT_CODE=$? | ||
pytest -c /dev/nul -n auto backends/test/suite/$SUITE/ -m flow_$FLOW --json-report --json-report-file="$REPORT_FILE" || EXIT_CODE=$? |
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.
FYI we don't exactly love pytest when not working in OSS.
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.
Do you have concerns with using pytest? Or suggested changes?
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.
Thanks!
|
||
|
||
@pytest.hookimpl(optionalhook=True) | ||
def pytest_json_runtest_metadata(item, call): |
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.
For pytest-json-report
I assume?
${CONDA_RUN} --no-capture-output pip install awscli==1.37.21 | ||
source .ci/scripts/test_backend_macos.sh "${{ matrix.suite }}" "${{ matrix.flow }}" "${RUNNER_ARTIFACT_DIR}" | ||
source .ci/scripts/test_backend.sh "${{ matrix.suite }}" "${{ matrix.flow }}" "${RUNNER_ARTIFACT_DIR}" |
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.
+1
@pytorchbot cherry-pick --onto release/1.0 -c testci |
❌ 🤖 pytorchbot command failed:
Try |
@pytorchbot cherry-pick --onto release/1.0 -c fixnewfeature |
Refactor the backend test suites to use pytest. This includes the following changes: * Define pytest markers for each backend and test flow (recipe). This allows for easy filter, such as by running `pytest some/path/... -m backend_xnnpack`. * Use a parameterized pytest fixture to handle test generation / expansion for each test flow. * Switch to using the pytest-json-report plugin for reporting. Update the markdown generation script to take json. * Shim the existing unittest-based logic for op tests. * I've updated add.py to show what they should look like long-term. I've also just updated the model tests, since there aren't as many. I'll update the remaining op tests later in this stack, though this is purely to clean up the code. The shimming logic makes them work properly with pytest in this PR. * Update the backend test CI to use pytest. This also has the benefit of making the jobs much faster by leveraging parallel execution. I've also added a repro command to the markdown summary. (cherry picked from commit d09dd79)
Cherry picking #14456The cherry pick PR is at #14661 and it is recommended to link a fixnewfeature cherry pick PR with an issue. The following tracker issues are updated: Details for Dev Infra teamRaised by workflow job |
Refactor the backend test suites to use pytest. This includes the following changes:
pytest some/path/... -m backend_xnnpack
.This also has the benefit of making the jobs much faster by leveraging parallel execution. I've also added a repro command to the markdown summary.