diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index a9ade0bdfae..83fed5a7a5d 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -248,6 +248,12 @@ jobs: env: SHARD_INDEX: ${{ matrix.shard }} SHARD_TOTAL: ${{ strategy.job-total }} + # A shard writes one report per process: coverage/lcov.info for the shared + # process, plus coverage/isolated// and coverage/tooling/ (see + # scripts/test-unit-ci.sh). Upload them all. + - name: List coverage reports + id: lcov + run: echo "files=$(find coverage -name lcov.info 2>/dev/null | LC_ALL=C sort | paste -sd, -)" >>"$GITHUB_OUTPUT" # Quality / CRAP merges every shard's report; keep them only briefly. A # failed shard's report is incomplete, so it uploads none and the summary # shows partial coverage. Informational: an upload error must not fail @@ -257,13 +263,13 @@ jobs: continue-on-error: true with: name: crap-lcov-unit-${{ matrix.shard }} - path: coverage/lcov.info + path: coverage/**/lcov.info if-no-files-found: ignore retention-days: 3 - uses: codecov/codecov-action@671740ac38dd9b0130fbe1cec585b89eea48d3de # v5.5.2 with: token: ${{ secrets.CODECOV_TOKEN }} - files: ./coverage/lcov.info + files: ${{ steps.lcov.outputs.files }} flags: unit-tests fail_ci_if_error: false diff --git a/scripts/crap/ci.test.ts b/scripts/crap/ci.test.ts index 2e1a6d1c33c..01b602712dd 100644 --- a/scripts/crap/ci.test.ts +++ b/scripts/crap/ci.test.ts @@ -1,5 +1,8 @@ import { expect, test } from "bun:test"; -import { annotation, coverageStatus } from "./ci"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { annotation, coverageStatus, lcovFiles } from "./ci"; import type { Finding } from "./policy"; test("coverage is partial when any expected shard report is missing", () => { @@ -37,3 +40,44 @@ test("annotations escape workflow-command syntax in paths and messages", () => { "::warning file=src/a%2Cb%3Ac.ts,line=3,endLine=9,title=CRAP new-function (violation)::100%25 new%0A::error::x" ); }); + +test("reads every nested lcov.info, but counts only artifacts with a top-level report", () => { + const dir = mkdtempSync(join(tmpdir(), "crap-ci-")); + try { + const write = (path: string) => { + mkdirSync(join(dir, path, ".."), { recursive: true }); + writeFileSync(join(dir, path), ""); + }; + // A unit shard: shared process, two isolated files and the tooling run. + write("crap-lcov-unit-1/lcov.info"); + write("crap-lcov-unit-1/isolated/12/lcov.info"); + write("crap-lcov-unit-1/isolated/3/lcov.info"); + write("crap-lcov-unit-1/tooling/lcov.info"); + write("crap-lcov-integration-1/lcov.info"); + // A shard whose shared process ran its last retry without coverage: its isolated + // reports still count as coverage, but the shard is not a complete report. + write("crap-lcov-unit-2/isolated/0/lcov.info"); + // Neither an artifact without a report nor a stray file is a shard report. + write("crap-lcov-unit-3/other.txt"); + write("stray.info"); + + const { artifacts, files } = lcovFiles(dir); + expect(artifacts).toEqual(["crap-lcov-integration-1", "crap-lcov-unit-1"]); + expect(files.map((file) => file.slice(dir.length + 1))).toEqual([ + "crap-lcov-integration-1/lcov.info", + "crap-lcov-unit-1/isolated/12/lcov.info", + "crap-lcov-unit-1/isolated/3/lcov.info", + "crap-lcov-unit-1/lcov.info", + "crap-lcov-unit-1/tooling/lcov.info", + "crap-lcov-unit-2/isolated/0/lcov.info", + ]); + expect(coverageStatus(artifacts, { unit: 2, integration: 1 })).toEqual({ + unit: 1, + integration: 1, + partial: true, + }); + expect(lcovFiles(join(dir, "missing"))).toEqual({ artifacts: [], files: [] }); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); diff --git a/scripts/crap/ci.ts b/scripts/crap/ci.ts index c3b620e96ec..77b5e46d193 100644 --- a/scripts/crap/ci.ts +++ b/scripts/crap/ci.ts @@ -7,8 +7,8 @@ // EXPECTED_UNIT and EXPECTED_INTEGRATION (shard counts), CRAP_DEBT_LABEL // ("true" when the PR has the crap-debt label), GITHUB_STEP_SUMMARY. import { spawnSync } from "node:child_process"; -import { appendFileSync, existsSync, readdirSync } from "node:fs"; -import { join } from "node:path"; +import { appendFileSync, existsSync, readdirSync, statSync } from "node:fs"; +import { basename, join } from "node:path"; import type { DebtUse, Finding } from "./policy"; export interface CheckReport { @@ -66,7 +66,7 @@ export function summary( "## Quality / CRAP (informational)", "", coverage, - "Unit reports cover the shared-process suites only (isolated suites run without coverage). No base coverage: the CRAP tax compares CC and only warns.", + "Unit reports cover every Bun unit suite except the isolated ones marked `no-coverage:` in scripts/test-unit-ci.sh (Bun coverage crashes them). A suite whose coverage run crashed twice passes on a retry without coverage, so it has no report: its Test / Unit job shows a warning. No base coverage: the CRAP tax compares CC and only warns.", "`.tsx` coverage comes from Bun unit tests only, because Jest does not collect `.tsx` coverage yet: a component only Jest UI tests render scores 0%.", "", ]; @@ -95,10 +95,29 @@ export function summary( return `${lines.join("\n")}\n`; } -function lcovFiles(dir: string): { artifacts: string[]; files: string[] } { +/** + * Lists every `lcov.info` under each artifact folder, at any depth: a unit shard + * uploads one report for its shared process plus one per isolated file and one for + * the tooling run (scripts/test-unit-ci.sh). An artifact counts as a shard report + * only with its top-level `lcov.info`: when the shared process passes only on its + * last retry, which runs without coverage, the shard's nested reports still count + * as coverage but the summary shows it as partial. + */ +export function lcovFiles(dir: string): { artifacts: string[]; files: string[] } { if (!existsSync(dir)) return { artifacts: [], files: [] }; - const artifacts = readdirSync(dir).filter((name) => existsSync(join(dir, name, "lcov.info"))); - return { artifacts, files: artifacts.map((name) => join(dir, name, "lcov.info")) }; + const artifacts: string[] = []; + const files: string[] = []; + for (const name of readdirSync(dir).sort()) { + const root = join(dir, name); + if (!statSync(root).isDirectory()) continue; + const found = readdirSync(root, { recursive: true, encoding: "utf8" }) + .filter((path) => basename(path) === "lcov.info") + .sort() + .map((path) => join(root, path)); + if (existsSync(join(root, "lcov.info"))) artifacts.push(name); + files.push(...found); + } + return { artifacts, files }; } function main(): void { diff --git a/scripts/test-unit-ci.sh b/scripts/test-unit-ci.sh index 5a0a227bb8c..0023de8f2ab 100755 --- a/scripts/test-unit-ci.sh +++ b/scripts/test-unit-ci.sh @@ -28,28 +28,34 @@ if ! [[ "$SHARD_INDEX" =~ ^[0-9]+$ && "$SHARD_TOTAL" =~ ^[0-9]+$ ]] \ exit 2 fi +# Each isolated file writes its own LCOV report (see the isolated loop below), so the +# CRAP gate scores its sources. A `no-coverage:` prefix marks a file that is isolated +# because Bun coverage crashes or breaks it: that file runs without coverage. The +# QuickJS group is marked as a whole: sandboxHostService and hookService joined it as +# QuickJS-heavy suites, and router.test.ts has sat under its coverage comment since the +# list began, with no other recorded reason. isolated_unit_tests=( # QuickJS-heavy tests can crash or lose runtime callbacks under Bun coverage # when sharing the monolithic process; keep them in fresh isolated processes. - src/node/services/workflows/WorkflowRunner.test.ts + no-coverage:src/node/services/workflows/WorkflowRunner.test.ts # The evaluate() lifecycle suites run a QuickJS workflow per case (~40 runtimes); # the shared process segfaulted Bun twice on the same head with them included. - src/node/services/workflows/WorkflowRunner.evaluate.test.ts - src/node/services/workflows/WorkflowService.evaluate.test.ts - src/node/services/ptc/quickjsRuntime.test.ts - src/node/services/tools/code_execution.test.ts + no-coverage:src/node/services/workflows/WorkflowRunner.evaluate.test.ts + no-coverage:src/node/services/workflows/WorkflowService.evaluate.test.ts + no-coverage:src/node/services/ptc/quickjsRuntime.test.ts + no-coverage:src/node/services/tools/code_execution.test.ts # This suite also creates QuickJS runtimes. Shared coverage can trap during startup. - src/node/services/tools/code_execution.integration.test.ts - src/node/services/sandbox/sandboxHostService.test.ts - src/node/services/agentPlugins/hookService.test.ts - src/node/orpc/router.test.ts + no-coverage:src/node/services/tools/code_execution.integration.test.ts + no-coverage:src/node/services/sandbox/sandboxHostService.test.ts + no-coverage:src/node/services/agentPlugins/hookService.test.ts + no-coverage:src/node/orpc/router.test.ts # In the shared monolithic process on CI runners this file can enter an # infinite 'Maximum update depth exceeded' render loop (timing/coverage # sensitive; also seen on main and sibling branches) that spews ~1GB of # warnings until the 15-minute job timeout. Passes reliably in isolation # and could not be reproduced locally even with the exact CI file order, # bun version, and coverage flags. - src/browser/components/CommandPalette/CommandPalette.prompt.test.tsx + no-coverage:src/browser/components/CommandPalette/CommandPalette.prompt.test.tsx # Its tooltip test asserts Radix portal content, and Radix tooltip module # state binds to the first document that mounted a TooltipProvider in the # process — any earlier test file rendering a tooltip (raw happy-dom swap @@ -71,7 +77,7 @@ isolated_unit_tests=( # coverage process Bun hit an allocator panic ("pas panic: deallocation did # fail", exit 133) as this file started on main run 35782976621, with zero # failing assertions; isolation gets it the signal-exit retry below. - src/node/services/mcpIconDecodeClient.test.ts + no-coverage:src/node/services/mcpIconDecodeClient.test.ts # Guards the DOM harness itself by deliberately poisoning process globals # (document/window set to undefined, a replaced baseline window), which would # perturb later suites in a shared process. @@ -80,6 +86,17 @@ isolated_unit_tests=( # Jest skips it. ./tests/bunEachTableGc.test.ts ) +# Strip the marker so every later use of isolated_unit_tests sees plain paths, and +# record it per index, so the coverage choice cannot drift from the list. +isolated_coverage=() +for i in "${!isolated_unit_tests[@]}"; do + if [[ "${isolated_unit_tests[$i]}" == no-coverage:* ]]; then + isolated_unit_tests[i]=${isolated_unit_tests[$i]#no-coverage:} + isolated_coverage[i]=0 + else + isolated_coverage[i]=1 + fi +done # One process per file rather than one shared isolated process. Sharing it still # segfaults Bun on the runner (exit 132, "Bun has crashed", zero failing assertions) @@ -153,12 +170,33 @@ bun_test_with_stall_watchdog() { return "$exit_code" } +# Root of every LCOV report this lane writes. scripts/test-unit-ci.test.ts runs this +# script from inside the tooling run, so it points the nested run elsewhere: a +# nested run on the default root would delete the outer run's reports. +UNIT_COVERAGE_DIR="${UNIT_COVERAGE_DIR:-coverage}" + +# Usage: bun_test_retrying_crashes