ci: type-check the test files a pull request changes (#1364)

.github/workflows/ci.yml

@@ -32,6 +32,14 @@ jobs:
       - name: Build
         run: npm run build
 
+      - name: Type-check changed tests
+        if: github.event_name == 'pull_request'
+        env:
+          BASE_SHA: ${{ github.event.pull_request.base.sha }}
+        run: |
+          git fetch --no-tags --depth=1 origin "$BASE_SHA"
+          node scripts/typecheck-changed-tests.mjs "$BASE_SHA"
+
       - name: Sync advisory database (required by e2e tests using --offline)
         run: node dist/index.js advisories sync
 

CONTRIBUTING.md

@@ -34,6 +34,7 @@ diagnosed a problem in the release notes, not only the author of the merged PR.
   ```
 
   `advisories sync` populates the local advisory database. E2E tests that scan with `--offline` read from it. Without the sync step, those tests fail while the rest of the suite passes.
+- CI also type-checks the test files a pull request adds or changes. Run it locally with `node scripts/typecheck-changed-tests.mjs <base-ref>` (for example, `upstream/main`) before opening the PR.
 - Keep changes tightly scoped to the issue. Do not refactor unrelated code in the same PR.
 - New utility functions belong in a focused module under `src/utils/` - not inlined in `src/index.ts`.
 - Use `node:fs`, `node:path`, etc. for Node built-in imports.

scripts/typecheck-changed-tests.mjs

@@ -0,0 +1,168 @@
+#!/usr/bin/env node
+/**
+ * Type-check the test files a pull request adds or changes.
+ *
+ * Jest transpiles tests without type-checking them, so a test can assert a
+ * shape the compiler would reject. This gate type-checks changed test files
+ * only: errors attached to unchanged files are ignored so a PR is not blocked
+ * by existing debt, while configuration and compiler failures always fail.
+ *
+ * Usage: node scripts/typecheck-changed-tests.mjs <base-ref> [--root <path>]
+ */
+
+import { execFileSync } from "node:child_process";
+import { dirname, resolve } from "node:path";
+import { fileURLToPath } from "node:url";
+import ts from "typescript";
+
+const defaultRoot = resolve(dirname(fileURLToPath(import.meta.url)), "..");
+
+/**
+ * List the test files the branch added, copied, modified or renamed.
+ *
+ * `-z` keeps Git from quoting paths, so filenames with spaces, Unicode or
+ * newlines survive intact. Splitting on NUL is the only safe delimiter here.
+ */
+function changedTestFiles(baseRef, root = defaultRoot) {
+  const output = execFileSync(
+    "git",
+    ["diff", "-z", "--name-only", "--diff-filter=ACMR", baseRef, "HEAD", "--", "tests"],
+    { cwd: root, encoding: "utf8" },
+  );
+
+  return output
+    .split("\0")
+    .filter(Boolean)
+    .filter((file) => file.endsWith(".ts"))
+    .sort();
+}
+
+function formatDiagnostic(diagnostic) {
+  const message = ts.flattenDiagnosticMessageText(diagnostic.messageText, "\n");
+  if (!diagnostic.file) {
+    return `error TS${diagnostic.code}: ${message}`;
+  }
+  const position = diagnostic.file.getLineAndCharacterOfPosition(diagnostic.start ?? 0);
+  return `${diagnostic.file.fileName}(${position.line + 1},${position.character + 1}): error TS${diagnostic.code}: ${message}`;
+}
+
+function typeCheckChangedTests(baseRef, root = defaultRoot) {
+  const files = changedTestFiles(baseRef, root);
+  if (files.length === 0) {
+    return { skipped: true, changedFiles: [], failures: [], global: [], ignored: 0 };
+  }
+
+  const configPath = resolve(root, "tsconfig.test.json");
+  const config = ts.readConfigFile(configPath, ts.sys.readFile);
+  if (config.error) {
+    return { skipped: false, changedFiles: files, failures: [], global: [config.error], ignored: 0 };
+  }
+
+  const parsed = ts.parseJsonConfigFileContent(config.config, ts.sys, root, undefined, configPath);
+  if (parsed.errors.length > 0) {
+    return { skipped: false, changedFiles: files, failures: [], global: parsed.errors, ignored: 0 };
+  }
+
+  const changed = new Set(files.map((file) => resolve(root, file)));
+  const options = { ...parsed.options, noEmit: true };
+
+  let diagnostics;
+  try {
+    const program = ts.createProgram({
+      rootNames: [...changed],
+      options,
+      projectReferences: parsed.projectReferences,
+    });
+    diagnostics = ts.getPreEmitDiagnostics(program);
+  } catch (error) {
+    const message = error instanceof Error ? error.message : String(error);
+    return {
+      skipped: false,
+      changedFiles: files,
+      failures: [],
+      global: [{ code: 0, category: ts.DiagnosticCategory.Error, messageText: message }],
+      ignored: 0,
+    };
+  }
+
+  const failures = [];
+  const global = [];
+  let ignored = 0;
+
+  for (const diagnostic of diagnostics) {
+    if (diagnostic.category !== ts.DiagnosticCategory.Error) continue;
+
+    if (!diagnostic.file) {
+      global.push(diagnostic);
+      continue;
+    }
+
+    if (changed.has(resolve(root, diagnostic.file.fileName))) {
+      failures.push(diagnostic);
+    } else {
+      ignored++;
+    }
+  }
+
+  return { skipped: false, changedFiles: files, failures, global, ignored };
+}
+
+function parseArgs(argv) {
+  let baseRef;
+  let root = defaultRoot;
+
+  for (let index = 0; index < argv.length; index++) {
+    const arg = argv[index];
+    if (arg === "--root") {
+      const value = argv[++index];
+      if (!value) throw new Error("--root requires a path");
+      root = resolve(value);
+    } else if (!baseRef) {
+      baseRef = arg;
+    }
+  }
+
+  return { baseRef, root };
+}
+
+function main() {
+  let args;
+  try {
+    args = parseArgs(process.argv.slice(2));
+  } catch (error) {
+    console.error(error instanceof Error ? error.message : String(error));
+    process.exit(2);
+  }
+
+  if (!args.baseRef) {
+    console.error("usage: node scripts/typecheck-changed-tests.mjs <base-ref> [--root <path>]");
+    process.exit(2);
+  }
+
+  const result = typeCheckChangedTests(args.baseRef, args.root);
+
+  if (result.skipped) {
+    console.log("No changed test files; skipping test-tree type-check.");
+    process.exit(0);
+  }
+
+  const diagnostics = [...result.failures, ...result.global];
+
+  if (diagnostics.length > 0) {
+    console.error(`Test type-check failed (${diagnostics.length}):\n`);
+    for (const diagnostic of diagnostics) console.error(formatDiagnostic(diagnostic));
+    if (result.ignored > 0) {
+      console.error(
+        `\nIgnored ${result.ignored} error(s) in unchanged files; those belong to the existing backlog.`,
+      );
+    }
+    process.exit(1);
+  }
+
+  console.log(
+    `Type-checked ${result.changedFiles.length} changed test file(s): no errors.` +
+      (result.ignored > 0 ? ` Ignored ${result.ignored} error(s) in unchanged files.` : ""),
+  );
+}
+
+main();

tests/scripts/typecheck-changed-tests.test.ts

@@ -0,0 +1,168 @@
+import { execFileSync, spawnSync } from "node:child_process";
+import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { fileURLToPath } from "node:url";
+
+/**
+ * The gate must see the exact files Git reports. Filenames are the one place
+ * where a naive newline split breaks: spaces survive trim(), but a Unicode
+ * path or a path with an embedded newline does not. These tests run the real
+ * script against throwaway repositories so the git plumbing is exercised too.
+ */
+
+const script = fileURLToPath(new URL("../../scripts/typecheck-changed-tests.mjs", import.meta.url));
+const roots: string[] = [];
+
+function git(cwd: string, ...args: string[]): void {
+  execFileSync("git", args, {
+    cwd,
+    stdio: "ignore",
+    env: {
+      ...process.env,
+      GIT_AUTHOR_NAME: "test",
+      GIT_AUTHOR_EMAIL: "test@example.com",
+      GIT_COMMITTER_NAME: "test",
+      GIT_COMMITTER_EMAIL: "test@example.com",
+    },
+  });
+}
+
+function commit(cwd: string, message: string): void {
+  git(cwd, "add", "-A");
+  git(cwd, "commit", "-q", "-m", message);
+}
+
+function write(root: string, relativePath: string, contents: string): void {
+  const fullPath = join(root, relativePath);
+  mkdirSync(join(fullPath, ".."), { recursive: true });
+  writeFileSync(fullPath, contents, "utf8");
+}
+
+function makeRepo(): string {
+  const root = mkdtempSync(join(tmpdir(), "cve-lite-typecheck-"));
+  roots.push(root);
+  git(root, "init", "-q");
+  write(
+    root,
+    "tsconfig.test.json",
+    JSON.stringify(
+      {
+        compilerOptions: {
+          target: "ES2022",
+          module: "NodeNext",
+          moduleResolution: "NodeNext",
+          strict: true,
+          noEmit: true,
+          skipLibCheck: true,
+          types: [],
+        },
+        include: ["tests/**/*.ts"],
+      },
+      null,
+      2,
+    ),
+  );
+  mkdirSync(join(root, "tests"), { recursive: true });
+  return root;
+}
+
+function runGate(root: string, base: string) {
+  return spawnSync(process.execPath, [script, base, "--root", root], { encoding: "utf8" });
+}
+
+afterEach(() => {
+  for (const root of roots.splice(0)) {
+    rmSync(root, { recursive: true, force: true });
+  }
+});
+
+describe("typecheck-changed-tests", () => {
+  it("fails a normal changed file with a type error", () => {
+    const root = makeRepo();
+    commit(root, "base");
+    write(root, "tests/broken.test.ts", 'const broken: number = "not a number";\n');
+    commit(root, "head");
+
+    const result = runGate(root, "HEAD~1");
+
+    expect(result.status).toBe(1);
+    expect(result.stderr).toContain("tests/broken.test.ts");
+    expect(result.stderr).toContain("error TS2322");
+  });
+
+  it("fails a Unicode filename with a type error", () => {
+    const root = makeRepo();
+    commit(root, "base");
+    write(root, "tests/café-测试.test.ts", 'const broken: number = "not a number";\n');
+    commit(root, "head");
+
+    const result = runGate(root, "HEAD~1");
+
+    expect(result.status).toBe(1);
+    expect(result.stderr).toContain("café-测试.test.ts");
+    expect(result.stderr).toContain("error TS2322");
+  });
+
+  it("fails a filename containing spaces even when an imported unchanged file also has errors", () => {
+    const root = makeRepo();
+    write(root, "tests/shared-helper.ts", 'export const shared: number = "debt";\n');
+    commit(root, "base");
+    write(
+      root,
+      "tests/space name.test.ts",
+      'import { shared } from "./shared-helper.js";\nconst broken: number = "not a number";\nexport const value = shared + broken;\n',
+    );
+    commit(root, "head");
+
+    const result = runGate(root, "HEAD~1");
+
+    expect(result.status).toBe(1);
+    expect(result.stderr).toContain("space name.test.ts");
+    expect(result.stderr).toContain("Ignored 1 error");
+  });
+
+  it("passes a valid changed file despite unchanged-file debt", () => {
+    const root = makeRepo();
+    write(root, "tests/legacy-debt.ts", 'export const legacy: number = "debt";\n');
+    commit(root, "base");
+    write(
+      root,
+      "tests/valid.test.ts",
+      'import { legacy } from "./legacy-debt.js";\nexport const value: number = legacy;\n',
+    );
+    commit(root, "head");
+
+    const result = runGate(root, "HEAD~1");
+
+    expect(result.status).toBe(0);
+    expect(result.stdout).toContain("Type-checked 1 changed test file(s): no errors.");
+    expect(result.stdout).toContain("Ignored 1 error");
+  });
+
+  it("skips successfully when no test files changed", () => {
+    const root = makeRepo();
+    commit(root, "base");
+
+    const result = runGate(root, "HEAD");
+
+    expect(result.status).toBe(0);
+    expect(result.stdout).toContain("No changed test files; skipping test-tree type-check.");
+  });
+});
+
+describe("typecheck-changed-tests configuration failures", () => {
+  it("fails when the test TypeScript config cannot be parsed", () => {
+    const root = makeRepo();
+    commit(root, "base");
+    write(root, "tsconfig.test.json", "{ this is not json");
+    write(root, "tests/valid.test.ts", "export const value = 1;\n");
+    commit(root, "head");
+
+    const result = runGate(root, "HEAD~1");
+
+    expect(result.status).toBe(1);
+    expect(result.stderr).toContain("Test type-check failed");
+    expect(result.stderr).toContain("error TS");
+  });
+});