Repository navigation
ci(mcpx): deal e2e shards by measured time and document the stop rule (#1994) - #5481
Conversation
… rule down (#1994) init now runs pkgs/mcpx/scripts/ci/e2e-shard-packer.py: it discovers every func Test in internal/e2e, weights each by the committed timing seed (pkgs/mcpx/scripts/ci/e2e-timings.json), gives unmeasured tests the seed median, and LPT-packs into N = clamp(ceil(T/target), 1, 6) shards. The job fails if the regexes do not cover every test exactly once. Each e2e leg runs go test -json, asserts the shard reported the assigned test count (top-level only), prints its own test-time total, and uploads the stream as an artifact for the next seed refresh. The stop rule is arithmetic in the workflow header: split only while T/N is below a leg's fixed cost plus queue delay, citing run 41530429335 where six shards ran 2m50s-3m05s of tests behind a 38-45 min queue delay.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 21 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ab18a829f
ℹ️ 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".
| if passed + failed != int(os.environ.get("EXPECTED_COUNT", "0")): | ||
| raise SystemExit("shard test count differs from the init assignment") |
There was a problem hiding this comment.
In the default Linux workflow, the packer counts four tests from real_filesystem_test.go, real_memory_test.go, and real_time_test.go, although those files require the realsoftware build tag. The e2e command supplies no such tag (go help buildconstraint identifies -tags as the source of additional satisfied tags), so those tests never produce events. Consequently, this new assertion fails every shard containing one even when all executable tests pass. Discovery should respect the same build constraints as execution; enabling the intentionally separate real-software suite would not be an equivalent fix.
Useful? React with 👍 / 👎.
| if event.get("Action") not in ("pass", "fail"): | ||
| continue |
There was a problem hiding this comment.
When an assigned test legitimately calls t.Skip, this filter discards its terminal event, so the subsequent count assertion fails an otherwise successful shard. For example, TestACallScopedFlagReachesADaemonStartedWithoutIt explicitly skips when its search result contains fewer than two matching tools (settings_test.go:218–220). The local go doc cmd/test2json documentation defines skip as “the test was skipped or the package contained no tests”; count named top-level skip events toward assignment coverage while tracking them separately from passes and failures.
Useful? React with 👍 / 👎.
Closes the measured-time half of #1994 (the static 6-shard round-robin from #1998 stays as the fallback behaviour).
What changed
pkgs/mcpx/scripts/ci/e2e-shard-packer.py— discovers everyfunc Testininternal/e2e, weights tests from the committed seed, gives unmeasured tests the seed median (equal weights at the run-41530429335 rate when there is no seed), and LPT-packs intoN = clamp(ceil(T / target_leg_seconds), 1, 6)shards. It fails if the emitted regexes do not assign every test exactly once, so no shard can silently run zero tests.pkgs/mcpx/scripts/ci/e2e-timings.json— the committed seed (test → seconds), currently empty and ready to be filled from the artifacts below..github/workflows/mcpx-test.yml:go test -json, asserts it reported its assigned top-level test count, prints its own test-time total, and uploads the stream asmcpx-e2e-timings-shard-<n>for the next seed refresh.T/Nis below a leg’s fixed cost (setup ~35 s) plus observed queue delay, or while the e2e leg is the longest job. Run 41530429335 is cited: six shards at 2 m 50 s–3 m 05 s of test work behind a 38–45 min queue delay.Verification
python3 pkgs/mcpx/scripts/ci/e2e-shard-packer.pyon current main: 357 tests → 6 shards, every test assigned once, expected longest leg 180 s test time.cd pkgs/mcpx && go test ./internal/e2e -run "^TestJSONOutputIsValid$" -count=1→ ok (3.28 s), so a generated shard regex is runnable as printed.yq).CI on this PR is the remaining acceptance evidence: init’s coverage assertion, per-shard summaries, and the uploaded
-jsonartifacts.No test sources or product behaviour changed.
Open in [code]smith · Need help on this PR? Tag
@codesmith-botwith what you need.