Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions .archgate/adrs/ARCH-005-testing-standards.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,18 @@ Bun's built-in test runner (`bun test`) provides a Jest-compatible API, native T

## Decision

Use Bun's built-in test runner (`bun test`) for all tests. Test files live in `tests/`, mirroring `src/`; fixtures live in `tests/fixtures/`. Target 90% code coverage, enforced in CI.
Use Bun's built-in test runner (`bun test`) for all tests. Test files live in `tests/`, mirroring `src/`; fixtures live in `tests/fixtures/`. Target 95% code coverage, enforced in CI.

**Key conventions:**

1. **Directory structure mirrors `src/`** — `src/engine/runner.ts` is tested by `tests/engine/runner.test.ts`, so tests are discoverable by convention.
2. **Fixtures in `tests/fixtures/`** — sample ADR files and mock codebases are shared across suites.
3. **Temp directories for filesystem tests** — tests that write files use `mkdtemp` for isolation and clean up in `afterEach` or `afterAll`.
4. **Test file naming** — `<module-name>.test.ts`.
5. **Coverage target: 90%** — enforced in CI. PRs that drop total line coverage below 90% are blocked by the `Validate Code` gate check.
5. **Coverage target: 95%** — enforced in CI. PRs that drop total line coverage below 95% are blocked by the `Validate Code` gate check.
6. **Isolation is the test author's job** — Bun runs every test file in one process, so environment writes, `mock.module()` calls, and un-restored spies escape into later files and produce order-dependent flakes. Restore env vars with `restoreEnv()`, mock first-party modules with `spyOn` over an `import * as mod` namespace, and keep every write inside a `mkdtemp` directory.
7. **Mock `os.homedir()`, never `HOME`** — Bun caches `os.homedir()` on Linux, so a runtime `HOME` override is silently ignored and the code under test resolves the REAL home directory. Env-var overrides remain valid ONLY for code that reads `Bun.env.*` at call time (`vscode-settings.ts`'s `APPDATA` branch, the `paths.ts` helpers documented as "resolved at call time"). Production code MUST NOT be rewritten to read `Bun.env.HOME` just to make an env override work.
8. **Per-test timeouts only ever raise the global** — the suite runs `bun test --timeout 60000`, so a shorter override such as `}, 30_000` makes that test _more_ likely to time out, not less.
8. **Per-test timeouts only ever raise the global** — `bun run test` applies `--timeout 60000`, so a shorter override such as `}, 30_000` makes that test _more_ likely to time out, not less.

## Do's and Don'ts

Expand Down Expand Up @@ -153,7 +153,7 @@ afterEach(() => {
- **Familiar API** — the Jest-compatible `describe`/`it`/`expect` surface and `bun test --watch` need no onboarding.
- **Fixtures are reusable** — a shared `tests/fixtures/` directory provides consistent sample data across suites.
- **Same runtime for tests and production** — no behavior discrepancies from testing in Node.js while shipping on Bun.
- **Lint-enforced test hygiene** — custom oxlint plugins catch assertion-less tests and unguarded env restores before CI runs the suite.
- **Lint-enforced test hygiene** — custom oxlint plugins catch assertion-less tests, unguarded env restores, and first-party module mocks before CI runs the suite.

### Negative

Expand All @@ -166,7 +166,7 @@ afterEach(() => {
- **Bun test runner API changes** — newer APIs may still evolve between minor versions.
- **Mitigation:** the project pins a Bun version via `.prototools`; API changes surface during controlled upgrades with full suite validation.
- **Coverage reporting gaps** — `bun test --coverage` may misreport code paths, especially dynamically imported modules.
- **Mitigation:** the 90% threshold is enforced on total line coverage, not per-file, and critical modules (engine, formats) are tested thoroughly regardless of the aggregate.
- **Mitigation:** the 95% threshold is enforced on total line coverage, not per-file, and critical modules (engine, formats) are tested thoroughly regardless of the aggregate.
- **Cross-file pollution from shared process state** — leaked env vars, leaked spies, and writes to real user-scope paths (`~/.config/Code/User/settings.json`, `%APPDATA%`, `~/.cursor/`, `~/.config/opencode/`) produce order-dependent flakes that pass on a PR run and fail after merge with identical code. `Bun.env.NODE_ENV` left unset instead of set to `"test"` before Sentry initializes is the same class of leak — the SDK sets `enabled: Bun.env.NODE_ENV !== "test"`.
- **Mitigation:** `restoreEnv()` for every env capture, `try/finally` around inline spies, and an `os.homedir()` spy that keeps writes inside a `mkdtemp` directory; `test-isolation/no-bare-env-restore` blocks the env variant at lint time.
- **Platform-specific hangs and timeouts** — an external SDK instance left open keeps Bun's event loop alive on Linux and hangs `bun test` after every test passes, while slow Windows CI filesystems let large fixtures blow the per-test timeout and kill the staging subprocess (`git add . failed (exit 143)`, where 143 = 128 + SIGTERM). Neither reproduces on macOS or locally.
Expand All @@ -179,9 +179,10 @@ afterEach(() => {
- **Archgate rule** `ARCH-005/test-mirrors-src`: scans `src/` and verifies a corresponding `.test.ts` file exists in `tests/`. Severity: `error`.
- **oxlint plugin** `bun-test/expect-expect` (`lint/expect-expect.ts`): enabled for `tests/**/*.test.ts`, it fails the build for any runnable `test()`/`it()` (including `test.skipIf(...)()` and `test.each(...)()`) whose body contains no `expect()` call, while ignoring `test.skip` and `test.todo`. oxlint's built-in `jest/expect-expect` recognizes only `jest`/`vitest` imports, so it does not cover `bun:test` — this plugin fills that gap.
- **oxlint plugin** `test-isolation/no-bare-env-restore` (`lint/no-bare-env-restore.ts`): enabled for `tests/**/*.test.ts`, it fails the build for any `Bun.env.NAME = <identifier>` or `process.env.NAME = <identifier>` assignment whose identifier was itself captured from an env read earlier in the same file (e.g. `const originalHome = Bun.env.HOME`). Tracking the capture rather than a naming convention such as `original*` is what separates a restore from an override — both are spelled alike, so `Bun.env.HOME = tempDir` is deliberately left alone, as is computed access (`Bun.env[key]`), which is the shape of the `restoreEnv` helper itself.
- Both plugins are registered via `jsPlugins` in `.oxlintrc.json` and run as part of `bun run lint` (and therefore `bun run validate` and CI).
- **CI pipeline**: `bun test --timeout 60000` runs on every pull request. Test failures and per-test timeouts block merge, and all workflow jobs set `timeout-minutes` to prevent indefinite hangs.
- **Coverage threshold**: the `Coverage Report` job enforces a 90% minimum line coverage; below that it fails and the `Validate Code` gate blocks the PR.
- **oxlint plugin** `test-mocking/no-first-party-module-mock` (`lint/no-first-party-module-mock.ts`): enabled for `tests/**/*.test.ts`, it fails the build for any `mock.module()` whose specifier is relative and carries a `src` path segment, while leaving third-party specifiers (`inquirer`, `node:readline`) alone. oxlint is the right layer for this Don't: the call is syntax-detectable from its specifier, and because `mock.module` is process-global and retroactive, an instance in one file corrupts files that never mention it — a defect no file-by-file review can see. Each plugin file MUST declare a `meta.name` no other plugin uses; a duplicate name silently drops the later file's rules, and oxlint then rejects the config with "Rule not found in plugin".
- All three plugins are registered via `jsPlugins` in `.oxlintrc.json` and run as part of `bun run lint` (and therefore `bun run validate` and CI).
- **CI pipeline**: every pull request runs `bun run validate:coverage`, which reaches the suite through the `test:coverage` script (`bun test --timeout 60000 --coverage`). Invoke the suite by script name (GEN-003) — a bare `bun test` applies Bun's 5-second default instead of the 60-second global and reports timeouts that the gate never sees. Test failures and per-test timeouts block merge, and all workflow jobs set `timeout-minutes` to prevent indefinite hangs.
- **Coverage threshold**: the `Coverage Report` job enforces a 95% minimum line coverage; below that it fails and the `Validate Code` gate blocks the PR.

### Manual Enforcement

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ A loop is "standing in for `test.each()`" when either:

A loop that only builds shared setup data or fixtures for a single overall assertion is not covered by this decision.

**Runtime-derived collections:** when the iterated collection is produced at runtime — parsed from command output, gathered by a filesystem scan, or returned by the code under test — `test.each()` does not apply, because the rows do not exist at registration time. Such a loop MUST NOT carry a test's only `expect()`: an empty collection runs the body zero times, so the test asserts nothing and passes even when the behavior under test is entirely broken. Assert the collection itself instead — `expect(names).toEqual([...])`, or a length assertion — so an empty result fails. `bun-test/expect-expect` does not catch this, because the `expect()` call is lexically present.

## Do's and Don'ts

### Do
Expand Down
2 changes: 2 additions & 0 deletions .claude/agent-memory/archgate-developer/MEMORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ Exceptions: minor follow-up tweaks after validation already passed, and non-code
- **`archgate` is not on PATH here** — this IS the CLI repo. Use `bun run cli <command>`.
- **`archgate check` emits non-blocking diagnostics** alongside rule failures: `[suppression]`, `[briefing]`, and `[adr]` lines are advisories that never affect `pass` by default — but `--strict` (or `.archgate/config.json`'s `strict: true`) elevates all three into failures, per ARCH-026. Check which kind you have, and whether strict mode is active, before treating a finding as a blocker.
- **Splitting a test file for `oxlint`'s 500-line `max-lines` cap: add a sibling `<name>-<suffix>.test.ts`, don't trim coverage.** Precedent already established by `check-max-warnings.test.ts` as a sibling of `check.test.ts`; followed again for `reporter-strict.test.ts`, `sync-strict.test.ts`, and the `*-strict.test.ts` integration files when ARCH-026's tests pushed their parent files over the cap.
- **A cache-busting dynamic import hides coverage: `await import(`../mod?t=${Date.now()}`)` makes Bun load a second module instance whose execution is attributed to nothing.** The source file then reports its paths uncovered while they are in fact tested, so the reporter understates and the "gap" is an illusion — check for this specifier before writing tests for any file that looks mysteriously uncovered. Removing it took `update-check.ts` from 76.47% to 100% with zero new tests. A static import is safe only when the module holds no mutable module-level state; verify that first (file-backed caches and `Bun.env` reads at call time are fine).
- **Reproduce CI's coverage number yourself — bun's `All files` summary line is not it.** CI filters the merged lcov to `src/*` and computes `sum(LH)/sum(LF)`; bun's own table includes `tests/` and averages differently, so the two disagree by several points. Locally: `awk -F: '/^SF:/{p=($2~/^src[\\\/]/)} /^LF:/{if(p)f+=$2} /^LH:/{if(p)h+=$2} END{printf "%.2f\n",h/f*100}' coverage/lcov.info`. A single-platform local run is a floor, not the CI figure — CI unions Linux and Windows and counts a line covered if either platform hit it.
- **`--update-snapshots` is never on its own the fix for a failing snapshot.** The repo's snapshot (`tests/helpers/__snapshots__/rules-shim.test.ts.snap`) is the entire `rules.d.ts` a governed project receives, and its diff is the review artifact — read every hunk and confirm it follows from an intended `src/formats/rules.ts` edit before regenerating. Deleting the file routes around nothing: Bun fails a missing snapshot whenever `CI` is set, and passes it locally.
- **Commit with `--signoff`** — the DCO check rejects commits without `Signed-off-by`.
- **This repo is PUBLIC** — no private sibling-repo internals, no Claude session links in PRs or commits.
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/code-pull-request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ jobs:
id: coverage-report
uses: ./.github/actions/coverage-report
with:
min-coverage: "90"
min-coverage: "95"
github-token: ${{ github.token }}
event-name: ${{ github.event_name }}
pr-number: ${{ github.event.pull_request.number }}
Expand All @@ -294,7 +294,7 @@ jobs:
# (zizmor: code injection via template expansion).
COVERAGE: ${{ steps.coverage-report.outputs.coverage }}
run: |
MIN_COVERAGE=90
MIN_COVERAGE=95
# Pass values as awk data (-v), never interpolated into the awk
# program source — a crafted step output could inject awk code.
BELOW=$(awk -v coverage="$COVERAGE" -v minimum="$MIN_COVERAGE" \
Expand Down
2 changes: 2 additions & 0 deletions .oxlintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"jsPlugins": [
"./lint/expect-expect.ts",
"./lint/no-bare-env-restore.ts",
"./lint/no-first-party-module-mock.ts",
"./.archgate/lint/oxlint.ts"
],
"options": { "typeAware": true },
Expand Down Expand Up @@ -121,6 +122,7 @@
"files": ["tests/**/*.test.ts"],
"rules": {
"bun-test/expect-expect": "error",
"test-mocking/no-first-party-module-mock": "error",
"test-isolation/no-bare-env-restore": "error"
}
},
Expand Down
104 changes: 104 additions & 0 deletions lint/no-first-party-module-mock.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
// SPDX-License-Identifier: Apache-2.0
// Copyright 2026 Archgate

// Custom oxlint JS plugin: tests stub first-party modules with `spyOn` over an
// `import * as mod` namespace, never `mock.module()` — which is process-global,
// retroactive, and not undone by `mock.restore()`, so one file's stub reaches
// files that never mention it (ARCH-005). Third-party specifiers are allowed:
// `inquirer` and `node:readline` have no namespace object to spy.

/** Minimal ESTree-ish node shape. The oxlint AST is ESLint-compatible. */
type AstNode = { type: string } & Record<string, unknown>;

function isAstNode(value: unknown): value is AstNode {
return (
typeof value === "object" &&
value !== null &&
"type" in value &&
typeof value.type === "string"
);
}

function asNode(value: unknown): AstNode | undefined {
return isAstNode(value) ? value : undefined;
}

/**
* The property name of a member expression, covering both `mock.module` and
* the equivalent computed string form `mock["module"]`. A computed key that is
* not a string literal stays undefined — its name is not knowable statically.
*/
function memberPropertyName(node: AstNode): string | undefined {
const property = asNode(node.property);
if (node.computed === true) {
return property?.type === "Literal" && typeof property.value === "string"
? property.value
: undefined;
}
if (property?.type === "Identifier" && typeof property.name === "string") {
return property.name;
}
return undefined;
}

/** Whether `callee` is the `mock.module` member expression. */
function isMockModuleCallee(callee: AstNode | undefined): boolean {
if (callee?.type !== "MemberExpression") return false;
if (memberPropertyName(callee) !== "module") return false;
const base = asNode(callee.object);
return base?.type === "Identifier" && base.name === "mock";
}

/** The literal string value of a node, or undefined when it is not a plain string literal. */
function stringLiteralValue(node: AstNode | undefined): string | undefined {
if (node?.type !== "Literal") return undefined;
return typeof node.value === "string" ? node.value : undefined;
}

/**
* Whether `specifier` names a module inside this repository's `src/` tree.
*
* Detection is lexical: a relative specifier carrying a `src` path segment.
* `src/` is the only first-party source root, and no directory under `tests/`
* is named `src`, so a `src` segment identifies first-party code without
* resolving the path against the importing file's location.
*/
function isFirstPartySpecifier(specifier: string): boolean {
if (!specifier.startsWith(".")) return false;
return specifier.split("/").includes("src");
}

interface ReportDescriptor {
node: AstNode;
message: string;
}

interface RuleContext {
report(descriptor: ReportDescriptor): void;
}

function message(specifier: string): string {
return `Stub "${specifier}" with \`spyOn\` over an \`import * as mod\` namespace instead of \`mock.module()\`. \`mock.module()\` is process-global and retroactive, and \`mock.restore()\` does not undo it, so this stub reaches every other test file in the run (ARCH-005).`;
}

const noFirstPartyModuleMock = {
create(context: RuleContext) {
return {
CallExpression(node: AstNode) {
if (!isMockModuleCallee(asNode(node.callee))) return;
const args = Array.isArray(node.arguments) ? node.arguments : [];
const specifier = stringLiteralValue(asNode(args[0]));
if (specifier === undefined) return;
if (!isFirstPartySpecifier(specifier)) return;
context.report({ node, message: message(specifier) });
},
};
},
};

const plugin = {
meta: { name: "test-mocking" },
rules: { "no-first-party-module-mock": noFirstPartyModuleMock },
};

export default plugin;
Loading
Loading