From 9965cbabbcf685971e5d71ae03086fe4137ec990 Mon Sep 17 00:00:00 2001 From: Charan Jagwani Date: Wed, 9 Sep 2026 13:19:06 -0700 Subject: [PATCH] perf(ci): trim plugin installs from CLI coverage Signed-off-by: Charan Jagwani --- .../actions/ci-cli-coverage-merge/action.yaml | 2 +- .../actions/ci-cli-coverage-shard/action.yaml | 2 +- .github/actions/ci-install-dependencies.sh | 24 ++++++- ci/cli-test-timing-hints.json | 1 - .../pr-workflow-contract.test.ts | 9 ++- .../ci-install-dependencies.test.ts | 69 +++++++++++++++---- test/repository/plugin-vitest-project.test.ts | 69 +++++++++++++------ 7 files changed, 137 insertions(+), 39 deletions(-) diff --git a/.github/actions/ci-cli-coverage-merge/action.yaml b/.github/actions/ci-cli-coverage-merge/action.yaml index 5acd8a7fd11..f338bde21a9 100644 --- a/.github/actions/ci-cli-coverage-merge/action.yaml +++ b/.github/actions/ci-cli-coverage-merge/action.yaml @@ -41,7 +41,7 @@ runs: shell: bash env: NODE_AUTH_TOKEN: ${{ github.event_name == 'push' && github.token || '' }} - run: bash "$GITHUB_ACTION_PATH/../ci-install-dependencies.sh" + run: bash "$GITHUB_ACTION_PATH/../ci-install-dependencies.sh" none - name: Download compiled CLI artifact uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 diff --git a/.github/actions/ci-cli-coverage-shard/action.yaml b/.github/actions/ci-cli-coverage-shard/action.yaml index ea642e0d58d..e107a936316 100644 --- a/.github/actions/ci-cli-coverage-shard/action.yaml +++ b/.github/actions/ci-cli-coverage-shard/action.yaml @@ -93,7 +93,7 @@ runs: shell: bash env: NODE_AUTH_TOKEN: ${{ github.event_name == 'push' && github.token || '' }} - run: bash "$GITHUB_ACTION_PATH/../ci-install-dependencies.sh" + run: bash "$GITHUB_ACTION_PATH/../ci-install-dependencies.sh" production - name: Validate changed live E2E mock parity if: ${{ inputs.shard == '1' }} diff --git a/.github/actions/ci-install-dependencies.sh b/.github/actions/ci-install-dependencies.sh index 4d6480789c4..c15c69f132b 100755 --- a/.github/actions/ci-install-dependencies.sh +++ b/.github/actions/ci-install-dependencies.sh @@ -4,6 +4,25 @@ set -euo pipefail +if [ "$#" -gt 1 ]; then + echo "Usage: ci-install-dependencies.sh [full|production|none]" >&2 + exit 1 +fi + +plugin_install_mode="${1:-full}" +plugin_install_args=(--prefix nemoclaw ci) +case "$plugin_install_mode" in + full) ;; + production) + plugin_install_args+=(--omit=dev) + ;; + none) ;; + *) + echo "Unsupported plugin dependency install mode: $plugin_install_mode" >&2 + exit 1 + ;; +esac + candidate_npmrc="$(find . -path './.git' -prune -o -name .npmrc -print -quit)" if [ -n "$candidate_npmrc" ]; then echo "Candidate repository npm configuration is not allowed during trusted dependency installation." >&2 @@ -57,4 +76,7 @@ if [ "$package_mode" = "registry" ] && [ -n "${NODE_AUTH_TOKEN:-}" ]; then fi npm ci --ignore-scripts --prefer-offline --no-audit --no-fund --cache "$npm_cache" -npm --prefix nemoclaw ci --ignore-scripts --prefer-offline --no-audit --no-fund --cache "$npm_cache" +if [ "$plugin_install_mode" != "none" ]; then + npm "${plugin_install_args[@]}" \ + --ignore-scripts --prefer-offline --no-audit --no-fund --cache "$npm_cache" +fi diff --git a/ci/cli-test-timing-hints.json b/ci/cli-test-timing-hints.json index 1d852284e49..04e3eb0ed47 100644 --- a/ci/cli-test-timing-hints.json +++ b/ci/cli-test-timing-hints.json @@ -177,7 +177,6 @@ "test/onboarding/onboard-terminal-dashboard.test.ts": 11927, "test/onboarding/onboard.test.ts": 7899, "test/repository/layer-import-boundaries.test.ts": 5292, - "test/repository/plugin-vitest-project.test.ts": 7766, "test/repository/source-architecture.test.ts": 5535, "test/repository/source-require-loader.test.ts": 5155, "test/runtime/gateway/gateway-drift-preflight.test.ts": 7324, diff --git a/test/automation/pull-requests/pr-workflow-contract.test.ts b/test/automation/pull-requests/pr-workflow-contract.test.ts index 3ad7bb39c01..363ca10e175 100644 --- a/test/automation/pull-requests/pr-workflow-contract.test.ts +++ b/test/automation/pull-requests/pr-workflow-contract.test.ts @@ -566,7 +566,14 @@ describe("pull request and main workflow contracts", () => { })), ); expect(actions.map((action) => requiredStep(action, "Install dependencies").run)).toEqual( - actions.map(() => 'bash "$GITHUB_ACTION_PATH/../ci-install-dependencies.sh"'), + [ + 'bash "$GITHUB_ACTION_PATH/../ci-install-dependencies.sh"', + 'bash "$GITHUB_ACTION_PATH/../ci-install-dependencies.sh"', + 'bash "$GITHUB_ACTION_PATH/../ci-install-dependencies.sh" none', + 'bash "$GITHUB_ACTION_PATH/../ci-install-dependencies.sh"', + 'bash "$GITHUB_ACTION_PATH/../ci-install-dependencies.sh" production', + 'bash "$GITHUB_ACTION_PATH/../ci-install-dependencies.sh"', + ], ); }); diff --git a/test/repository/ci-install-dependencies.test.ts b/test/repository/ci-install-dependencies.test.ts index c6e7e718937..4dc5985bd58 100644 --- a/test/repository/ci-install-dependencies.test.ts +++ b/test/repository/ci-install-dependencies.test.ts @@ -42,6 +42,22 @@ function makeFixture(): { root: string; trace: string; path: string } { return { root, trace, path: `${bin}:${process.env.PATH || ""}` }; } +function runInstaller(fixture: ReturnType, args: string[] = []) { + return spawnSync("bash", [installer, ...args], { + cwd: fixture.root, + encoding: "utf8", + env: { + ...process.env, + GITHUB_ACTION_PATH: compositeActionPath, + GITHUB_EVENT_NAME: "pull_request", + NPM_CONFIG_CACHE: join(fixture.root, "npm-cache"), + NPM_TRACE: fixture.trace, + PATH: fixture.path, + RUNNER_TEMP: join(fixture.root, "runner-temp"), + }, + }); +} + afterEach(() => { for (const root of temporaryRoots.splice(0)) rmSync(root, { force: true, recursive: true }); }); @@ -49,20 +65,7 @@ afterEach(() => { describe("shared CI dependency installer", () => { it("installs from a composite-action path without lifecycle scripts", () => { const fixture = makeFixture(); - - const result = spawnSync("bash", [installer], { - cwd: fixture.root, - encoding: "utf8", - env: { - ...process.env, - GITHUB_ACTION_PATH: compositeActionPath, - GITHUB_EVENT_NAME: "pull_request", - NPM_CONFIG_CACHE: join(fixture.root, "npm-cache"), - NPM_TRACE: fixture.trace, - PATH: fixture.path, - RUNNER_TEMP: join(fixture.root, "runner-temp"), - }, - }); + const result = runInstaller(fixture); expect(result.status, result.stderr).toBe(0); expect(readFileSync(fixture.trace, "utf8").trim().split("\n")).toEqual([ @@ -71,6 +74,44 @@ describe("shared CI dependency installer", () => { ]); }); + it("can install only plugin production dependencies", () => { + const fixture = makeFixture(); + const result = runInstaller(fixture, ["production"]); + + expect(result.status, result.stderr).toBe(0); + expect(readFileSync(fixture.trace, "utf8").trim().split("\n")).toEqual([ + `ci --ignore-scripts --prefer-offline --no-audit --no-fund --cache ${join(fixture.root, "npm-cache")}`, + `--prefix nemoclaw ci --omit=dev --ignore-scripts --prefer-offline --no-audit --no-fund --cache ${join(fixture.root, "npm-cache")}`, + ]); + }); + + it("can skip plugin dependency installation", () => { + const fixture = makeFixture(); + const result = runInstaller(fixture, ["none"]); + + expect(result.status, result.stderr).toBe(0); + expect(readFileSync(fixture.trace, "utf8").trim()).toBe( + `ci --ignore-scripts --prefer-offline --no-audit --no-fund --cache ${join(fixture.root, "npm-cache")}`, + ); + }); + + it.each([ + [["invalid"], "Unsupported plugin dependency install mode: invalid\n"], + [["production", "extra"], "Usage: ci-install-dependencies.sh [full|production|none]\n"], + ] as const)("rejects unsupported install arguments before npm runs [case %#]", (args, error) => { + const fixture = makeFixture(); + + const result = spawnSync("bash", [installer, ...args], { + cwd: fixture.root, + encoding: "utf8", + env: { ...process.env, NPM_TRACE: fixture.trace, PATH: fixture.path }, + }); + + expect(result.status).toBe(1); + expect(result.stderr).toBe(error); + expect(existsSync(fixture.trace)).toBe(false); + }); + it("rejects candidate npm configuration before npm receives the package token", () => { const fixture = makeFixture(); writeFileSync( diff --git a/test/repository/plugin-vitest-project.test.ts b/test/repository/plugin-vitest-project.test.ts index 23d7e6098d8..3c2e5a0ed03 100644 --- a/test/repository/plugin-vitest-project.test.ts +++ b/test/repository/plugin-vitest-project.test.ts @@ -9,17 +9,37 @@ import { describe, expect, it } from "vitest"; const repositoryRoot = path.resolve(import.meta.dirname, "../.."); const rootRequire = createRequire(path.join(repositoryRoot, "package.json")); -const pluginRequire = createRequire(path.join(repositoryRoot, "nemoclaw", "package.json")); -const pluginTypeScript = pluginRequire.resolve("typescript/bin/tsc"); +const rootTypeScript = rootRequire.resolve("typescript/bin/tsc"); -function installedVersion(requireFromPackage: NodeJS.Require, packageName: string): string { - return (requireFromPackage(`${packageName}/package.json`) as { version: string }).version; +type NpmDependencyTree = { + dependencies?: Record; + version?: string; +}; + +function lockedDependencyTree(prefix?: string): NpmDependencyTree { + const prefixArgs = prefix ? ["--prefix", prefix] : []; + return JSON.parse( + execFileSync( + "npm", + [...prefixArgs, "ls", "--package-lock-only", "--json", "typescript", "vitest", "vite"], + { + cwd: repositoryRoot, + encoding: "utf8", + }, + ), + ) as NpmDependencyTree; +} + +function requiredLockedVersion(version: string | undefined, dependency: string): string { + expect(version, `${dependency} lockfile version`).toBeTypeOf("string"); + expect(version, `${dependency} lockfile version`).not.toBe(""); + return version as string; } function listedTypeScriptFiles(configPath: string): string[] { return execFileSync( process.execPath, - [pluginTypeScript, "--noEmit", "-p", configPath, "--listFilesOnly"], + [rootTypeScript, "--noEmit", "-p", configPath, "--listFilesOnly"], { cwd: repositoryRoot, encoding: "utf8" }, ) .trim() @@ -28,27 +48,36 @@ function listedTypeScriptFiles(configPath: string): string[] { } describe("plugin Vitest project contract", () => { - it.each(["vitest", "vite"] as const)( - "keeps standalone plugin dependencies on the root Vitest toolchain [case %#]", - (packageName) => { - expect(installedVersion(pluginRequire, packageName), packageName).toBe( - installedVersion(rootRequire, packageName), - ); - }, - ); - - it("typechecks plugin production and test sources without emitting tests", () => { + it("keeps the standalone plugin lock on the root test toolchain", () => { + const rootTree = lockedDependencyTree(); + const pluginTree = lockedDependencyTree("nemoclaw"); + + expect(requiredLockedVersion(pluginTree.dependencies?.vitest?.version, "plugin vitest")).toBe( + requiredLockedVersion(rootTree.dependencies?.vitest?.version, "root vitest"), + ); + expect( + requiredLockedVersion( + pluginTree.dependencies?.vitest?.dependencies?.vite?.version, + "plugin vite", + ), + ).toBe( + requiredLockedVersion( + rootTree.dependencies?.vitest?.dependencies?.vite?.version, + "root vite", + ), + ); + expect( + requiredLockedVersion(pluginTree.dependencies?.typescript?.version, "plugin typescript"), + ).toBe(requiredLockedVersion(rootTree.dependencies?.typescript?.version, "root typescript")); + }); + + it("keeps plugin production and test TypeScript projects disjoint", () => { const productionFiles = listedTypeScriptFiles("nemoclaw/tsconfig.json"); const testFiles = listedTypeScriptFiles("nemoclaw/tsconfig.test.json"); - const typecheckOutput = execFileSync("npm", ["--prefix", "nemoclaw", "run", "typecheck"], { - cwd: repositoryRoot, - encoding: "utf8", - }); expect(productionFiles.some((file) => file.endsWith(".test.ts"))).toBe(false); expect(testFiles).toContain(path.join(repositoryRoot, "nemoclaw", "src", "register.test.ts")); expect(testFiles).toContain(path.join(repositoryRoot, "nemoclaw", "vitest.config.ts")); expect(testFiles).toContain(path.join(repositoryRoot, "nemoclaw", "vitest.project.ts")); - expect(typecheckOutput).toContain("tsc --noEmit -p tsconfig.test.json"); }); });