Repository navigation
build(tensilelite): require pytest-xdist 3.2 or newer - #14798
Merged
Merged
Conversation
Alex-Vasile
enabled auto-merge (squash)
October 30, 2026 21:54
github-actions
Bot
added
project: hipblaslt
project: hipsparselt
ci:hipsparselt-fast
labels
Merged
1 task done
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🎉 All checks passed! This PR is ready for review. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #14798 +/- ##
========================================
Coverage 77.51% 77.51%
========================================
Files 2903 2903
Lines 490384 490384
Branches 74217 74217
========================================
Hits 380084 380084
Misses 85390 85390
Partials 24910 24910
*This pull request uses carry forward flags. Click here to find out more. 🚀 New features to boost your workflow:
|
1 task done
newling
approved these changes
Nov 2, 2026
Contributor
PR Pundit review(Testing PR Pundit Code Review agent. Not all comments may be relevant. This is a one-time automated pass: please address what is useful, no need to request a re-review. Please reply to each point, whether it applies or not: we harvest these replies, and both kinds are enormously helpful for tuning the agent. Reach out to @peymanr for suggestions.)
1. 🟠 Should fix:
|
The --dist=worksteal scheduler first appeared in pytest-xdist 3.2. The requirements-test.txt, tox.ini and pyproject.toml lists all allowed pytest-xdist>=1.32.0, so an old cached environment satisfies them and then fails at startup with "argument --dist: invalid choice: 'worksteal'" (checked with xdist 3.1.0). Raise the limit in all three lists to >=3.2, which requirements-test.txt says have to stay equal. The branch that needs it is users/alvasile/unit-tests-xdist-worksteal-8-workers (#14605), which passes --dist=worksteal in the quick and standard categories of test_categories.yaml. Also update the pytest-xdist specifier recorded in uv.lock to match. The locked version (3.8.0) already satisfies it, so no resolved package changes.
Alex-Vasile
force-pushed
the
users/alvasile/tensilelite-require-xdist-3-2
branch
from
November 3, 2026 14:27
7e44605 to
ccdda5f
Compare
Alex-Vasile
added a commit
that referenced
this pull request
Nov 5, 2026
…eal and eight workers (#14605) ISSUE ID : #14049 ## Motivation The CI categories that run the TensileLite unit tests use one worker (`parallel_workers: 1`), so the whole suite runs in one process. The six PRs below remove the per-process costs that made running more workers unprofitable: machine-wide process pools, repeated assembler and architecture probes, and a slow collection phase. With them in place, running the suite in parallel pays off. ## Technical Details `test_categories.yaml` sets `--dist=worksteal` and `parallel_workers: 8` for the quick and standard categories. The comprehensive and full categories also run the GPU unit tests, which take no per-GPU lock, so eight workers would all share the runner's one GPU; they keep one worker until parallel GPU runs are validated. The `hw-common`, `ffm-quick`, `emu-fast` and `emu-full` categories keep their own values. TheRock's `pytest_runner.py` passes `--numprocesses` only when a category's value is above 1, so the quick and standard categories now run under pytest-xdist. `worksteal` needs pytest-xdist 3.2 or newer, which #14798 requires. The `TESTING.md` Known Risks row that said the job runs the unit tree on one worker in every tier is updated. Under xdist, syrupy no longer prints its snapshot summary or reports unused snapshots; mismatches still fail tests, and unused snapshots were only warnings. `worksteal` lets an idle worker take tests another worker has not started yet. The default `load` scheduler hands each worker a fixed block of consecutive tests and never takes them back. About half of the suite's runtime sits in one block (`characterization/_codegen`), so some workers finished early and sat idle. The CI job has a 25-CPU allocation, and every worker repeats its own startup, so more workers stop paying for themselves. In an earlier local sample under a 25-CPU limit, one worker took 394 s, eight took 98.5 s and 25 took 102.4 s. A comment block in the file records this. Only CI configuration changes. Unit-test run time for each PR in this stack, one run each on a gfx942 host pinned to 25 CPUs, serial except the last row. Collection is the median of 3 runs. Run phase is total minus collection. The first row is the commit this stack is based on. | Ref | Collection (s) | Run phase (s) | Total wall (s) | |---|---|---|---| | b279712 (base) | 52.3 | 600.4 | 652.7 | | #14599 | 49.4 | 461.1 | 510.5 | | #14600 | 47.0 | 456.6 | 503.6 | | #14601 | 50.9 | 443.9 | 494.8 | | #14602 | 12.1 | 461.1 | 473.2 | | #14603 | 7.8 | 466.8 | 474.6 | | #14604 | 7.6 | 311.6 | 319.2 | | #14605 | 7.7 | 313.7 | 321.4 | | #14605 (`-n 8 --dist=worksteal`) | n/a | n/a | 125.6 | ## Test Plan Run the unit tests serially and with the new arguments, and compare outcomes: `pytest -m "not gpu" --snapshot-warn-unused tensilelite/Tests/unit` and the same command with `-n 8 --dist=worksteal`. The categories file is read by TheRock's `pytest_runner.py`, which is not in this repository, so I reproduced its arguments by hand. ## Test Result - Serial, one worker, the baseline for this PR: 321 s. - `-n 8 --dist=worksteal`: 126 s, with the same outcomes as the serial run. Both were single runs on gfx942, pinned to 25 CPUs. - The same suite took 653 s on develop. Measured cumulatively, the six PRs below bring it to 321 s and this PR brings it to 126 s. - I only ran `tensilelite/Tests/unit`, not `rocisa_tests`, and not through `pytest_runner.py`. The first CI run on this branch is the real check of how the runner reads the file. - `pre-commit run --files` on the changed file passes. Automated GitHub checks are pending. ## Related - Depends on #14798 (pytest-xdist 3.2 floor). - Work tracking: [#14049](#14049), the issue about the unit tests exceeding the 15-minute quick-tier limit on gfx94X. This PR is part of the follow-up to [#14058](#14058), which fixed the immediate cause. ## Device / Architecture Coverage CI configuration only; no product code or GPU behavior changes. The hardware categories, and the comprehensive and full categories that include the GPU unit tests, keep their current worker counts, so passing PR checks are sufficient. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/TheRock/blob/main/GOVERNANCE.md#pull-requests. ## Risk level Low (2/5): CI configuration only. If eight workers contend on a CI machine, jobs could run slower or fail intermittently; reverting the file restores one worker.
Contributor
TheRock Submodule Bump ActivityNewest first
|
1 task done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ISSUE ID : #14049
Motivation
#14605 (branch
users/alvasile/unit-tests-xdist-worksteal-8-workers) passes--dist=workstealto pytest in thequickandstandardcategories oftest_categories.yaml. That scheduler first appeared in pytest-xdist 3.2, but the TensileLite dependency lists still saypytest-xdist>=1.32.0. An old cached or pinned environment satisfies that limit and then fails at startup. With xdist 3.1.0, pytest exits withargument --dist: invalid choice: 'worksteal', so the whole category fails. A fresh install resolves to a newer version today, which is why nothing has broken yet.Technical Details
Raises the lower bound to
pytest-xdist>=3.2in the three places that list it:requirements-test.txt(the artifact-based test requirements TheRock installs), thetox.initest deps and thepyproject.tomldev group.requirements-test.txtsays its limits are meant to stay equal to the other two. pytest-xdist 3.2 requires pytest 6.2 or newer, so pip will also pick a newer pytest than thepytest>=5.4.1in those lists. Only dependency metadata changes.Test Plan
Install pytest-xdist 3.1.0 and 3.2.0 into separate directories and run
pytest -n 2 --dist=workstealon a two-test file with each.Test Result
argument --dist: invalid choice: 'worksteal'.-n 2 --dist=worksteal.pre-commit run --fileson the three changed files passes.Related
Device / Architecture Coverage
Dependency metadata only; no product code, device or architecture behavior changes. Passing PR checks are sufficient.
Submission Checklist
Risk level
Low (1/5): raises a test-only dependency floor to a version that a fresh install already resolves to.