Repository navigation
ci: parallel test shards, nested unit tests, and a pre-push GIT_DIR leak (draft, measuring) - #233
Closed
amirkhakshour wants to merge 3 commits into
Closed
amirkhakshour wants to merge 3 commits into
amirkhakshour wants to merge 3 commits into
Conversation
The unit shards ran serially and took 23-39 min of pytest each (36-39 min on unit-a), against a 50 min timeout raised two days ago to stop passing runs being cancelled. Setup is seconds; it is all pytest. Measured before changing anything: the whole tests/unit tier on 8 pytest-xdist workers passes (14,000 passed, 58 skipped, 0 failed, 0 errors) in 13.6 min, and the rest tier in 48 s instead of 123 s. The time is spread thin -- the slowest file is ~4% of the total and the top 30 files ~31% -- so parallelism is what moves it, not trimming single tests. - install pytest-xdist and run every shard with `-n auto` - lower the timeout to 30: a shard that needs more is a hang or a regression worth seeing Claude-Session: https://claude.ai/code/session_01E9VqwxxzV8Ad926sce4CgW
`ls tests/unit/*.py` matches top-level files only, and the rest shard passes --ignore=tests/unit, so tests/unit/acp/ (2 files, 28 tests) ran in no CI job. `find tests/unit -name 'test_*.py' | sort` reaches them; every top-level file already matches test_*.py, so nothing drops out. All 28 pass. The 804 files still split one-to-one across the shards. Also record the evidence for running the rest shard under xdist: 734 passed on 8 workers; its one local failure (test_no_populated_table_is_dark, which audits ./.mini-ork/state.db) reproduces serially and passes in CI. Found by pre-push review 175. Claude-Session: https://claude.ai/code/session_01E9VqwxxzV8Ad926sce4CgW
git exports GIT_DIR to hooks (always, in a linked worktree), and every layer of this hook inherits os.environ, including the review lenses, which are agents that may run the test suite. A test that runs `git init` / `add -A` / `commit` in a temp dir then operates on the pushing repository instead: `git init` re-initialises it, `add -A` from the temp dir replaces its index with the fixture, and `git config` writes to the shared .git/config. That happened on 2026-10-29 during one push of wt/ci-speed: 5 fixture commits landed on the branch (the tree went from 3,574 files to 8), and `[user] name = t, email = t@t` was written to the primary's .git/config, so every session committing in this repo authored as `t` until it was removed. One other session's commit on local main picked it up. Drop the repo-binding variables at entry, after the chdir to the worktree root, so git rediscovers the same repository from cwd. The list comes from `git rev-parse --local-env-vars`, which is what git clears for a submodule process. Reproduced in a scratch repo with a linked worktree: without the fix the hook's grandchild commits onto the branch and sets user.name; with it, neither happens. Claude-Session: https://claude.ai/code/session_01E9VqwxxzV8Ad926sce4CgW
amirkhakshour
added a commit
that referenced
this pull request
Oct 31, 2026
Review 175 and 177 both objected that 30 rested on a dev-box run. PR #233 measured it on the real 4-vCPU runners with -n auto: pytest took 16.1 / 11.6 min (unit-a, 3.11 / 3.12) and 10.8 / 13.5 (unit-b), down from 35.6 / 38.9 and 22.9 / 27.4 serially; 14,004 passed per Python version. 30 keeps the old ~1.8x headroom over the slowest shard. Claude-Session: https://claude.ai/code/session_01E9VqwxxzV8Ad926sce4CgW
Contributor
Author
|
Landed on main through |
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.
Draft, for measuring only. This PR exists to run CI on real runners before the change lands; it will be closed, and the branch merged through
make worktree-merge.What changes
pytest-xdist,-n auto). The unit shards ran serially and spent 23–39 min in pytest (36–39 on unit-a), against a 50 min timeout. Setup is seconds.tests/unittier passes on 8 workers (14,000 passed, 58 skipped, 0 failed, 0 errors) in 13.6 min; the rest tier in 48 s instead of 123 s.ls tests/unit/*.pynever reachedtests/unit/acp/(2 files, 28 tests), and the rest shard ignorestests/unit, so they ran in no job.find tests/unit -name 'test_*.py' | sortcovers all 804 files, one shard each. Found by pre-push review 175..githooks/pre-pushstops its children writing to the pushing repo. git exportsGIT_DIRto hooks (always in a linked worktree), and the review lenses inherit it. A test doinggit init/add -A/commit/configin a temp dir then operates on the pushing repo. During one push of this branch that left 5 fixture commits on it (tree 3,574 → 8 files) and[user] name = tin the shared.git/config; in the same windowcore.bare = truewas written there too (git init --barewith no path, under an inheritedGIT_DIR), which broke every work-tree git command in the primary. The hook now dropsgit rev-parse --local-env-varsat entry. All three effects reproduced in a scratch linked worktree and gone with the fix; the 269 existing hook tests pass.Measured here
To be filled from this PR's CI run: per-shard pytest wall time on 3.11 / 3.12.
https://claude.ai/code/session_01E9VqwxxzV8Ad926sce4CgW