Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 66 additions & 33 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2029,7 +2029,10 @@ jobs:
# The contract-only gate, verbatim, and the draft gate (see `ci-status`).
if: ${{ !(github.event.pull_request.head.repo.full_name == github.repository && (contains(fromJSON('["labeled","unlabeled"]'), github.event.action) || (github.event.action == 'edited' && !github.event.changes.base))) && github.event.pull_request.draft != true }}
runs-on: ubuntu-24.04
timeout-minutes: 11
# A whole-tree leg runs its share of the shell corpus and then of the
# Python corpus, whose planning surface server suite alone takes about
# 200 s; the leg that also runs the Node sub-projects ran past 11 minutes.
timeout-minutes: 16
# ONE TO FOUR LEGS, SIZED FROM THE SELECTION. The job key stays one entry in
# `ci-status.needs` at any size; the contract suites are partitioned across
# up to four runners instead of executed by one: the affected selection on a
Expand Down Expand Up @@ -2215,14 +2218,18 @@ jobs:
# 3 every shell suite it selected ran, but it also selected suites in
# other ecosystems whose runner it will not guess. Those suites are
# RUN HERE rather than waved through: the steps further down this
# job cover four Node sub-projects and one Python module, which is
# 2 of this tree's 40 Python suites, so treating exit 3 as success
# would report a pass over suites that never executed. That is the
# precise thing the selector's own header refuses to do.
# job cover four Node sub-projects and one Python module, so
# treating exit 3 as success would report a pass over suites that
# never executed. That is the precise thing the selector's own
# header refuses to do.
# 1 an unmapped changed file OR a failing suite. Only the first is
# recoverable, and it announces itself with UNMAPPED: on stderr:
# nothing here knows what covers that file, so fall back to the full
# corpus and record the fallback so its rate is countable per run.
# shell corpus and record the fallback so its rate is countable per
# run. The shell corpus holds no Python or Node suite, so those run
# as on exit 3, from this leg's slice of the `--unmapped-corpus`
# listing: the selection's own, plus every Python suite when the
# unmapped file is a .py and every Node suite when it is Node.
# * anything else fails.
#
# Three suites at a time, not four. The runner has 4 vCPUs and the suites
Expand All @@ -2243,10 +2250,16 @@ jobs:
# instead, the matrix result would be `skipped` and `ci-status`, which is
# fail-closed on `skipped`, would red the pull request.
#
# The whole tree, and the UNMAPPED fallback, run the full corpus through
# run-plugin-tests.sh with the same `--shard`, which partitions its
# discovered suites the same way, so the four legs run it once between
# them.
# The whole tree, and the UNMAPPED fallback, run the full shell corpus
# through run-plugin-tests.sh with the same `--shard`, which partitions
# its discovered suites the same way, so the four legs run it once
# between them. The whole tree also runs every Python suite, each leg
# every legs-th one.
#
# ONE PYTEST PROCESS PER PYTHON SUITE, under the pinned pytest, which
# collects unittest.TestCase suites as well as pytest-style ones. Suites
# in different directories import same-named helpers (`from conftest
# import ...`), and a shared process hands each the first one loaded.
- name: Run plugin contract tests
if: needs.scope.outputs.run_tests == 'true'
env:
Expand All @@ -2261,14 +2274,36 @@ jobs:
# The code-tidying eval fixtures fail on a missing tree-sitter here instead of skipping green.
CODE_TIDYING_REQUIRE_TREE_SITTER: '1'
run: |
# Eval fixtures are data the skill evals read, not suites.
pytest_each() {
awk '!/\/evals\/fixtures\//' "$1" | tr '\n' '\0' |
xargs -0 -r -t -n1 python -m pytest -q -o tmp_path_retention_policy=none --
}
# A listing's suites the selector does not run itself: Python here,
# Node through each package's own npm test. Pester is the
# test-windows lane. #3703.
run_delegated() {
local rc=0
grep -E '(^|/)test_[^/]*\.py$' "$1" >"$RUNNER_TEMP/python-suites.txt" || true
grep -Ev '(^$|\.test\.sh$|\.py$)' "$1" >"$RUNNER_TEMP/outside-node-paths.txt" || true
pytest_each "$RUNNER_TEMP/python-suites.txt" || rc=1
if [ -s "$RUNNER_TEMP/outside-node-paths.txt" ]; then
scripts/run-outside-node-suites.sh --paths "$RUNNER_TEMP/outside-node-paths.txt" || rc=1
fi
return "$rc"
}
if [ -z "$DIFF_BASE" ]; then
scripts/run-plugin-tests.sh --jobs 3 --shard "$LEG/$LEGS"
status=0
scripts/run-plugin-tests.sh --jobs 3 --shard "$LEG/$LEGS" || status=1
# The shell corpus is sharded above. The outside-Node packages are
# a fixed pair, so one leg runs them once on the whole-tree path (#3703).
if [ "$LEG" = $((3 % LEGS)) ]; then
scripts/run-outside-node-suites.sh
scripts/run-outside-node-suites.sh || status=1
fi
exit 0
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
Comment on lines +2300 to +2302

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep the full Python corpus within the job timeout

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 👍 / 👎.

