diff --git a/.github/workflows/codebase-growth-guardrails.yaml b/.github/workflows/codebase-growth-guardrails.yaml index c1ebb94edf3..c5c9f42d0af 100644 --- a/.github/workflows/codebase-growth-guardrails.yaml +++ b/.github/workflows/codebase-growth-guardrails.yaml @@ -12,6 +12,7 @@ on: types: [opened, reopened, synchronize, ready_for_review] permissions: + contents: read pull-requests: read jobs: @@ -105,3 +106,140 @@ jobs: in the top-level onboard entrypoint. EOF exit 1 + + - name: Require changed test files to stay within size budget + env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ github.event.pull_request.number }} + REPO: ${{ github.repository }} + BASE_SHA: ${{ github.event.pull_request.base.sha }} + HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + set -euo pipefail + + node <<'NODE' + const BUDGET_FILE = "ci/test-file-size-budget.json"; + const FALLBACK_BUDGET = '{"defaultMaxLines":1500,"legacyMaxLines":{}}'; + const TEST_FILE_RE = /^(test|src|nemoclaw\/src)\/.*\.(test|spec)\.(ts|js|mts|mjs|cts|cjs)$/; + const { BASE_SHA, GH_TOKEN, HEAD_REPO, HEAD_SHA, PR_NUMBER, REPO } = process.env; + const headers = { Authorization: `Bearer ${GH_TOKEN}`, "X-GitHub-Api-Version": "2022-11-28" }; + const violations = []; + + function countLines(text) { + return text === "" ? 0 : (text.match(/\r\n|\r|\n/g)?.length ?? 0) + (/(?:\r\n|\r|\n)$/.test(text) ? 0 : 1); + } + + function parseBudget(text, label) { + const budget = JSON.parse(text); + const legacyMaxLines = budget.legacyMaxLines ?? {}; + if (!Number.isInteger(budget.defaultMaxLines) || budget.defaultMaxLines <= 0) { + throw new Error(`${label} must define positive integer defaultMaxLines`); + } + if (typeof legacyMaxLines !== "object" || legacyMaxLines === null || Array.isArray(legacyMaxLines)) { + throw new Error(`${label} legacyMaxLines must be an object`); + } + for (const [file, maxLines] of Object.entries(legacyMaxLines)) { + if (!Number.isInteger(maxLines) || maxLines <= 0) { + throw new Error(`${label} has invalid legacy budget for ${file}: ${maxLines}`); + } + } + return { defaultMaxLines: budget.defaultMaxLines, legacyMaxLines }; + } + + async function getJson(url) { + const response = await fetch(url, { headers }); + if (!response.ok) throw new Error(`${url}: HTTP ${response.status}`); + return response.json(); + } + + async function getPullFiles() { + const files = []; + for (let page = 1; ; page += 1) { + const batch = await getJson(`https://api.github.com/repos/${REPO}/pulls/${PR_NUMBER}/files?per_page=100&page=${page}`); + files.push(...batch); + if (batch.length < 100) return files; + } + } + + async function getContent(repo, ref, file) { + const encodedPath = file.split("/").map(encodeURIComponent).join("/"); + const url = `https://api.github.com/repos/${repo}/contents/${encodedPath}?ref=${encodeURIComponent(ref)}`; + const response = await fetch(url, { headers }); + if (response.status === 404) return null; + if (!response.ok) throw new Error(`${url}: HTTP ${response.status}`); + const body = await response.json(); + if (body.type !== "file" || body.encoding !== "base64" || typeof body.content !== "string") { + throw new Error(`Could not decode file contents for ${file}`); + } + return Buffer.from(body.content.replace(/\s/g, ""), "base64").toString("utf8"); + } + + async function checkLegacyFile(file, maxLines, headBudget) { + const text = await getContent(HEAD_REPO, HEAD_SHA, file); + if (text === null) { + violations.push(`${file} has a legacy budget but no matching test file at the PR head`); + return; + } + const lines = countLines(text); + if (lines > maxLines) violations.push(`${file} has ${lines} line(s), above its legacy budget ${maxLines}`); + if (lines < maxLines) { + violations.push(`${file}: ${lines} line(s) < ${maxLines} legacy budget; lower the budget entry`); + } + } + + async function validateBudget(baseBudget, headBudget, baseWasFallback) { + if (baseWasFallback) return; + if (headBudget.defaultMaxLines > baseBudget.defaultMaxLines) { + violations.push(`defaultMaxLines increased from ${baseBudget.defaultMaxLines} to ${headBudget.defaultMaxLines}`); + } + for (const [file, baseMax] of Object.entries(baseBudget.legacyMaxLines)) { + const headMax = headBudget.legacyMaxLines[file]; + const text = headMax === undefined ? await getContent(HEAD_REPO, HEAD_SHA, file) : null; + if (headMax > baseMax) violations.push(`${file} legacy budget increased from ${baseMax} to ${headMax}`); + if (headMax === undefined && text !== null && countLines(text) > headBudget.defaultMaxLines) { + violations.push(`${file} removed its legacy budget while still exceeding defaultMaxLines`); + } + } + for (const [file, headMax] of Object.entries(headBudget.legacyMaxLines)) { + if (baseBudget.legacyMaxLines[file] === undefined && headMax > headBudget.defaultMaxLines) { + violations.push(`${file} adds a new legacy budget (${headMax}) above defaultMaxLines (${headBudget.defaultMaxLines})`); + } + await checkLegacyFile(file, headMax, headBudget); + } + } + + async function main() { + const files = await getPullFiles(); + const baseText = await getContent(REPO, BASE_SHA, BUDGET_FILE); + const baseWasFallback = baseText === null; + const budgetChanged = files.some(({ filename, previous_filename }) => filename === BUDGET_FILE || previous_filename === BUDGET_FILE); + const headText = budgetChanged ? await getContent(HEAD_REPO, HEAD_SHA, BUDGET_FILE) : baseText; + if (budgetChanged && headText === null) throw new Error(`${BUDGET_FILE} must remain present and parseable at the PR head`); + + const baseBudget = parseBudget(baseText ?? FALLBACK_BUDGET, "base budget"); + const headBudget = parseBudget(headText ?? FALLBACK_BUDGET, "head budget"); + const changedTests = files.filter(({ filename, status }) => status !== "removed" && TEST_FILE_RE.test(filename)); + + await validateBudget(baseBudget, headBudget, baseWasFallback); + for (const { filename } of changedTests) { + const text = await getContent(HEAD_REPO, HEAD_SHA, filename); + if (text === null) throw new Error(`Changed test file ${filename} was not found at the PR head`); + const lines = countLines(text); + const maxLines = headBudget.legacyMaxLines[filename] ?? headBudget.defaultMaxLines; + if (lines > maxLines) violations.push(`${filename}: ${lines} line(s) > ${maxLines}`); + } + + if (violations.length > 0) { + console.error("FAIL: test size budget policy would be weakened or exceeded."); + for (const violation of violations) console.error(`- ${violation}`); + process.exit(1); + } + console.log(`PASS: test size budget policy is monotonic and ${changedTests.length} changed test file(s) are within budget.`); + } + + main().catch((error) => { + console.error(error); + process.exit(1); + }); + NODE diff --git a/.github/workflows/pr.yaml b/.github/workflows/pr.yaml index 1d45a2c1595..b5c3853a295 100644 --- a/.github/workflows/pr.yaml +++ b/.github/workflows/pr.yaml @@ -112,11 +112,15 @@ jobs: --skip test-cli \ --skip test-plugin \ --skip source-shape-test-budget \ + --skip test-file-size-budget \ --skip test-skills-yaml - name: Run source-shape budget run: npm run source-shape:check + - name: Run test file size budget + run: npm run test-size:check + - name: Run skills YAML tests run: npx vitest run test/skills-frontmatter.test.ts diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 2d09b6cdd1a..b09bc8e2dc6 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,3 +1,6 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + # NemoClaw — prek hook configuration # prek: https://github.com/j178/prek — single binary, no Python required for the runner # Installed as an npm devDependency (@j178/prek) — available after `npm install`. @@ -304,6 +307,14 @@ repos: files: ^(test/|scripts/find-source-shape-tests\.ts$|ci/source-shape-test-budget\.json$) priority: 20 + - id: test-file-size-budget + name: Test file size budget + entry: npm run test-size:check + language: system + pass_filenames: false + files: ^(test/|src/.*\.(test|spec)\.(ts|js|mts|mjs|cts|cjs)$|nemoclaw/src/.*\.(test|spec)\.(ts|js|mts|mjs|cts|cjs)$|scripts/check-test-file-size-budget\.ts$|ci/test-file-size-budget\.json$) + priority: 20 + - id: test-skills-yaml name: Test (skills YAML) entry: npx vitest run test/skills-frontmatter.test.ts diff --git a/ci/test-file-size-budget.json b/ci/test-file-size-budget.json new file mode 100644 index 00000000000..236a84beffe --- /dev/null +++ b/ci/test-file-size-budget.json @@ -0,0 +1,18 @@ +{ + "$comment": "SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.\nSPDX-License-Identifier: Apache-2.0", + "defaultMaxLines": 1500, + "legacyMaxLines": { + "nemoclaw/src/commands/migration-state.test.ts": 1566, + "src/lib/inference/nim.test.ts": 2079, + "src/lib/onboard/preflight.test.ts": 1905, + "test/channels-add-preset.test.ts": 1915, + "test/generate-openclaw-config.test.ts": 2106, + "test/install-preflight.test.ts": 4397, + "test/nemoclaw-start.test.ts": 5319, + "test/onboard-messaging.test.ts": 2122, + "test/onboard-selection.test.ts": 7757, + "test/onboard.test.ts": 4887, + "test/policies.test.ts": 3147, + "test/sandbox-connect-inference.test.ts": 1577 + } +} diff --git a/package.json b/package.json index d5686f173ec..9179ec36a36 100644 --- a/package.json +++ b/package.json @@ -1,4 +1,5 @@ { + "$comment": "SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.\nSPDX-License-Identifier: Apache-2.0", "name": "nemoclaw", "version": "0.1.0", "description": "NemoClaw — run OpenClaw inside OpenShell with NVIDIA inference", @@ -35,6 +36,7 @@ "type-safety:hotspots": "tsx scripts/type-safety-hotspots.ts", "source-shape:scan": "tsx scripts/find-source-shape-tests.ts --metrics", "source-shape:check": "tsx scripts/find-source-shape-tests.ts --check", + "test-size:check": "tsx scripts/check-test-file-size-budget.ts", "bump:version": "tsx scripts/bump-version.ts", "release:plan": "tsx scripts/release-plan.ts", "release:cut": "bash scripts/release-cut-tag.sh", diff --git a/scripts/check-test-file-size-budget.ts b/scripts/check-test-file-size-budget.ts new file mode 100755 index 00000000000..932d197d8a4 --- /dev/null +++ b/scripts/check-test-file-size-budget.ts @@ -0,0 +1,225 @@ +#!/usr/bin/env -S npx tsx +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { existsSync, readdirSync, readFileSync, statSync } from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +export type TestFileSizeBudget = { + readonly defaultMaxLines: number; + readonly legacyMaxLines?: Readonly>; +}; + +export type TestFileSizeEntry = { + readonly file: string; + readonly lines: number; +}; + +export type TestFileSizeViolation = + | { + readonly kind: "oversized"; + readonly file: string; + readonly lines: number; + readonly maxLines: number; + readonly budgetKind: "default" | "legacy"; + } + | { + readonly kind: "legacy-ratchet"; + readonly file: string; + readonly lines: number; + readonly maxLines: number; + } + | { + readonly kind: "stale-legacy-budget"; + readonly file: string; + readonly maxLines: number; + }; + +const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); +const BUDGET_PATH = path.join(REPO_ROOT, "ci", "test-file-size-budget.json"); +const TEST_FILE_PATTERN = /\.(?:test|spec)\.(?:[cm]?[jt]s)$/; +const SCAN_ROOTS = ["test", "src", "nemoclaw/src"]; +const SKIP_DIRS = new Set([ + ".git", + ".venv", + "coverage", + "dist", + "docs/_build", + "nemoclaw/dist", + "nemoclaw/node_modules", + "node_modules", +]); + +function toRepoPath(absPath: string): string { + return path.relative(REPO_ROOT, absPath).split(path.sep).join("/"); +} + +function isSkipped(absPath: string): boolean { + const rel = toRepoPath(absPath); + return [...SKIP_DIRS].some((skipDir) => rel === skipDir || rel.startsWith(`${skipDir}/`)); +} + +function* walkFiles(dir: string): Generator { + if (!existsSync(dir) || isSkipped(dir)) return; + + for (const entry of readdirSync(dir)) { + const absPath = path.join(dir, entry); + if (isSkipped(absPath)) continue; + + const stats = statSync(absPath); + if (stats.isDirectory()) { + yield* walkFiles(absPath); + } else if (stats.isFile() && TEST_FILE_PATTERN.test(entry)) { + yield absPath; + } + } +} + +export function countLines(text: string): number { + if (text.length === 0) return 0; + const newlineCount = text.match(/\r\n|\r|\n/g)?.length ?? 0; + return newlineCount + (/(?:\r\n|\r|\n)$/.test(text) ? 0 : 1); +} + +export function collectTestFileSizes(roots = SCAN_ROOTS): TestFileSizeEntry[] { + return roots + .flatMap((root) => [...walkFiles(path.join(REPO_ROOT, root))]) + .map((absPath) => ({ + file: toRepoPath(absPath), + lines: countLines(readFileSync(absPath, "utf-8")), + })) + .sort((a, b) => a.file.localeCompare(b.file)); +} + +function assertPositiveInteger(value: unknown, label: string): number { + if (!Number.isInteger(value) || Number(value) <= 0) { + throw new Error(`${label} must be a positive integer`); + } + return Number(value); +} + +export function parseBudget(sourceText: string, filePath = BUDGET_PATH): TestFileSizeBudget { + const parsed = JSON.parse(sourceText) as { + readonly defaultMaxLines?: unknown; + readonly legacyMaxLines?: unknown; + }; + const defaultMaxLines = assertPositiveInteger( + parsed.defaultMaxLines, + `${filePath}: defaultMaxLines`, + ); + + if (parsed.legacyMaxLines !== undefined && !isRecord(parsed.legacyMaxLines)) { + throw new Error(`${filePath}: legacyMaxLines must be an object when present`); + } + + const legacyMaxLines: Record = {}; + for (const [legacyPath, value] of Object.entries(parsed.legacyMaxLines ?? {})) { + legacyMaxLines[legacyPath] = assertPositiveInteger( + value, + `${filePath}: legacyMaxLines.${legacyPath}`, + ); + } + + return { defaultMaxLines, legacyMaxLines }; +} + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +export function evaluateTestFileSizeBudget( + entries: readonly TestFileSizeEntry[], + budget: TestFileSizeBudget, +): TestFileSizeViolation[] { + const legacyMaxLines = budget.legacyMaxLines ?? {}; + const seenFiles = new Set(entries.map((entry) => entry.file)); + const violations: TestFileSizeViolation[] = []; + + for (const entry of entries) { + const legacyMax = legacyMaxLines[entry.file]; + const maxLines = legacyMax ?? budget.defaultMaxLines; + const budgetKind = legacyMax === undefined ? "default" : "legacy"; + + if (entry.lines > maxLines) { + violations.push({ + kind: "oversized", + file: entry.file, + lines: entry.lines, + maxLines, + budgetKind, + }); + } else if (legacyMax !== undefined && entry.lines < legacyMax) { + violations.push({ + kind: "legacy-ratchet", + file: entry.file, + lines: entry.lines, + maxLines: legacyMax, + }); + } + } + + for (const [legacyPath, maxLines] of Object.entries(legacyMaxLines)) { + if (!seenFiles.has(legacyPath)) { + violations.push({ kind: "stale-legacy-budget", file: legacyPath, maxLines }); + } + } + + return violations.sort((a, b) => a.file.localeCompare(b.file)); +} + +export function formatViolations( + violations: readonly TestFileSizeViolation[], + budgetPath = "ci/test-file-size-budget.json", +): string { + const lines = [ + "Test file size budget failed.", + "", + `Default test-file ceiling is configured in ${budgetPath}.`, + "Split oversized tests into focused files, or ratchet the legacy budget down after shrinking them.", + "", + ]; + + for (const violation of violations) { + if (violation.kind === "oversized") { + lines.push( + `- ${violation.file}: ${violation.lines} line(s) > ${violation.maxLines} ${violation.budgetKind} budget`, + ); + } else if (violation.kind === "legacy-ratchet") { + lines.push( + `- ${violation.file}: ${violation.lines} line(s) < ${violation.maxLines} legacy budget; lower the budget entry`, + ); + } else { + lines.push( + `- ${violation.file}: legacy budget entry (${violation.maxLines}) has no matching test file; remove it`, + ); + } + } + + return lines.join("\n"); +} + +function main(): void { + const budget = parseBudget(readFileSync(BUDGET_PATH, "utf-8"), BUDGET_PATH); + const entries = collectTestFileSizes(); + const violations = evaluateTestFileSizeBudget(entries, budget); + + if (violations.length > 0) { + console.error(formatViolations(violations)); + process.exitCode = 1; + return; + } + + const maxEntry = entries.reduce( + (max, entry) => (max === null || entry.lines > max.lines ? entry : max), + null, + ); + const maxText = maxEntry ? `${maxEntry.file} (${maxEntry.lines} lines)` : "no test files"; + console.log( + `Test file size budget passed: ${entries.length} files scanned; largest is ${maxText}.`, + ); +} + +if (fileURLToPath(import.meta.url) === path.resolve(process.argv[1] ?? "")) { + main(); +} diff --git a/test/pr-workflow-contract.test.ts b/test/pr-workflow-contract.test.ts index 63cbcc2749c..c7e76cc0064 100644 --- a/test/pr-workflow-contract.test.ts +++ b/test/pr-workflow-contract.test.ts @@ -13,6 +13,10 @@ type PullRequestWorkflow = { jobs: Record; }; +type CodebaseGrowthGuardrailsWorkflow = { + jobs: Record; +}; + function stepRuns(job: WorkflowJob): string[] { return (job.steps ?? []).flatMap((step) => (step.run ? [step.run] : [])); } @@ -116,6 +120,7 @@ describe("pull request workflow contract", () => { "test-cli", "test-plugin", "source-shape-test-budget", + "test-file-size-budget", "test-skills-yaml", ]) { expect(staticPrekRun).toContain(`--skip ${skippedHook}`); @@ -134,9 +139,28 @@ describe("pull request workflow contract", () => { expect(pluginTestRun).toContain("npx vitest run --project plugin"); expect(pluginTestRun).toContain("npx tsx scripts/check-coverage-ratchet.ts"); expect(staticRuns).toContain("npm run source-shape:check"); + expect(staticRuns).toContain("npm run test-size:check"); expect(staticRuns).toContain("npx vitest run test/skills-frontmatter.test.ts"); }); + it("keeps the trusted test-size guard closed around budget policy changes", () => { + const growthGuardrails = readYaml( + ".github/workflows/codebase-growth-guardrails.yaml", + ); + const guardRun = stepRuns(growthGuardrails.jobs["codebase-growth-guardrails"]).join( + "\n", + ); + + expect(guardRun).toContain("HEAD_REPO"); + expect(guardRun).toContain("HEAD_SHA"); + expect(guardRun).not.toContain(".raw_url"); + expect(guardRun).toContain("previous_filename"); + expect(guardRun).toContain("budgetChanged"); + expect(guardRun).toContain( + "has a legacy budget but no matching test file at the PR head", + ); + }); + it("uploads CLI Vitest JSON results for timing analysis", () => { const uploadStep = workflow.jobs["cli-tests"].steps?.find( (step) => step.name === "Upload CLI Vitest timing report", diff --git a/test/test-file-size-budget.test.ts b/test/test-file-size-budget.test.ts new file mode 100644 index 00000000000..9bce33c207a --- /dev/null +++ b/test/test-file-size-budget.test.ts @@ -0,0 +1,98 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it } from "vitest"; + +import { + countLines, + evaluateTestFileSizeBudget, + formatViolations, + parseBudget, +} from "../scripts/check-test-file-size-budget"; + +describe("test file size budget", () => { + it("counts trailing-newline and non-trailing-newline files consistently", () => { + expect(countLines("")).toBe(0); + expect(countLines("one")).toBe(1); + expect(countLines("one\n")).toBe(1); + expect(countLines("one\r\ntwo")).toBe(2); + }); + + it("flags non-grandfathered test files above the default ceiling", () => { + const violations = evaluateTestFileSizeBudget( + [{ file: "test/new-large.test.ts", lines: 151 }], + { defaultMaxLines: 150 }, + ); + + expect(violations).toEqual([ + { + kind: "oversized", + file: "test/new-large.test.ts", + lines: 151, + maxLines: 150, + budgetKind: "default", + }, + ]); + }); + + it("grandfathers existing large tests without allowing growth", () => { + const budget = { defaultMaxLines: 150, legacyMaxLines: { "test/legacy.test.ts": 250 } }; + + expect( + evaluateTestFileSizeBudget([{ file: "test/legacy.test.ts", lines: 250 }], budget), + ).toEqual([]); + expect( + evaluateTestFileSizeBudget([{ file: "test/legacy.test.ts", lines: 251 }], budget), + ).toEqual([ + { + kind: "oversized", + file: "test/legacy.test.ts", + lines: 251, + maxLines: 250, + budgetKind: "legacy", + }, + ]); + }); + + it("requires legacy budgets to ratchet down when oversized tests shrink", () => { + const violations = evaluateTestFileSizeBudget( + [{ file: "test/legacy.test.ts", lines: 200 }], + { defaultMaxLines: 150, legacyMaxLines: { "test/legacy.test.ts": 250 } }, + ); + + expect(violations).toEqual([ + { + kind: "legacy-ratchet", + file: "test/legacy.test.ts", + lines: 200, + maxLines: 250, + }, + ]); + expect(formatViolations(violations)).toContain("lower the budget entry"); + }); + + it("rejects stale legacy budget entries", () => { + const violations = evaluateTestFileSizeBudget([], { + defaultMaxLines: 150, + legacyMaxLines: { "test/deleted.test.ts": 200 }, + }); + + expect(violations).toEqual([ + { kind: "stale-legacy-budget", file: "test/deleted.test.ts", maxLines: 200 }, + ]); + }); + + it("parses the JSON budget format", () => { + expect( + parseBudget( + JSON.stringify({ + defaultMaxLines: 1500, + legacyMaxLines: { "test/legacy.test.ts": 2000 }, + }), + ), + ).toEqual({ + defaultMaxLines: 1500, + legacyMaxLines: { "test/legacy.test.ts": 2000 }, + }); + }); +});