diff --git a/.archgate/adrs/ARCH-005-testing-standards.md b/.archgate/adrs/ARCH-005-testing-standards.md index 8a7dfc5f..053deb50 100644 --- a/.archgate/adrs/ARCH-005-testing-standards.md +++ b/.archgate/adrs/ARCH-005-testing-standards.md @@ -44,6 +44,10 @@ Use Bun's built-in test runner (`bun test`) for all tests. Test files go in `tes - **When a test creates a temp git repo and needs to call `git commit`, configure local user identity first** — CI runners have no global git config, so commits fail without explicit local identity. Set it with `await Bun.$\`git config user.email "test@test.com"\`.cwd(tempDir).quiet()`and`await Bun.$\`git config user.name "Test"\`.cwd(tempDir).quiet()`immediately after`git init`. - Test public module interfaces, not private implementation details - Use descriptive test names that explain the expected behavior +- **Every test MUST contain at least one `expect()` assertion** — enforced by the custom `bun-test/expect-expect` oxlint plugin at `lint/expect-expect.ts` (registered via `jsPlugins` in `.oxlintrc.json`, enabled for `tests/**/*.test.ts`). `bun run lint` fails on any runnable `test()`/`it()` whose body contains no `expect()`. oxlint's built-in `jest/expect-expect` does not cover `bun:test`, which is why this plugin exists. +- **Make implicit "does not throw" contracts explicit** — for a smoke test that merely invokes a function, assert the contract: `expect(() => fn()).not.toThrow()` for synchronous calls, or `await expect(promise).resolves.toBeUndefined()` for async calls. Calling a function with no assertion provides false confidence and is blocked by the assertion rule. +- **Use `test.skip` or `test.todo` for intentionally empty or disabled tests** — the `bun-test/expect-expect` rule deliberately ignores `.skip` and `.todo` so placeholders remain explicit and self-documenting. +- **When adding the first `expect()` to a previously assertion-less test file, add `expect` to the `bun:test` import** — older smoke-test files (e.g., `sentry.test.ts`) historically omitted it because their bodies never asserted. - **When mocking `fetch` in tests, assign directly to `globalThis.fetch`** — use `globalThis.fetch = mockFn as unknown as typeof fetch`. Restore in `afterEach` via `mock.restore()` (from `bun:test`) or by reassigning the original reference before the test. - **Wrap `spyOn` / `mockImplementation` calls in `try/finally` to guarantee `mockRestore()` runs** — when `expect()` assertions fail, they throw immediately, skipping any `mockRestore()` that follows. The un-restored spy leaks into subsequent tests, causing false positives or false negatives. Pattern: `const spy = spyOn(...).mockImplementation(() => {}); try { /* assertions */ } finally { spy.mockRestore(); }`. Alternatively, create and restore spies in `beforeEach`/`afterEach` hooks instead of inline. - **Make large production thresholds injectable so tests can use a small value** — When production code only acts past a large numeric threshold (e.g., `SCOPE_FILE_WARN_THRESHOLD = 1000` in `src/engine/git-files.ts`), add an optional parameter that defaults to the module constant (e.g., `resolveScopedFiles(root, globs, { fileWarnThreshold })`). Tests inject a tiny value such as `5` and create only a handful of files to exercise the same code path. This keeps the test fast and deterministic on every platform instead of materializing thousands of fixture files. @@ -58,6 +62,8 @@ Use Bun's built-in test runner (`bun test`) for all tests. Test files go in `tes - **Don't rely on globally-configured git identity in temp git repos** — always set `user.email` and `user.name` locally in any repo that makes commits. Omitting this works locally (where developers have global git config) but fails silently in CI, producing a cryptic `ShellPromise` error with no indication that git identity is the cause. - **Don't let tests send real events to Sentry** — set `Bun.env.NODE_ENV = "test"` in `beforeEach` (and restore in `afterEach`) for any test that initializes Sentry. The Sentry SDK is configured with `enabled: Bun.env.NODE_ENV !== "test"` to prevent test noise from polluting production error tracking. - Don't skip tests without a tracking issue +- **Don't write assertion-less tests** — a `test()`/`it()` body with no `expect()` call silently passes and gives false confidence. The `bun-test/expect-expect` oxlint plugin blocks these at lint time. +- **Don't use a bare early `return` or an empty callback body to conditionally skip a test** — use `test.skipIf(condition)`, `test.skip`, or `test.todo` so the skip is explicit and the assertion rule can recognize it. Bare returns and empty bodies are exactly how assertion-less tests accumulated silently before the rule existed. - Don't import test utilities from `node:test` — use Bun's built-in `bun:test` module - **Don't use `mock.module("node:fetch", ...)` to intercept HTTP fetch calls** — in Bun, the runtime fetch is `globalThis.fetch` and `mock.module` targeting `node:fetch` does not intercept it. The mock silently has no effect: the real network is hit, making tests non-deterministic and dependent on external services. Assign `globalThis.fetch` directly instead (see Do's above). - **Don't place `mockRestore()` after assertions without `try/finally` protection** — if the assertion throws, the spy is never restored and contaminates subsequent tests. This is especially dangerous when spying on globals like `console.warn` or `globalThis.fetch`, where a leaked spy silently suppresses or redirects output for every test that follows. @@ -202,6 +208,7 @@ globalThis.fetch = (() => ### Automated Enforcement - **Archgate rule** `ARCH-005/test-mirrors-src`: Scans all source files in `src/` and verifies a corresponding `.test.ts` file exists in `tests/`. Severity: `error`. +- **oxlint plugin** `bun-test/expect-expect` (`lint/expect-expect.ts`): A custom oxlint JS plugin enabled for `tests/**/*.test.ts` that fails the build for any runnable `test()`/`it()` (including `test.skipIf(...)()`, `test.each(...)()`) whose body contains no `expect()` call. It ignores `test.skip` and `test.todo`. Runs as part of `bun run lint` (and therefore `bun run validate` and CI). oxlint's built-in `jest/expect-expect` only recognizes `jest`/`vitest` imports, so it does not cover `bun:test` — this plugin fills that gap. - **CI pipeline**: `bun test --timeout 60000` runs on every pull request. Test failures and per-test timeouts block merge. All workflow jobs have `timeout-minutes` set to prevent indefinite hangs. - **Coverage threshold**: The `Coverage Report` job enforces a 90% minimum line coverage. If total coverage drops below 90%, the job fails and the `Validate Code` gate blocks the PR. @@ -216,7 +223,8 @@ Code reviewers MUST verify: 5. Tests that call `git commit` on a temp repo configure `user.email` and `user.name` locally before committing 6. Tests that mock HTTP fetch assign `globalThis.fetch` directly — no `mock.module("node:fetch", ...)` usage 7. Tests that use `spyOn` or `mockImplementation` inline (not in `beforeEach`/`afterEach`) wrap the spy lifecycle in `try/finally` to guarantee `mockRestore()` runs even when assertions fail -8. Tests that exercise a numeric production threshold inject a small threshold value rather than generating fixtures large enough to trip the production default, and no per-test timeout override is shorter than the global `--timeout 60000` +8. Every test asserts something with `expect()` — no smoke tests that merely call a function; "does not throw" contracts are made explicit via `expect(() => fn()).not.toThrow()` or `await expect(promise).resolves.toBeUndefined()` +9. Tests that exercise a numeric production threshold inject a small threshold value rather than generating fixtures large enough to trip the production default, and no per-test timeout override is shorter than the global `--timeout 60000` ## References diff --git a/.claude/agent-memory/archgate-developer/MEMORY.md b/.claude/agent-memory/archgate-developer/MEMORY.md index 7bf2ebe6..a38baac8 100644 --- a/.claude/agent-memory/archgate-developer/MEMORY.md +++ b/.claude/agent-memory/archgate-developer/MEMORY.md @@ -30,6 +30,10 @@ Skipping steps 2 or 3 is a workflow violation. The user should NEVER have to inv ## Patterns & Fixes +- **Custom oxlint JS plugins live in `lint/*.ts`, registered via `jsPlugins` in `.oxlintrc.json`** — The repo has a custom plugin `lint/expect-expect.ts` (rule id `bun-test/expect-expect`) that fails the build for any runnable `bun:test` `test()`/`it()` (incl. `test.skipIf(...)()`, `test.each(...)()`) whose body has no `expect()` call; it ignores `test.skip`/`test.todo`. It exists because oxlint's built-in `jest/expect-expect` only recognizes `jest`/`vitest` imports and silently skips `bun:test`. The plugin uses the ESLint-compatible default-export shape (`{ meta:{name}, rules:{ "x": { create(ctx){ return { CallExpression(node){...} } } } } }`) and runs as native TS under Bun (no build step). Important scoping: `lint/` is NOT in `tsconfig.json` `include` (so `tsc` does not typecheck it) and NOT in `knip.json` `project` (so knip ignores it) — but `oxlint --deny-warnings .` DOES lint it, so the plugin file must itself pass all oxlint rules. To enable a jsPlugin rule only for tests, list it under an `overrides` entry for `tests/**/*.test.ts` (the plugin is loaded top-level via `jsPlugins`, the rule is turned on in the override). The convention is now documented in ARCH-005. +- **Converting assertion-less smoke tests: assert the "does not throw" contract explicitly** — `expect(() => fn()).not.toThrow()` for sync void calls, `await expect(promise).resolves.toBeUndefined()` for async void calls. Watch for two gotchas: (1) older smoke-test files may NOT import `expect` from `bun:test` (their bodies never asserted) — add it or you get `ReferenceError: expect is not defined` at runtime, not at lint time; (2) a Windows-only `test.skipIf(process.platform !== "win32")` test that was a no-op placeholder can often be made real by stubbing `Bun.spawn`/`Bun.which` in a `try/finally` (see `tests/helpers/git.test.ts` forcing the `wsl which git` fallback to miss). +- **oxlint `unicorn/no-array-callback-reference`** — Don't pass a bare function reference to `.map()`/`.find()`/`.filter()` etc. (e.g. `args.map(asNode)`); the rule wants the element-only contract made explicit. Wrap in an arrow: `args.map((x) => asNode(x)).find((x) => isFn(x))`. +- **oxlint `require-unicode-regexp`** — Regex literals need the `u` flag: `/Git is not installed/u`, not `/Git is not installed/`. Applies in tests too (e.g. `expect(...).rejects.toThrow(/.../u)`). - **`oxfmt` formats markdown too — run `bun run format` after editing any `.md`, including ADRs** — `format:check` (part of `bun run validate` and the CI "Lint, Test & Check" job) runs `oxfmt --check .` over ALL files, not just `.ts`. It normalizes markdown (e.g., emphasis `*word*` → `_word_`). The `adr-author` skill writes ADR markdown but does NOT auto-format it, so ADR edits frequently fail CI `format:check` even when the TS changes are clean. Always run `bun run format` before committing ADR/markdown edits. Tripped CI on PR #372 (ARCH-005 edit). - **Make large production thresholds injectable rather than building giant test fixtures** — Tests that must cross a big numeric threshold in production code (e.g., the 1000-file `SCOPE_FILE_WARN_THRESHOLD` in `src/engine/git-files.ts`) should NOT create 1000+ real files + `git add .` to trip it. On Windows CI runners that filesystem work is slow enough to exceed the per-test timeout — `git add .` gets SIGTERM'd mid-run (`git add . failed (exit 143)`, 143 = 128+SIGTERM) and the whole suite fails flakily. Fix: add an optional override (e.g., `fileWarnThreshold?: number` defaulting to the module constant) to the function under test, and have the test inject a tiny value (5) so it only needs ~6 files. Caused the `git-files > resolveScopedFiles > warns when file scope exceeds threshold` flake on PR #366's Windows smoke test. Also captured in ARCH-005. - **Never set a per-test timeout override SHORTER than the global `--timeout`** — CI runs `bun test --timeout 60000` (60s). A per-test `}, 30_000` override makes that test MORE likely to time out than the default, the opposite of the usual intent. Only override the per-test timeout to raise it above the global, never to lower it. The 30s override on the git-files threshold test was the proximate cause of its Windows flake. diff --git a/.oxlintrc.json b/.oxlintrc.json index 518f0ab9..4d26490e 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -1,4 +1,5 @@ { + "jsPlugins": ["./lint/expect-expect.ts"], "categories": { "correctness": "error", "pedantic": "error", @@ -15,6 +16,10 @@ "typescript/no-deprecated": "error" }, "overrides": [ + { + "files": ["tests/**/*.test.ts"], + "rules": { "bun-test/expect-expect": "error" } + }, { "files": [".archgate/adrs/*.rules.ts"], "rules": { "typescript/triple-slash-reference": "off" } diff --git a/lint/expect-expect.ts b/lint/expect-expect.ts new file mode 100644 index 00000000..9105e800 --- /dev/null +++ b/lint/expect-expect.ts @@ -0,0 +1,177 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2026 Archgate + +// Custom oxlint JS plugin: enforce that every runnable bun:test test/it call +// contains at least one `expect()` assertion. +// +// Why this exists: oxlint ships `jest/expect-expect`, but its Rust +// implementation only recognizes `jest` and `vitest` imports — it silently +// ignores `bun:test`. This plugin reimplements the rule for the bun:test API +// using oxlint's JS plugins API (https://oxc.rs/docs/guide/usage/linter/js-plugins.html). +// +// The plugin runs natively as TypeScript under Bun, so there is no build step. + +/** Minimal ESTree-ish node shape. The oxlint AST is ESLint-compatible. */ +type AstNode = { type: string } & Record; + +/** Narrow an unknown value to an AST node (an object with a string `type`). */ +function asNode(value: unknown): AstNode | undefined { + if ( + value !== null && + typeof value === "object" && + typeof (value as { type?: unknown }).type === "string" + ) { + return value as AstNode; + } + return undefined; +} + +/** True when the node is a function expression that can serve as a test body. */ +function isFunctionNode(node: AstNode | undefined): boolean { + return ( + node?.type === "ArrowFunctionExpression" || + node?.type === "FunctionExpression" + ); +} + +/** + * Resolve the leftmost identifier name of a callee chain. + * + * Examples (callee -> result): + * test -> "test" + * test.skip -> "test" + * test.skipIf(cond) -> "test" + * expect(x).toBe -> "expect" + */ +function leftmostName(node: AstNode | undefined): string | undefined { + let current = node; + while (current) { + switch (current.type) { + case "Identifier": { + return typeof current.name === "string" ? current.name : undefined; + } + case "MemberExpression": { + current = asNode(current.object); + break; + } + case "CallExpression": { + current = asNode(current.callee); + break; + } + default: { + return undefined; + } + } + } + return undefined; +} + +/** + * Collect the member method names used in a callee chain. + * + * Examples (callee -> result): + * test.skip -> ["skip"] + * test.skipIf(cond) -> ["skipIf"] + * test.each(rows) -> ["each"] + * test -> [] + */ +function memberMethods(node: AstNode | undefined): string[] { + const methods: string[] = []; + let current = node; + while (current) { + if (current.type === "MemberExpression") { + const property = asNode(current.property); + if ( + property?.type === "Identifier" && + typeof property.name === "string" + ) { + methods.push(property.name); + } + current = asNode(current.object); + } else if (current.type === "CallExpression") { + current = asNode(current.callee); + } else { + break; + } + } + return methods; +} + +/** Depth-first walk over an AST subtree, skipping back-references and locations. */ +function walk(node: AstNode, visit: (node: AstNode) => void): void { + visit(node); + for (const key of Object.keys(node)) { + if (key === "parent" || key === "loc" || key === "range") continue; + const value = node[key]; + if (Array.isArray(value)) { + for (const item of value) { + const child = asNode(item); + if (child) walk(child, visit); + } + } else { + const child = asNode(value); + if (child) walk(child, visit); + } + } +} + +/** True when the subtree contains a call whose callee resolves to `expect`. */ +function containsExpect(root: AstNode): boolean { + let found = false; + walk(root, (node) => { + if (found || node.type !== "CallExpression") return; + if (leftmostName(asNode(node.callee)) === "expect") found = true; + }); + return found; +} + +interface ReportDescriptor { + node: AstNode; + message: string; +} + +interface RuleContext { + report(descriptor: ReportDescriptor): void; +} + +const MESSAGE = + "Test has no assertions. Add at least one `expect()` call, or use `test.skip` / `test.todo` for an intentionally empty test."; + +const expectExpect = { + create(context: RuleContext) { + return { + CallExpression(node: AstNode) { + const callee = asNode(node.callee); + const base = leftmostName(callee); + if (base !== "test" && base !== "it") return; + + // Skip the inner `test.skipIf(cond)` call of `test.skipIf(cond)(...)`: + // only the outermost invocation carries the test callback. + const parent = asNode(node.parent); + if (parent?.type === "CallExpression" && asNode(parent.callee) === node) + return; + + // Disabled/placeholder tests are not expected to assert. + const methods = memberMethods(callee); + if (methods.includes("skip") || methods.includes("todo")) return; + + const args = Array.isArray(node.arguments) ? node.arguments : []; + const callback = args + .map((arg) => asNode(arg)) + .find((arg) => isFunctionNode(arg)); + if (!callback) return; + + if (!containsExpect(callback)) { + context.report({ node, message: MESSAGE }); + } + }, + }; + }, +}; + +const plugin = { + meta: { name: "bun-test" }, + rules: { "expect-expect": expectExpect }, +}; + +export default plugin; diff --git a/tests/helpers/auth.test.ts b/tests/helpers/auth.test.ts index a83f9687..cfce34c6 100644 --- a/tests/helpers/auth.test.ts +++ b/tests/helpers/auth.test.ts @@ -101,8 +101,7 @@ describe("auth", () => { const { clearCredentials } = await import("../../src/helpers/credential-store"); - // Should not throw - await clearCredentials(); + await expect(clearCredentials()).resolves.toBeUndefined(); }); }); diff --git a/tests/helpers/credential-store-impl.test.ts b/tests/helpers/credential-store-impl.test.ts index 7fac9c10..0b24d034 100644 --- a/tests/helpers/credential-store-impl.test.ts +++ b/tests/helpers/credential-store-impl.test.ts @@ -144,8 +144,7 @@ describe("credential-store", () => { describe("clearCredentials", () => { test("does not throw when no credentials exist", async () => { - // Should not throw - await clearCredentials(); + await expect(clearCredentials()).resolves.toBeUndefined(); }); test("cleans up legacy metadata file", async () => { diff --git a/tests/helpers/git.test.ts b/tests/helpers/git.test.ts index 2c086769..ed1997e6 100644 --- a/tests/helpers/git.test.ts +++ b/tests/helpers/git.test.ts @@ -61,14 +61,23 @@ describe("installGit", () => { test.skipIf(process.platform !== "win32")( "throws when git is not found on Windows", async () => { - // On other platforms, installGit would attempt brew/apt install instead of throwing. - await withBunWhich( - () => null, - async () => { - // Even on Windows, git is typically available so this won't reach - // the throw. This test documents the expected error for the path. - } - ); + // On other platforms, installGit would attempt brew/apt install instead + // of throwing. Force the WSL fallback (`wsl which git`) to report a miss + // so resolveCommand returns null and installGit reaches the Windows throw. + const originalSpawn = Bun.spawn; + Bun.spawn = (() => ({ + exited: Promise.resolve(1), + })) as unknown as typeof Bun.spawn; + try { + await withBunWhich( + () => null, + async () => { + await expect(installGit()).rejects.toThrow(/Git is not installed/u); + } + ); + } finally { + Bun.spawn = originalSpawn; + } } ); }); diff --git a/tests/helpers/platform.test.ts b/tests/helpers/platform.test.ts index 58c000de..746328e6 100644 --- a/tests/helpers/platform.test.ts +++ b/tests/helpers/platform.test.ts @@ -272,11 +272,13 @@ describe("_resetAllCaches", () => { }); test("clears Windows home dir cache", async () => { - // Call once to potentially populate the cache - await getWindowsHomeDirFromWSL(); - // Reset and call again — should not throw + // Call once to populate the cache. + const before = await getWindowsHomeDirFromWSL(); + // Reset and re-detect — the platform hasn't changed, so the freshly + // detected value must match the previously cached one. _resetAllCaches(); - await getWindowsHomeDirFromWSL(); + const after = await getWindowsHomeDirFromWSL(); + expect(after).toBe(before); }); test.skipIf(inWSL)( diff --git a/tests/helpers/sentry.test.ts b/tests/helpers/sentry.test.ts index 1f5c8e32..cb360720 100644 --- a/tests/helpers/sentry.test.ts +++ b/tests/helpers/sentry.test.ts @@ -1,6 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 // Copyright 2026 Archgate -import { describe, test, beforeEach, afterEach, mock } from "bun:test"; +import { describe, expect, test, beforeEach, afterEach, mock } from "bun:test"; import { mkdtempSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -47,8 +47,8 @@ describe("sentry", () => { test("initializes Sentry SDK when telemetry is enabled", async () => { const { initSentry } = await import("../../src/helpers/sentry"); - // Should not throw — Sentry.init is called internally - initSentry(); + // Sentry.init is called internally — initialization must resolve cleanly. + await expect(initSentry()).resolves.toBeUndefined(); }); test("does not initialize when telemetry is disabled", async () => { @@ -57,9 +57,11 @@ describe("sentry", () => { const { initSentry, captureException } = await import("../../src/helpers/sentry"); - initSentry(); - // captureException should be a no-op - captureException(new Error("should not send")); + await initSentry(); + // captureException should be a no-op when telemetry is disabled. + expect(() => + captureException(new Error("should not send")) + ).not.toThrow(); }); }); @@ -67,17 +69,19 @@ describe("sentry", () => { test("is a no-op when not initialized", async () => { const { captureException } = await import("../../src/helpers/sentry"); - // Should not throw - captureException(new Error("should not send")); + expect(() => + captureException(new Error("should not send")) + ).not.toThrow(); }); test("handles non-Error values without throwing", async () => { const { initSentry, captureException } = await import("../../src/helpers/sentry"); - initSentry(); - // Should not throw - captureException("string error", { command: "init" }); + await initSentry(); + expect(() => + captureException("string error", { command: "init" }) + ).not.toThrow(); }); }); @@ -85,17 +89,17 @@ describe("sentry", () => { test("is a no-op when not initialized", async () => { const { addBreadcrumb } = await import("../../src/helpers/sentry"); - // Should not throw - addBreadcrumb("test", "test breadcrumb"); + expect(() => addBreadcrumb("test", "test breadcrumb")).not.toThrow(); }); test("adds breadcrumb when initialized", async () => { const { initSentry, addBreadcrumb } = await import("../../src/helpers/sentry"); - initSentry(); - // Should not throw - addBreadcrumb("command", "Running: check", { staged: true }); + await initSentry(); + expect(() => + addBreadcrumb("command", "Running: check", { staged: true }) + ).not.toThrow(); }); }); @@ -103,17 +107,15 @@ describe("sentry", () => { test("is a no-op when not initialized", async () => { const { flushSentry } = await import("../../src/helpers/sentry"); - // Should not throw - await flushSentry(); + await expect(flushSentry()).resolves.toBeUndefined(); }); test("flushes when initialized", async () => { const { initSentry, flushSentry } = await import("../../src/helpers/sentry"); - initSentry(); - // Should not throw - await flushSentry(100); + await initSentry(); + await expect(flushSentry(100)).resolves.toBeUndefined(); }); }); }); diff --git a/tests/helpers/telemetry.test.ts b/tests/helpers/telemetry.test.ts index f7bbe491..c711c00e 100644 --- a/tests/helpers/telemetry.test.ts +++ b/tests/helpers/telemetry.test.ts @@ -82,23 +82,24 @@ describe("telemetry", () => { await import("../../src/helpers/telemetry"); await initTelemetry(); - // Should not throw — events are queued internally by the SDK (and - // suppressed entirely in test mode by trackEvent itself) - trackEvent("command_executed", { command: "check" }); + // Events are queued internally by the SDK (and suppressed entirely in + // test mode by trackEvent itself), so the contract is simply: no throw. + expect(() => + trackEvent("command_executed", { command: "check" }) + ).not.toThrow(); }); test("is a no-op when not initialized", async () => { const { trackEvent } = await import("../../src/helpers/telemetry"); - // Should not throw - trackEvent("should_not_capture"); + expect(() => trackEvent("should_not_capture")).not.toThrow(); }); test("is a no-op with no properties argument", async () => { const { trackEvent } = await import("../../src/helpers/telemetry"); // trackEvent with no properties — exercises the undefined branch - trackEvent("bare_event"); + expect(() => trackEvent("bare_event")).not.toThrow(); }); }); @@ -108,14 +109,13 @@ describe("telemetry", () => { await import("../../src/helpers/telemetry"); await initTelemetry(); - // Should not throw - trackCommand("adr create", { json: true }); + expect(() => trackCommand("adr create", { json: true })).not.toThrow(); }); test("is a no-op when not initialized", async () => { const { trackCommand } = await import("../../src/helpers/telemetry"); - trackCommand("check"); + expect(() => trackCommand("check")).not.toThrow(); }); }); @@ -125,7 +125,9 @@ describe("telemetry", () => { await import("../../src/helpers/telemetry"); await initTelemetry(); - trackCommandResult("check", 0, 120, { outcome: "success" }); + expect(() => + trackCommandResult("check", 0, 120, { outcome: "success" }) + ).not.toThrow(); }); test("handles non-zero exit code and extra properties", async () => { @@ -133,17 +135,19 @@ describe("telemetry", () => { await import("../../src/helpers/telemetry"); await initTelemetry(); - trackCommandResult("check", 1, 450, { - outcome: "user_error", - error_kind: "violations_found", - }); + expect(() => + trackCommandResult("check", 1, 450, { + outcome: "user_error", + error_kind: "violations_found", + }) + ).not.toThrow(); }); test("is a no-op when not initialized", async () => { const { trackCommandResult } = await import("../../src/helpers/telemetry"); - trackCommandResult("check", 0, 100); + expect(() => trackCommandResult("check", 0, 100)).not.toThrow(); }); }); @@ -153,20 +157,22 @@ describe("telemetry", () => { await import("../../src/helpers/telemetry"); await initTelemetry(); - trackCheckResult({ - total_rules: 5, - passed: 4, - failed: 1, - warnings: 2, - errors: 1, - rule_errors: 0, - pass: false, - output_format: "console", - used_staged: false, - used_base: false, - used_file_filter: false, - used_adr_filter: false, - }); + expect(() => + trackCheckResult({ + total_rules: 5, + passed: 4, + failed: 1, + warnings: 2, + errors: 1, + rule_errors: 0, + pass: false, + output_format: "console", + used_staged: false, + used_base: false, + used_file_filter: false, + used_adr_filter: false, + }) + ).not.toThrow(); }); test("accepts optional fields (files_scanned, durations)", async () => { @@ -174,23 +180,25 @@ describe("telemetry", () => { await import("../../src/helpers/telemetry"); await initTelemetry(); - trackCheckResult({ - total_rules: 10, - passed: 10, - failed: 0, - warnings: 0, - errors: 0, - rule_errors: 0, - pass: true, - output_format: "json", - used_staged: true, - used_base: true, - used_file_filter: true, - used_adr_filter: true, - files_scanned: 42, - load_duration_ms: 15, - check_duration_ms: 200, - }); + expect(() => + trackCheckResult({ + total_rules: 10, + passed: 10, + failed: 0, + warnings: 0, + errors: 0, + rule_errors: 0, + pass: true, + output_format: "json", + used_staged: true, + used_base: true, + used_file_filter: true, + used_adr_filter: true, + files_scanned: 42, + load_duration_ms: 15, + check_duration_ms: 200, + }) + ).not.toThrow(); }); }); @@ -200,12 +208,14 @@ describe("telemetry", () => { await import("../../src/helpers/telemetry"); await initTelemetry(); - trackInitResult({ - editor: "claude", - plugin_installed: true, - plugin_auto_installed: true, - had_existing_project: false, - }); + expect(() => + trackInitResult({ + editor: "claude", + plugin_installed: true, + plugin_auto_installed: true, + had_existing_project: false, + }) + ).not.toThrow(); }); }); @@ -215,12 +225,14 @@ describe("telemetry", () => { await import("../../src/helpers/telemetry"); await initTelemetry(); - trackUpgradeResult({ - from_version: "0.24.0", - to_version: "0.25.0", - install_method: "binary", - success: true, - }); + expect(() => + trackUpgradeResult({ + from_version: "0.24.0", + to_version: "0.25.0", + install_method: "binary", + success: true, + }) + ).not.toThrow(); }); test("accepts optional failure fields", async () => { @@ -228,14 +240,16 @@ describe("telemetry", () => { await import("../../src/helpers/telemetry"); await initTelemetry(); - trackUpgradeResult({ - from_version: "0.24.0", - to_version: "0.25.0", - install_method: "binary", - success: false, - prompted_by_update_check: true, - failure_reason: "download_failed", - }); + expect(() => + trackUpgradeResult({ + from_version: "0.24.0", + to_version: "0.25.0", + install_method: "binary", + success: false, + prompted_by_update_check: true, + failure_reason: "download_failed", + }) + ).not.toThrow(); }); }); @@ -245,7 +259,9 @@ describe("telemetry", () => { await import("../../src/helpers/telemetry"); await initTelemetry(); - trackLoginResult({ subcommand: "login", success: true }); + expect(() => + trackLoginResult({ subcommand: "login", success: true }) + ).not.toThrow(); }); test("accepts failure_reason", async () => { @@ -253,11 +269,13 @@ describe("telemetry", () => { await import("../../src/helpers/telemetry"); await initTelemetry(); - trackLoginResult({ - subcommand: "login", - success: false, - failure_reason: "network", - }); + expect(() => + trackLoginResult({ + subcommand: "login", + success: false, + failure_reason: "network", + }) + ).not.toThrow(); }); test("tracks logout subcommand", async () => { @@ -265,7 +283,9 @@ describe("telemetry", () => { await import("../../src/helpers/telemetry"); await initTelemetry(); - trackLoginResult({ subcommand: "logout", success: true }); + expect(() => + trackLoginResult({ subcommand: "logout", success: true }) + ).not.toThrow(); }); }); @@ -275,16 +295,18 @@ describe("telemetry", () => { await import("../../src/helpers/telemetry"); await initTelemetry(); - trackProjectInitialized({ - editors: ["claude"], - editor_primary: "claude", - plugin_installed: false, - had_existing_project: false, - identity_shared: false, - repo_host: "github", - repo_is_git: true, - repo_public: null, - }); + expect(() => + trackProjectInitialized({ + editors: ["claude"], + editor_primary: "claude", + plugin_installed: false, + had_existing_project: false, + identity_shared: false, + repo_host: "github", + repo_is_git: true, + repo_public: null, + }) + ).not.toThrow(); }); test("accepts identity fields when sharing", async () => { @@ -292,19 +314,21 @@ describe("telemetry", () => { await import("../../src/helpers/telemetry"); await initTelemetry(); - trackProjectInitialized({ - editors: ["claude"], - editor_primary: "claude", - plugin_installed: false, - had_existing_project: false, - identity_shared: true, - repo_host: "github", - repo_is_git: true, - repo_public: true, - remote_url: "https://github.com/archgate/cli.git", - repo_owner: "archgate", - repo_name: "cli", - }); + expect(() => + trackProjectInitialized({ + editors: ["claude"], + editor_primary: "claude", + plugin_installed: false, + had_existing_project: false, + identity_shared: true, + repo_host: "github", + repo_is_git: true, + repo_public: true, + remote_url: "https://github.com/archgate/cli.git", + repo_owner: "archgate", + repo_name: "cli", + }) + ).not.toThrow(); }); }); @@ -314,7 +338,9 @@ describe("telemetry", () => { await import("../../src/helpers/telemetry"); await initTelemetry(); - trackTelemetryPreferenceChange({ enabled: false }); + expect(() => + trackTelemetryPreferenceChange({ enabled: false }) + ).not.toThrow(); }); test("tracks re-enabling telemetry", async () => { @@ -322,7 +348,9 @@ describe("telemetry", () => { await import("../../src/helpers/telemetry"); await initTelemetry(); - trackTelemetryPreferenceChange({ enabled: true }); + expect(() => + trackTelemetryPreferenceChange({ enabled: true }) + ).not.toThrow(); }); }); @@ -332,14 +360,14 @@ describe("telemetry", () => { await import("../../src/helpers/telemetry"); await initTelemetry(); - trackGreenfieldWizardShown(); + expect(() => trackGreenfieldWizardShown()).not.toThrow(); }); test("is a no-op when not initialized", async () => { const { trackGreenfieldWizardShown } = await import("../../src/helpers/telemetry"); - trackGreenfieldWizardShown(); + expect(() => trackGreenfieldWizardShown()).not.toThrow(); }); }); @@ -350,11 +378,13 @@ describe("telemetry", () => { await initTelemetry(); // Exercises the filter logic: "packs/" prefix = official, others = third-party - trackPackImportedAtInit([ - "packs/security", - "packs/testing", - "my-custom-pack", - ]); + expect(() => + trackPackImportedAtInit([ + "packs/security", + "packs/testing", + "my-custom-pack", + ]) + ).not.toThrow(); }); test("handles empty pack list", async () => { @@ -362,7 +392,7 @@ describe("telemetry", () => { await import("../../src/helpers/telemetry"); await initTelemetry(); - trackPackImportedAtInit([]); + expect(() => trackPackImportedAtInit([])).not.toThrow(); }); test("handles all-official packs", async () => { @@ -370,14 +400,16 @@ describe("telemetry", () => { await import("../../src/helpers/telemetry"); await initTelemetry(); - trackPackImportedAtInit(["packs/security", "packs/testing"]); + expect(() => + trackPackImportedAtInit(["packs/security", "packs/testing"]) + ).not.toThrow(); }); test("is a no-op when not initialized", async () => { const { trackPackImportedAtInit } = await import("../../src/helpers/telemetry"); - trackPackImportedAtInit(["packs/foo"]); + expect(() => trackPackImportedAtInit(["packs/foo"])).not.toThrow(); }); }); @@ -387,7 +419,7 @@ describe("telemetry", () => { await import("../../src/helpers/telemetry"); await initTelemetry(); - trackWizardSkipped(); + expect(() => trackWizardSkipped()).not.toThrow(); }); }); @@ -397,11 +429,13 @@ describe("telemetry", () => { await import("../../src/helpers/telemetry"); await initTelemetry(); - trackCustomDomainAdded({ - domain_name: "security", - prefix: "SEC", - total_custom_domains: 1, - }); + expect(() => + trackCustomDomainAdded({ + domain_name: "security", + prefix: "SEC", + total_custom_domains: 1, + }) + ).not.toThrow(); }); }); @@ -411,11 +445,13 @@ describe("telemetry", () => { await import("../../src/helpers/telemetry"); await initTelemetry(); - trackCustomDomainRemoved({ - domain_name: "security", - prefix: "SEC", - total_custom_domains: 0, - }); + expect(() => + trackCustomDomainRemoved({ + domain_name: "security", + prefix: "SEC", + total_custom_domains: 0, + }) + ).not.toThrow(); }); }); @@ -426,15 +462,14 @@ describe("telemetry", () => { await initTelemetry(); - // Flush with no pending events — should resolve quickly - await flushTelemetry(); + // Flush with no pending events — should resolve quickly to undefined. + await expect(flushTelemetry()).resolves.toBeUndefined(); }); test("is a no-op when not initialized", async () => { const { flushTelemetry } = await import("../../src/helpers/telemetry"); - // Should not throw - await flushTelemetry(); + await expect(flushTelemetry()).resolves.toBeUndefined(); }); test("respects custom timeout argument", async () => { @@ -443,8 +478,8 @@ describe("telemetry", () => { await initTelemetry(); - // Short timeout — should still resolve (no pending events) - await flushTelemetry(100); + // Short timeout — should still resolve (no pending events). + await expect(flushTelemetry(100)).resolves.toBeUndefined(); }); });