From 14cd827cb7a06027c67c69a58e0f516e406234a0 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 04:31:41 +0000 Subject: [PATCH 1/2] Balance coverage shards on measured test time The two `coverage` shards used the count split: shard 1 gets the unit tests plus a third of the integration targets, shard 2 the rest. On the last 8 green CI runs shard 2 took 4.33-6.23 min against shard 1's 3.07-3.40, and its test step 236-351 s against 160-179 s. Shard 2 is the last job before `ci-ok` in every PR and merge-queue run. Coverage shards now default to the checked-in runtime sample (scripts/ci-test-durations.json) that the Windows `test` shards already use. CI_TEST_TIMINGS still overrides it. Every target still runs in exactly one shard. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_011f9aePpyeqhTUQcsKL5coJ --- scripts/ci-test-shard.py | 15 +++++++++++---- scripts/tests/test_ci_test_shard.py | 16 ++++++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/scripts/ci-test-shard.py b/scripts/ci-test-shard.py index 079861c6b..d0c5d0510 100644 --- a/scripts/ci-test-shard.py +++ b/scripts/ci-test-shard.py @@ -12,12 +12,17 @@ Windows debug CI sets CI_TEST_TIMINGS to the checked-in runtime sample. Those shards balance measured test time plus linking work, with shard 1's -unit tests reserved up front. Other runners retain the count-based split. +unit tests reserved up front. The Linux `coverage` shards use the same +sample by default (see CI_TEST_COVERAGE below). Other runners retain the +count-based split. CI_TEST_COVERAGE=1 runs the shard under cargo-llvm-cov for ci.yml's `coverage` job: `cargo llvm-cov --no-report` in place of `cargo test`, and no doctests, which cargo-llvm-cov leaves out without `--doctests`. A later -`cargo llvm-cov report` exports the shard's profiles. +`cargo llvm-cov report` exports the shard's profiles. Unless CI_TEST_TIMINGS +is set, coverage shards balance on scripts/ci-test-durations.json: the count +split left shard 2 running ~80 s longer than shard 1 (246 s vs 167 s of +tests), and it set the end of every CI run. Extra arguments after `SHARD COUNT` go to every `cargo` invocation. Each invocation runs with `--no-fail-fast`; the exit status is non-zero if @@ -33,6 +38,8 @@ import subprocess import sys +DEFAULT_COVERAGE_TIMINGS = Path(__file__).resolve().parent / "ci-test-durations.json" + # Shard 1's share of the integration targets, relative to every other # shard's 1.0 (it also carries the unit tests and doctests). FIRST_SHARD_WEIGHT = 0.5 @@ -99,9 +106,9 @@ def main(argv): ["cargo", "metadata", "--no-deps", "--format-version", "1"], check=True, capture_output=True, text=True).stdout) names = integration_targets(metadata) - timing_path = os.environ.get("CI_TEST_TIMINGS") - timings = json.loads(Path(timing_path).read_text(encoding="utf-8")) if timing_path else None coverage = os.environ.get("CI_TEST_COVERAGE") == "1" + timing_path = os.environ.get("CI_TEST_TIMINGS") or (DEFAULT_COVERAGE_TIMINGS if coverage else None) + timings = json.loads(Path(timing_path).read_text(encoding="utf-8")) if timing_path else None print(f"ci-test-shard: shard {shard}/{count}: " f"{len(partition(names, count, timings)[shard - 1])} of {len(names)} integration targets", flush=True) status = 0 diff --git a/scripts/tests/test_ci_test_shard.py b/scripts/tests/test_ci_test_shard.py index 6ffd95ffa..2a7b9a493 100644 --- a/scripts/tests/test_ci_test_shard.py +++ b/scripts/tests/test_ci_test_shard.py @@ -96,6 +96,22 @@ def test_coverage_mode_comes_from_the_environment(self): self.assertEqual(run.call_count, 2) self.assertEqual(run.call_args_list[-1].args[0][:3], ["cargo", "llvm-cov", "--no-report"]) + def test_coverage_shards_default_to_the_timing_sample(self): + names = ["e2e_heavy", "e2e_light", "e2e_mid", "unit_only"] + metadata = {"workspace_members": ["a"], "packages": [ + {"id": "a", "targets": [{"name": n, "kind": ["test"]} for n in names]}]} + timings = json.loads(shard.DEFAULT_COVERAGE_TIMINGS.read_text()) + env = {k: v for k, v in shard.os.environ.items() if k != "CI_TEST_TIMINGS"} + for coverage, expected in (("1", shard.partition(names, 2, timings)), ("", shard.partition(names, 2))): + with self.subTest(coverage=coverage), \ + patch.dict(shard.os.environ, {**env, "CI_TEST_COVERAGE": coverage}, clear=True), \ + patch.object(shard.subprocess, "run", side_effect=[ + subprocess.CompletedProcess([], 0, stdout=json.dumps(metadata)), + subprocess.CompletedProcess([], 0), + ]) as run: + self.assertEqual(shard.main(["2", "2"]), 0) + self.assertCountEqual(selected([c.args[0] for c in run.call_args_list[1:]]), expected[1]) + def test_negative_bad_shard(self): with self.assertRaises(ValueError): shard.invocations(3, 2, self.NAMES) From 43902c2441df5b44b0b2fff5ab909e495220ad49 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 04:33:30 +0000 Subject: [PATCH 2/2] Let an empty CI_TEST_TIMINGS pick the count split Matches ci.yml's existing `CI_TEST_TIMINGS: ... || ''` pattern, where the empty string means the count split. The test now uses the sampled target names and checks both shards in each mode. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_011f9aePpyeqhTUQcsKL5coJ --- scripts/ci-test-shard.py | 10 +++++----- scripts/tests/test_ci_test_shard.py | 28 +++++++++++++++++----------- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/scripts/ci-test-shard.py b/scripts/ci-test-shard.py index d0c5d0510..0142bce5a 100644 --- a/scripts/ci-test-shard.py +++ b/scripts/ci-test-shard.py @@ -19,10 +19,10 @@ CI_TEST_COVERAGE=1 runs the shard under cargo-llvm-cov for ci.yml's `coverage` job: `cargo llvm-cov --no-report` in place of `cargo test`, and no doctests, which cargo-llvm-cov leaves out without `--doctests`. A later -`cargo llvm-cov report` exports the shard's profiles. Unless CI_TEST_TIMINGS -is set, coverage shards balance on scripts/ci-test-durations.json: the count -split left shard 2 running ~80 s longer than shard 1 (246 s vs 167 s of -tests), and it set the end of every CI run. +`cargo llvm-cov report` exports the shard's profiles. Coverage shards +balance on scripts/ci-test-durations.json unless CI_TEST_TIMINGS is set (an +empty value selects the count split): under the count split shard 2 ran well +past shard 1 and set the end of every CI run. Extra arguments after `SHARD COUNT` go to every `cargo` invocation. Each invocation runs with `--no-fail-fast`; the exit status is non-zero if @@ -107,7 +107,7 @@ def main(argv): check=True, capture_output=True, text=True).stdout) names = integration_targets(metadata) coverage = os.environ.get("CI_TEST_COVERAGE") == "1" - timing_path = os.environ.get("CI_TEST_TIMINGS") or (DEFAULT_COVERAGE_TIMINGS if coverage else None) + timing_path = os.environ.get("CI_TEST_TIMINGS", DEFAULT_COVERAGE_TIMINGS if coverage else None) timings = json.loads(Path(timing_path).read_text(encoding="utf-8")) if timing_path else None print(f"ci-test-shard: shard {shard}/{count}: " f"{len(partition(names, count, timings)[shard - 1])} of {len(names)} integration targets", flush=True) diff --git a/scripts/tests/test_ci_test_shard.py b/scripts/tests/test_ci_test_shard.py index 2a7b9a493..830d1f04f 100644 --- a/scripts/tests/test_ci_test_shard.py +++ b/scripts/tests/test_ci_test_shard.py @@ -97,20 +97,26 @@ def test_coverage_mode_comes_from_the_environment(self): self.assertEqual(run.call_args_list[-1].args[0][:3], ["cargo", "llvm-cov", "--no-report"]) def test_coverage_shards_default_to_the_timing_sample(self): - names = ["e2e_heavy", "e2e_light", "e2e_mid", "unit_only"] + timings = json.loads(shard.DEFAULT_COVERAGE_TIMINGS.read_text()) + names = sorted(timings["targets"]) + timed, counted = shard.partition(names, 2, timings), shard.partition(names, 2) + self.assertNotEqual(timed, counted) metadata = {"workspace_members": ["a"], "packages": [ {"id": "a", "targets": [{"name": n, "kind": ["test"]} for n in names]}]} - timings = json.loads(shard.DEFAULT_COVERAGE_TIMINGS.read_text()) env = {k: v for k, v in shard.os.environ.items() if k != "CI_TEST_TIMINGS"} - for coverage, expected in (("1", shard.partition(names, 2, timings)), ("", shard.partition(names, 2))): - with self.subTest(coverage=coverage), \ - patch.dict(shard.os.environ, {**env, "CI_TEST_COVERAGE": coverage}, clear=True), \ - patch.object(shard.subprocess, "run", side_effect=[ - subprocess.CompletedProcess([], 0, stdout=json.dumps(metadata)), - subprocess.CompletedProcess([], 0), - ]) as run: - self.assertEqual(shard.main(["2", "2"]), 0) - self.assertCountEqual(selected([c.args[0] for c in run.call_args_list[1:]]), expected[1]) + cases = (({"CI_TEST_COVERAGE": "1"}, timed), + ({"CI_TEST_COVERAGE": "1", "CI_TEST_TIMINGS": ""}, counted), + ({}, counted)) + for extra, expected in cases: + for k in (1, 2): + with self.subTest(env=extra, shard=k), \ + patch.dict(shard.os.environ, {**env, **extra}, clear=True), \ + patch.object(shard.subprocess, "run", side_effect=[ + subprocess.CompletedProcess([], 0, stdout=json.dumps(metadata)), + ] + [subprocess.CompletedProcess([], 0)] * 2) as run: + self.assertEqual(shard.main([str(k), "2"]), 0) + self.assertCountEqual(selected([c.args[0] for c in run.call_args_list[1:]]), + expected[k - 1]) def test_negative_bad_shard(self): with self.assertRaises(ValueError):