test(ci): probe an unmapped Python file through the fallback - #6116
kyle-sexton wants to merge 5 commits into
Conversation
An unmapped changed file sent test-bash to the full shell corpus and ran no Python suite, so every test_*.py the selector picked in that pull request went unrun, and the whole-tree runs (schedule, dispatch, a push with no base) never ran pytest at all. #6059 widened the unmapped set by dropping the Python import rule, so this now hits more pull requests than before. - The UNMAPPED branch keeps the full shell corpus and also runs this leg's slice of the selector's --unmapped-corpus listing that is not a shell suite: the selection's Python and Node suites, plus every Python suite when the unmapped file is a .py and every Node suite when it is Node. - The whole tree runs every Python suite, each leg every legs-th one. - Python suites run one pytest process each: suites in different directories import same-named helpers (from conftest import ...), so one process broke collection once suites from two such directories ran together. - Eval fixtures (*/evals/fixtures/*) are data the skill evals read and are not run; two of them error under pytest by design. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Never merged: this commit only makes the selector report an unmapped .py so the test-bash fallback branch runs on a pull request. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Claude finished @kyle-sexton's task in 1m 28s —— View job Security review complete
No security issues found. This PR is a stated probe (labeled
No |
|
Claude finished @kyle-sexton's task in 6m 38s —— View job CI code review
SummaryReviewed the two changed files against
One finding posted inline at ci.yml#L2327-L2329:
This PR is labeled |
|
claude-security-review has reviewed this pull request through 0f244c3; a later push is reviewed from there. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| # Same fixed pair the whole-tree path runs once on leg 3 (#3703). | ||
| if [ "$LEG" = $((3 % LEGS)) ]; then | ||
| scripts/run-outside-node-suites.sh | ||
| scripts/run-outside-node-suites.sh || status=1 |
There was a problem hiding this comment.
This "fixed pair" run-outside-node-suites.sh call and the run_delegated call a few lines below it (line 2340, which internally runs scripts/run-outside-node-suites.sh --paths ... at line 2288) can both select the same package on this leg.
When the UNMAPPED file's lang_family is node, affected-tests.sh --unmapped-corpus widens the listing to "every Node suite" (any *.test.js/*.test.mjs), which includes the fixed pair's own suites — confirmed, both contain matching files:
plugins/knowledge/vendor/repo-analysis/repo-analysis.test.jsplugins/knowledge/vendor/video-digestion/**/*.test.js(6 files)
So on the leg where LEG = 3 % LEGS, an UNMAPPED Node-family file causes npm test for plugins/knowledge/vendor/repo-analysis and video-digestion to run twice in the same step: once unconditionally here, and again via run_delegated's --paths call once package_of matches the widened listing's entries back to the same packages. That directly contradicts the comment on line 2327 ("runs once on leg 3") for this fallback path, and wastes a full npm test pass (and risks a flaky second run if either suite isn't safely re-runnable back-to-back in one job).
Repro: a PR whose only changed file is an unmapped .js/.mjs file outside the suite-mapped set, on the leg where LEG = 3 % LEGS.
Fix direction: skip the unconditional call here when the widened listing will already cover the fixed pair (e.g. only run the "same fixed pair" call when corpus_used's language isn't node, or dedupe the fixed-pair packages out of outside-node-paths.txt before calling run_delegated).
|
claude-review has reviewed this pull request through 0f244c3; a later push is reviewed from there. |
A whole-tree dispatch run (37141114979) ran the Python corpus on every leg for the first time: legs 0-2 passed in 7 to 7.5 minutes, and leg 3, which also runs the outside-Node packages and the Node sub-projects, was cancelled at the 11-minute limit in its Node steps after its Python share (the planning surface server suite alone took 202 s) had passed. 16 minutes is the ceiling for a job here. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
xargs -t prints every pytest command before it runs, so a leg's log names each Python suite it ran, not only pytest's count line. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…nto probe/unmapped-python-ci
|
Claude finished @kyle-sexton's task in 1m 1s —— View job Reviewing incremental changes to
|
|
Claude finished @kyle-sexton's task in 52s —— View job Security review complete
No security issues found. This is an incremental review: only the diff in
No new trust boundary, no widened permissions, no instruction-surface changes in this increment (no rules/CLAUDE.md/AGENTS.md/hook files are touched here). The rest of this PR's files ( This PR is labeled |
|
Probe done: runs 37141180493 and 37141969828 show test-bash's UNMAPPED fallback running the full shell corpus plus every Python corpus suite once across the four legs (139), all green. Closing unmerged. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1beb9ab249
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| git ls-files | awk -v leg="$LEG" -v legs="$LEGS" '{ b = $0; sub(/.*\//, "", b) } | ||
| b ~ /^test_.*\.py$/ && n++ % legs == leg' >"$RUNNER_TEMP/python-suites.txt" | ||
| pytest_each "$RUNNER_TEMP/python-suites.txt" || status=1 |
There was a problem hiding this comment.
On schedule/dispatch runs and pushes without a usable base, this adds roughly 34 sequential pytest processes to every matrix leg after the full shell corpus. In the exact four-way partition used here, leg 3's Python slice alone took about 540 seconds locally, while test-bash still has an 11-minute timeout and the workflow documents the pre-change job maximum as 593 seconds; checkout/toolchain setup, shell tests, and the later Node steps therefore leave no viable timeout margin. Split or parallelize this workload (or recompute the timeout) so whole-tree runs do not terminate before completing.
Useful? React with 👍 / 👎.
1 participant
No related issue: CI probe for #6115, closed unmerged once its test-bash run is read.
Summary
One commit on top of #6115 that adds a comment to plugins/harness-ops/lib/plugin_cache_versions.py, a file the selector reports UNMAPPED, so the test-bash fallback branch of #6115 runs on a pull request.
Fix
None: a probe. check-plugins is expected to fail (no version bump); only test-bash is read.
Verification
test-bash should run the full shell corpus plus its leg's slice of the Python corpus, one pytest process per suite.
Related
🤖 Generated with Claude Code