exit "$status"
fi
err="$RUNNER_TEMP/affected-tests.err"
set +e
Expand All @@ -2281,34 +2316,32 @@ jobs:
;;
3)
# The selector prints the suites it declined to run under a
# "NOT RUN:" heading, one indented path each. Run the Python ones
# with the pinned pytest: it collects unittest.TestCase suites as
# well as pytest-style ones, and each of these files puts its own
# module directory on sys.path, so no per-suite runner has to be
# guessed. Node suites outside the four sub-projects run through
# each package's own npm test. Pester is the test-windows lane.
# Eval fixtures are excluded. #3703.
notrun=$(awk '/^NOT RUN:/{f=1} f && /^ - /{sub(/^ - /,""); print}' "$err")
py=$(printf '%s\n' "$notrun" | grep -E '\.py$' || true)
rest=$(printf '%s\n' "$notrun" | grep -Ev '(^$|\.py$)' || true)
if [ -n "$py" ]; then
printf '%s\n' "$py" | tr '\n' '\0' | xargs -0 python -m pytest -q -o tmp_path_retention_policy=none --
fi
if [ -n "$rest" ]; then
printf '%s\n' "$rest" >"$RUNNER_TEMP/outside-node-paths.txt"
scripts/run-outside-node-suites.sh --paths "$RUNNER_TEMP/outside-node-paths.txt"
fi
# "NOT RUN:" heading, one indented path each.
awk '/^NOT RUN:/{f=1} f && /^ - /{sub(/^ - /,""); print}' "$err" >"$RUNNER_TEMP/delegated.txt"
run_delegated "$RUNNER_TEMP/delegated.txt"
;;
1)
if grep -q '^UNMAPPED:' "$err"; then
summary=$(grep -m1 '^UNMAPPED:' "$err")
echo "::warning::$summary Falling back to the full contract corpus."
echo "::warning::$summary Falling back to the full shell corpus."
printf 'affected-tests fallback: %s\n' "$summary" >> "$GITHUB_STEP_SUMMARY"
scripts/run-plugin-tests.sh --jobs 3 --shard "$LEG/$LEGS"
status=0
scripts/run-plugin-tests.sh --jobs 3 --shard "$LEG/$LEGS" || status=1
# 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
Comment on lines 2330 to +2332

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.js
  • plugins/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).

fi
listed=0
scripts/affected-tests.sh --unmapped-corpus --shard "$LEG/$LEGS" --base "$DIFF_BASE" \
>"$RUNNER_TEMP/listing.txt" 2>"$RUNNER_TEMP/listing.err" || listed=$?
if [ "$listed" != 0 ] && [ "$listed" != 4 ]; then
cat "$RUNNER_TEMP/listing.err" >&2
echo "::error::affected-tests.sh --unmapped-corpus exited $listed, so the Python and Node suites to run are unknown."
exit 1
fi
grep -v '\.test\.sh$' "$RUNNER_TEMP/listing.txt" >"$RUNNER_TEMP/delegated.txt" || true
run_delegated "$RUNNER_TEMP/delegated.txt" || status=1
exit "$status"
else
exit 1
fi
Expand Down
1 change: 1 addition & 0 deletions plugins/harness-ops/lib/plugin_cache_versions.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,3 +108,4 @@ def orphan_marker(read: Reader, version_rel: str, now: float) -> dict[str, Any]
}
except (OSError, ValueError, OverflowError):
return None
# CI probe: an unmapped .py must run the Python corpus. Never merged.
Loading