From 4e34fb3b51ff9024ceda7a30caf1e89bab9eecc2 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 22:31:42 +0000 Subject: [PATCH 1/2] Shard coverage across two runners coverage is the merge queue's critical path since #1697 split the Gradle e2e leg: it ends at 6.05 min in merge_group run 40032760038, while the next-slowest leg ends at 4.82. Its test step is ~330 s, of which the instrumented build is ~110 s (the push run's warm step), so ~220 s is test execution that a second runner can halve. Run it as two shards with the partitioner `test` and `test-release` already use. ci-test-shard.py gains a CI_TEST_COVERAGE mode that runs `cargo llvm-cov --no-report` and skips doctests, as the unsharded `cargo llvm-cov --workspace` did. Each shard uploads coverage-host-, which coverage-merge's `coverage-{host*,docker-*}` pattern already picks up. Shard 1 alone warms and saves the cache. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_0174QivDXDqjSDhKth66oLF4 --- .github/workflows/ci.yml | 39 +++++++++++++++++++-------- scripts/ci-test-shard.py | 18 +++++++++---- scripts/tests/test_ci_test_shard.py | 42 +++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 16 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 330b83311..38f06196e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -588,7 +588,17 @@ jobs: # numbers are report-only so contributors get visibility without flaky # CI when coverage shifts naturally with test edits. A failing TEST # fails the job: this is the Linux leg of `test`. + # + # Two shards (scripts/ci-test-shard.py, as `test` and `test-release`): + # the instrumented build takes ~110 s and running the tests ~220 s, so + # one leg was the queue's critical path (~6 min). Each shard compiles + # and runs only its targets and uploads its own LCOV; coverage-merge + # unions them. `ci-ok` waits for every shard. runs-on: ${{ vars.DISABLE_DEPOT_RUNNERS == 'true' && 'ubuntu-latest' || 'depot-ubuntu-24.04-16' }} + strategy: + fail-fast: false + matrix: + shard: [1, 2] timeout-minutes: 35 permissions: contents: read @@ -616,12 +626,14 @@ jobs: - name: Cache cargo # Swatinem/rust-cache, main-only saves: see the first `Cache cargo` # step in this file. + # The shards share one cache key; shard 1 alone writes it, from the + # warm step that builds every target. uses: Swatinem/rust-cache@c19371144df3bb44fab255c43d04cbc2ab54d1c4 # v2.9.1 with: - save-if: ${{ github.ref == 'refs/heads/main' }} + save-if: ${{ github.ref == 'refs/heads/main' && matrix.shard == 1 }} - name: Warm the coverage cache after merge-queue validation - if: needs.release-readiness.outputs.reuse == 'true' + if: needs.release-readiness.outputs.reuse == 'true' && matrix.shard == 1 run: | cargo llvm-cov show-env --sh > "$RUNNER_TEMP/coverage-env.sh" source "$RUNNER_TEMP/coverage-env.sh" @@ -672,6 +684,9 @@ jobs: env: SOCKET_PATCH_GO_E2E_REQUIRED: '1' SOCKET_PATCH_GO_E2E_VERSION: '1.24' + CI_TEST_COVERAGE: '1' + TEST_SHARD: ${{ matrix.shard }} + TEST_SHARD_COUNT: ${{ strategy.job-total }} # # The test output is also teed to a file so that a failing test's # panic becomes an annotation (scripts/ci-test-failures.py): in a @@ -680,27 +695,29 @@ jobs: # can't be diagnosed once the re-run has passed. run: | set -o pipefail - cargo llvm-cov --workspace --no-fail-fast \ - --no-report 2>&1 | tee "$RUNNER_TEMP/coverage-tests.log" || { + python3 scripts/ci-test-shard.py "$TEST_SHARD" "$TEST_SHARD_COUNT" \ + 2>&1 | tee "$RUNNER_TEMP/coverage-tests.log" || { status=$? python3 scripts/ci-test-failures.py "$RUNNER_TEMP/coverage-tests.log" || true exit "$status" } set +o pipefail - cargo llvm-cov report --lcov --output-path coverage-host.lcov - python3 scripts/ci-lcov-summary.py coverage-host.lcov | tee coverage-summary.txt + cargo llvm-cov report --lcov --output-path "coverage-host-$TEST_SHARD.lcov" + python3 scripts/ci-lcov-summary.py "coverage-host-$TEST_SHARD.lcov" | tee coverage-summary.txt - name: Publish coverage summary to job summary if: needs.release-readiness.outputs.reuse != 'true' # Render the per-file LCOV line/function/branch totals as a fenced # block in the GitHub Actions job summary so reviewers don't # need to crack open the artifact for a quick look. + env: + TEST_SHARD: ${{ matrix.shard }} run: | { - echo "## Host coverage summary" + echo "## Host coverage summary (shard $TEST_SHARD)" echo "" - echo "(In-process tests only. See coverage-merge for the" - echo "full picture including docker-e2e binary coverage.)" + echo "(This shard's in-process tests only. See coverage-merge" + echo "for the full picture including docker-e2e binary coverage.)" echo "" echo '```' cat coverage-summary.txt @@ -711,8 +728,8 @@ jobs: if: needs.release-readiness.outputs.reuse != 'true' uses: ./.github/actions/upload-artifact with: - name: coverage-host - path: coverage-host.lcov + name: coverage-host-${{ matrix.shard }} + path: coverage-host-${{ matrix.shard }}.lcov if-no-files-found: error retention-days: 30 diff --git a/scripts/ci-test-shard.py b/scripts/ci-test-shard.py index 28b7e02f0..ff6ec6c05 100644 --- a/scripts/ci-test-shard.py +++ b/scripts/ci-test-shard.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -"""Run one shard of ci.yml's `test` or `test-release` job: +"""Run one shard of ci.yml's `test`, `test-release` or `coverage` job: `cargo test --workspace` split over `COUNT` runners. Each runner compiles and executes only its assigned integration targets, shortening both the debug and release-mode jobs without changing their compilation profiles. @@ -14,6 +14,11 @@ 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. +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. + Extra arguments after `SHARD COUNT` go to every `cargo test` invocation. Each invocation runs with `--no-fail-fast`; the exit status is non-zero if any of them failed. @@ -68,16 +73,18 @@ def cost(name): return shards -def invocations(shard, count, names, extra=(), timings=None): +def invocations(shard, count, names, extra=(), timings=None, coverage=False): """The `cargo test` argument lists shard `shard` (1-based) runs.""" if not 1 <= shard <= count: raise ValueError(f"shard {shard} not in 1..{count}") - base = ["cargo", "test", "--workspace", "--no-fail-fast", *extra] + runner = ["llvm-cov", "--no-report"] if coverage else ["test"] + base = ["cargo", *runner, "--workspace", "--no-fail-fast", *extra] mine = partition(names, count, timings)[shard - 1] runs = [] if shard == 1: runs.append(base + ["--lib", "--bins"] + [a for n in mine for a in ("--test", n)]) - runs.append(base + ["--doc"]) + if not coverage: + runs.append(base + ["--doc"]) elif mine: runs.append(base + [a for n in mine for a in ("--test", n)]) return runs @@ -94,10 +101,11 @@ def main(argv): 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" 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 - for args in invocations(shard, count, names, argv[2:], timings): + for args in invocations(shard, count, names, argv[2:], timings, coverage): print("+ " + " ".join(args), flush=True) if subprocess.run(args).returncode != 0: status = 1 diff --git a/scripts/tests/test_ci_test_shard.py b/scripts/tests/test_ci_test_shard.py index 7ca653e16..7b24f33f0 100644 --- a/scripts/tests/test_ci_test_shard.py +++ b/scripts/tests/test_ci_test_shard.py @@ -71,6 +71,30 @@ def test_failed_unit_or_integration_run_still_runs_docs_and_fails_the_shard(self self.assertEqual(run.call_count, 3) self.assertIn("--doc", run.call_args_list[-1].args[0]) + def test_coverage_shards_run_llvm_cov_without_doctests(self): + # The unsharded `cargo llvm-cov --workspace` ran no doctests; the + # shards must neither add them nor drop a unit or integration target. + for count in (1, 2): + runs = [args for k in range(1, count + 1) + for args in shard.invocations(k, count, self.NAMES, coverage=True)] + self.assertTrue(all(a[:5] == ["cargo", "llvm-cov", "--no-report", "--workspace", "--no-fail-fast"] + for a in runs)) + self.assertFalse(any("--doc" in a for a in runs)) + self.assertEqual(sum("--lib" in a and "--bins" in a for a in runs), 1) + self.assertEqual(sorted(selected(runs)), self.NAMES) + + def test_coverage_mode_comes_from_the_environment(self): + metadata = {"workspace_members": ["a"], "packages": [ + {"id": "a", "targets": [{"name": "integration", "kind": ["test"]}]}]} + with patch.dict(shard.os.environ, {"CI_TEST_COVERAGE": "1"}), \ + patch.object(shard.subprocess, "run", side_effect=[ + subprocess.CompletedProcess([], 0, stdout=json.dumps(metadata)), + subprocess.CompletedProcess([], 0), + ]) as run: + self.assertEqual(shard.main(["1", "2"]), 0) + self.assertEqual(run.call_count, 2) + self.assertEqual(run.call_args_list[-1].args[0][:3], ["cargo", "llvm-cov", "--no-report"]) + def test_negative_bad_shard(self): with self.assertRaises(ValueError): shard.invocations(3, 2, self.NAMES) @@ -134,6 +158,24 @@ def test_all_release_shards_are_required_and_use_the_matrix_size(self): self.assertIn("if: always()", verdict) self.assertIn('if v["result"] not in ("success", "skipped")', verdict) + def test_all_coverage_shards_are_required_and_upload_distinct_lcov(self): + # coverage is the gating Linux test run: every shard must run under + # cargo-llvm-cov, gate ci-ok, and upload an LCOV coverage-merge picks + # up (its `coverage-{host*,docker-*}` pattern) without name clashes. + workflow = (ROOT / ".github/workflows/ci.yml").read_text(encoding="utf-8") + cov = workflow.split("\n coverage:\n")[1].split("\n docker-base:\n")[0] + count = json.loads(re.search(r"^ shard: (\[.*\])$", cov, re.M)[1]) + self.assertEqual(count, list(range(1, len(count) + 1))) + self.assertIn("fail-fast: false", cov) + self.assertIn("CI_TEST_COVERAGE: '1'", cov) + self.assertIn('python3 scripts/ci-test-shard.py "$TEST_SHARD" "$TEST_SHARD_COUNT"', cov) + self.assertIn("TEST_SHARD_COUNT: ${{ strategy.job-total }}", cov) + self.assertIn('--output-path "coverage-host-$TEST_SHARD.lcov"', cov) + self.assertIn("name: coverage-host-${{ matrix.shard }}", cov) + self.assertIn("pattern: coverage-{host*,docker-*}", workflow) + verdict = workflow.split("\n ci-ok:\n")[1] + self.assertIn("coverage", re.search(r"^ needs: \[(.*)\]$", verdict, re.M)[1].split(", ")) + @unittest.skipUnless(shutil.which("cargo"), "cargo not available") class CargoSelection(unittest.TestCase): From 0351fd2ea178459a0356d10bb3c7d585c80b143f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 22:44:41 +0000 Subject: [PATCH 2/2] Correct coverage cache comment and shard docs Review follow-ups: rust-cache stores dependencies only, so shard 1's save does not depend on which targets it built; the shard script's extra arguments now reach `cargo llvm-cov` too; the coverage-mode test no longer reads CI_TEST_TIMINGS from the caller's environment. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_0174QivDXDqjSDhKth66oLF4 --- .github/workflows/ci.yml | 4 ++-- scripts/ci-test-shard.py | 2 +- scripts/tests/test_ci_test_shard.py | 3 ++- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 38f06196e..bd976e031 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -626,8 +626,8 @@ jobs: - name: Cache cargo # Swatinem/rust-cache, main-only saves: see the first `Cache cargo` # step in this file. - # The shards share one cache key; shard 1 alone writes it, from the - # warm step that builds every target. + # The shards share one cache key and shard 1 alone writes it. + # rust-cache keeps only dependencies, which every shard builds. uses: Swatinem/rust-cache@c19371144df3bb44fab255c43d04cbc2ab54d1c4 # v2.9.1 with: save-if: ${{ github.ref == 'refs/heads/main' && matrix.shard == 1 }} diff --git a/scripts/ci-test-shard.py b/scripts/ci-test-shard.py index ff6ec6c05..079861c6b 100644 --- a/scripts/ci-test-shard.py +++ b/scripts/ci-test-shard.py @@ -19,7 +19,7 @@ no doctests, which cargo-llvm-cov leaves out without `--doctests`. A later `cargo llvm-cov report` exports the shard's profiles. -Extra arguments after `SHARD COUNT` go to every `cargo test` invocation. +Extra arguments after `SHARD COUNT` go to every `cargo` invocation. Each invocation runs with `--no-fail-fast`; the exit status is non-zero if any of them failed. diff --git a/scripts/tests/test_ci_test_shard.py b/scripts/tests/test_ci_test_shard.py index 7b24f33f0..6ffd95ffa 100644 --- a/scripts/tests/test_ci_test_shard.py +++ b/scripts/tests/test_ci_test_shard.py @@ -86,7 +86,8 @@ def test_coverage_shards_run_llvm_cov_without_doctests(self): def test_coverage_mode_comes_from_the_environment(self): metadata = {"workspace_members": ["a"], "packages": [ {"id": "a", "targets": [{"name": "integration", "kind": ["test"]}]}]} - with patch.dict(shard.os.environ, {"CI_TEST_COVERAGE": "1"}), \ + env = {k: v for k, v in shard.os.environ.items() if k != "CI_TEST_TIMINGS"} + with patch.dict(shard.os.environ, {**env, "CI_TEST_COVERAGE": "1"}, clear=True), \ patch.object(shard.subprocess, "run", side_effect=[ subprocess.CompletedProcess([], 0, stdout=json.dumps(metadata)), subprocess.CompletedProcess([], 0),