From 8effb32a90bfa9b62fff70bf482c1c56c237267a Mon Sep 17 00:00:00 2001 From: joshavant <830519+joshavant@users.noreply.github.com> Date: Tue, 17 Mar 2026 21:53:35 -0500 Subject: [PATCH] CI: bound extension-fast smoke scope and enforce SLA --- .github/workflows/ci.yml | 58 +++++++++++++------- scripts/test-extension.mjs | 84 ++++++++++++++++++++++------- test/scripts/test-extension.test.ts | 15 +++++- 3 files changed, 117 insertions(+), 40 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6545b1798ad..680d1d556f6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -463,20 +463,23 @@ jobs: OPENCLAW_CHANGED_EXTENSION: ${{ matrix.extension }} run: | set -euo pipefail - plan_json="$(node scripts/test-extension.mjs "$OPENCLAW_CHANGED_EXTENSION" --allow-empty --dry-run --json)" + plan_json="$(node scripts/test-extension.mjs "$OPENCLAW_CHANGED_EXTENSION" --allow-empty --max-tests 1 --dry-run --json)" config="$(printf '%s' "$plan_json" | jq -r '.config')" roots="$(printf '%s' "$plan_json" | jq -r '.roots | join(", ")')" - tests="$(printf '%s' "$plan_json" | jq -r '.testFiles | length')" + tests="$(printf '%s' "$plan_json" | jq -r '(.selectedTestFiles // .testFiles | length)')" + total_tests="$(printf '%s' "$plan_json" | jq -r '.testFiles | length')" { echo "config=$config" echo "tests=$tests" + echo "total_tests=$total_tests" } >> "$GITHUB_OUTPUT" - echo "extension-fast plan: config=$config tests=$tests roots=$roots" + echo "extension-fast plan: config=$config selected-tests=$tests total-tests=$total_tests roots=$roots" { echo "### extension-fast (${OPENCLAW_CHANGED_EXTENSION}) plan" echo "- config: \`$config\`" echo "- roots: \`$roots\`" - echo "- test files: \`$tests\`" + echo "- selected test files: \`$tests\`" + echo "- total test files: \`$total_tests\`" } >> "$GITHUB_STEP_SUMMARY" - name: Run changed extension tests (timed) @@ -484,11 +487,12 @@ jobs: OPENCLAW_CHANGED_EXTENSION: ${{ matrix.extension }} PLAN_CONFIG: ${{ steps.plan.outputs.config }} PLAN_TESTS: ${{ steps.plan.outputs.tests }} + PLAN_TOTAL_TESTS: ${{ steps.plan.outputs.total_tests }} run: | set -euo pipefail test_start="$(date +%s)" set +e - pnpm test:extension "$OPENCLAW_CHANGED_EXTENSION" --allow-empty -- --pool=forks --maxWorkers=1 --bail=1 + pnpm test:extension "$OPENCLAW_CHANGED_EXTENSION" --allow-empty --max-tests 1 -- --pool=forks --maxWorkers=1 --bail=1 test_status=$? set -e test_end="$(date +%s)" @@ -503,6 +507,7 @@ jobs: --arg extension "$OPENCLAW_CHANGED_EXTENSION" \ --arg config "${PLAN_CONFIG:-unknown}" \ --argjson tests "${PLAN_TESTS:-0}" \ + --argjson totalTests "${PLAN_TOTAL_TESTS:-0}" \ --arg runId "$GITHUB_RUN_ID" \ --arg runAttempt "$GITHUB_RUN_ATTEMPT" \ --arg sha "$GITHUB_SHA" \ @@ -513,6 +518,7 @@ jobs: extension: $extension, config: $config, tests: $tests, + totalTests: $totalTests, runId: $runId, runAttempt: $runAttempt, sha: $sha, @@ -521,11 +527,12 @@ jobs: durationSeconds: $durationSeconds }' > "$metrics_json" - printf "extension\tconfig\ttests\tstatus\tduration_seconds\trun_id\trun_attempt\tsha\tref\n" > "$metrics_tsv" - printf "%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\n" \ + printf "extension\tconfig\ttests\ttotal_tests\tstatus\tduration_seconds\trun_id\trun_attempt\tsha\tref\n" > "$metrics_tsv" + printf "%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\n" \ "$OPENCLAW_CHANGED_EXTENSION" \ "${PLAN_CONFIG:-unknown}" \ "${PLAN_TESTS:-0}" \ + "${PLAN_TOTAL_TESTS:-0}" \ "$test_status" \ "$test_duration" \ "$GITHUB_RUN_ID" \ @@ -538,6 +545,8 @@ jobs: echo "### extension-fast (${OPENCLAW_CHANGED_EXTENSION}) runtime" echo "- duration: ${test_duration}s" echo "- exit code: ${test_status}" + echo "- selected tests: ${PLAN_TESTS:-0}" + echo "- total tests: ${PLAN_TOTAL_TESTS:-0}" echo "- metrics json: \`${metrics_json}\`" echo "- metrics tsv: \`${metrics_tsv}\`" } >> "$GITHUB_STEP_SUMMARY" @@ -580,8 +589,8 @@ jobs: const slaMaxSeconds = 1500; if (!existsSync(metricsDir)) { - console.log("::warning::No extension-fast timing artifacts found."); - process.exit(0); + console.error("::error::No extension-fast timing artifacts found."); + process.exit(1); } const rows = readdirSync(metricsDir) @@ -593,8 +602,8 @@ jobs: .filter((row) => typeof row.durationSeconds === "number"); if (rows.length === 0) { - console.log("::warning::No extension-fast timing JSON rows were found."); - process.exit(0); + console.error("::error::No extension-fast timing JSON rows were found."); + process.exit(1); } const durations = rows.map((row) => row.durationSeconds).toSorted((a, b) => a - b); @@ -615,12 +624,13 @@ jobs: writeFileSync(summaryPath, `${JSON.stringify(summary, null, 2)}\n`, "utf8"); const tsvLines = [ - "extension\tconfig\ttests\tstatus\tduration_seconds\trun_id\trun_attempt\tsha\tref", + "extension\tconfig\ttests\ttotal_tests\tstatus\tduration_seconds\trun_id\trun_attempt\tsha\tref", ...rows.map((row) => [ row.extension, row.config, row.tests, + row.totalTests ?? 0, row.status, row.durationSeconds, row.runId, @@ -640,26 +650,36 @@ jobs: `- p95: \`${p95}s\``, `- max: \`${max}s\``, "", - "| extension | config | tests | status | duration (s) |", - "| --- | --- | ---: | ---: | ---: |", + "| extension | config | selected tests | total tests | status | duration (s) |", + "| --- | --- | ---: | ---: | ---: | ---: |", ...rows .toSorted((a, b) => b.durationSeconds - a.durationSeconds) .map( (row) => - `| ${row.extension} | ${row.config} | ${row.tests} | ${row.status} | ${row.durationSeconds} |`, + `| ${row.extension} | ${row.config} | ${row.tests} | ${row.totalTests ?? 0} | ${row.status} | ${row.durationSeconds} |`, ), ].join("\n"); writeFileSync(process.env.GITHUB_STEP_SUMMARY, `${markdown}\n`, { flag: "a" }); + let failedSla = false; + if (failed > 0) { + console.error(`::error::extension-fast contains ${failed} failing lanes.`); + failedSla = true; + } if (p95 > slaP95Seconds) { - console.log( - `::warning::extension-fast p95 ${p95}s exceeds SLA target ${slaP95Seconds}s.`, + console.error( + `::error::extension-fast p95 ${p95}s exceeds SLA target ${slaP95Seconds}s.`, ); + failedSla = true; } if (max > slaMaxSeconds) { - console.log( - `::warning::extension-fast max ${max}s exceeds SLA ceiling ${slaMaxSeconds}s.`, + console.error( + `::error::extension-fast max ${max}s exceeds SLA ceiling ${slaMaxSeconds}s.`, ); + failedSla = true; + } + if (failedSla) { + process.exit(1); } EOF diff --git a/scripts/test-extension.mjs b/scripts/test-extension.mjs index aa8eda2761e..65f8722c9e1 100644 --- a/scripts/test-extension.mjs +++ b/scripts/test-extension.mjs @@ -182,6 +182,7 @@ function printUsage() { console.error("Usage: pnpm test:extension [vitest args...]"); console.error(" node scripts/test-extension.mjs [extension-name|path] [vitest args...]"); console.error(" node scripts/test-extension.mjs --allow-empty"); + console.error(" node scripts/test-extension.mjs --max-tests "); console.error(" node scripts/test-extension.mjs --list"); console.error( " node scripts/test-extension.mjs --list-changed --base [--head ]", @@ -190,20 +191,55 @@ function printUsage() { async function run() { const rawArgs = process.argv.slice(2); - const dryRun = rawArgs.includes("--dry-run"); - const json = rawArgs.includes("--json"); - const allowEmpty = rawArgs.includes("--allow-empty"); - const list = rawArgs.includes("--list"); - const listChanged = rawArgs.includes("--list-changed"); - const args = rawArgs.filter( - (arg) => - arg !== "--" && - arg !== "--dry-run" && - arg !== "--json" && - arg !== "--allow-empty" && - arg !== "--list" && - arg !== "--list-changed", - ); + let dryRun = false; + let json = false; + let allowEmpty = false; + let list = false; + let listChanged = false; + let maxTests; + const args = []; + + for (let index = 0; index < rawArgs.length; index += 1) { + const arg = rawArgs[index]; + + if (arg === "--") { + continue; + } + if (arg === "--dry-run") { + dryRun = true; + continue; + } + if (arg === "--json") { + json = true; + continue; + } + if (arg === "--allow-empty") { + allowEmpty = true; + continue; + } + if (arg === "--list") { + list = true; + continue; + } + if (arg === "--list-changed") { + listChanged = true; + continue; + } + if (arg === "--max-tests") { + const value = rawArgs[index + 1]; + const parsed = Number.parseInt(String(value ?? ""), 10); + if (!Number.isInteger(parsed) || parsed <= 0) { + printUsage(); + console.error(`Invalid --max-tests value "${value ?? ""}". Expected a positive integer.`); + process.exit(1); + } + maxTests = parsed; + index += 1; + continue; + } + + args.push(arg); + } let base = ""; let head = "HEAD"; @@ -274,14 +310,22 @@ async function run() { process.exit(1); } - if (plan.testFiles.length === 0 && !allowEmpty) { + const selectedTestFiles = + typeof maxTests === "number" ? plan.testFiles.slice(0, maxTests) : [...plan.testFiles]; + const outputPlan = { + ...plan, + maxTests, + selectedTestFiles, + }; + + if (selectedTestFiles.length === 0 && !allowEmpty) { console.error( `No tests found for ${plan.extensionDir}. Run "pnpm test:extension ${plan.extensionId} -- --dry-run" to inspect the resolved roots.`, ); process.exit(1); } - if (plan.testFiles.length === 0 && allowEmpty && !dryRun) { + if (selectedTestFiles.length === 0 && allowEmpty && !dryRun) { const message = `[test-extension] Skipping ${plan.extensionId}: no test files were found under ${plan.roots.join(", ")}`; console.warn(message); if (process.env.GITHUB_ACTIONS === "true") { @@ -292,23 +336,23 @@ async function run() { if (dryRun) { if (json) { - process.stdout.write(`${JSON.stringify(plan, null, 2)}\n`); + process.stdout.write(`${JSON.stringify(outputPlan, null, 2)}\n`); } else { console.log(`[test-extension] ${plan.extensionId}`); console.log(`config: ${plan.config}`); console.log(`roots: ${plan.roots.join(", ")}`); - console.log(`tests: ${plan.testFiles.length}`); + console.log(`tests: ${selectedTestFiles.length}`); } return; } console.log( - `[test-extension] Running ${plan.testFiles.length} test files for ${plan.extensionId} with ${plan.config}`, + `[test-extension] Running ${selectedTestFiles.length} test files for ${plan.extensionId} with ${plan.config}`, ); const child = spawn( pnpm, - ["exec", "vitest", "run", "--config", plan.config, ...plan.testFiles, ...passthroughArgs], + ["exec", "vitest", "run", "--config", plan.config, ...selectedTestFiles, ...passthroughArgs], { cwd: repoRoot, stdio: "inherit", diff --git a/test/scripts/test-extension.test.ts b/test/scripts/test-extension.test.ts index 8265f1c8661..594d45d7846 100644 --- a/test/scripts/test-extension.test.ts +++ b/test/scripts/test-extension.test.ts @@ -8,13 +8,17 @@ import { } from "../../scripts/test-extension.mjs"; const scriptPath = path.join(process.cwd(), "scripts", "test-extension.mjs"); +type DryRunPlan = ReturnType & { + maxTests?: number; + selectedTestFiles: string[]; +}; function readPlan(args: string[], cwd = process.cwd()) { const stdout = execFileSync(process.execPath, [scriptPath, ...args, "--dry-run", "--json"], { cwd, encoding: "utf8", }); - return JSON.parse(stdout) as ReturnType; + return JSON.parse(stdout) as DryRunPlan; } function findZeroTestExtensionId(): string | undefined { @@ -90,5 +94,14 @@ describe("scripts/test-extension.mjs", () => { const plan = readPlan([extensionId!, "--allow-empty"]); expect(plan.extensionId).toBe(extensionId); expect(plan.testFiles).toHaveLength(0); + expect(plan.selectedTestFiles).toHaveLength(0); + }); + + it("limits selected tests when --max-tests is passed", () => { + const plan = readPlan(["discord", "--allow-empty", "--max-tests", "1"]); + expect(plan.extensionId).toBe("discord"); + expect(plan.testFiles.length).toBeGreaterThan(1); + expect(plan.selectedTestFiles).toHaveLength(1); + expect(plan.maxTests).toBe(1); }); });