feat(checks): add TODOs & stubs check, replace noop template
/pygienium-todos inventories TODO/FIXME/HACK markers and stub bodies via a deterministic pre-scan plus the todos sub-agent; --fix converts silent stubs (placeholder returns, empty/pass bodies) into loud failures, never implementing TODOs or deleting markers. Replaces the noop reference check; the extensibility suite now registers a synthetic witness. Adds agents/todos.md and tests/todos.test.ts.
This commit is contained in:
@@ -1,38 +1,62 @@
|
||||
/**
|
||||
* extensibility.test.ts — the registry extensibility claim (task 14).
|
||||
*
|
||||
* Proves a NEW check added as a file in `src/checks/` plus one `registerCheck()`
|
||||
* entry yields a working `/pygienium-<name>` command with ZERO `index.ts`
|
||||
* command-wiring changes. The witness is `src/checks/noop.ts`: importing it
|
||||
* self-registers the `noop` check, after which the generic command-binding path
|
||||
* (`registerPygieniumCommands`, the exact function `index.ts` calls) exposes
|
||||
* `/pygienium-noop` and `/pygienium-help` lists it.
|
||||
* Proves a NEW check registered via the public API yields a working
|
||||
* `/pygienium-<name>` command with ZERO `index.ts` command-wiring changes: an
|
||||
* in-test `registerCheck()` call makes the generic command-binding path
|
||||
* (`registerPygieniumCommands`, the exact function `index.ts` calls) expose
|
||||
* `/pygienium-witness` and `/pygienium-help` lists it. The shipped checks are
|
||||
* each exercised by their own test files, so this suite only needs a synthetic
|
||||
* witness.
|
||||
*/
|
||||
import { describe, expect, it } from "bun:test";
|
||||
import "../src/checks/noop.js";
|
||||
import { getCheck, getAllChecks } from "../src/checks/registry.js";
|
||||
import { describe, expect, it, afterEach } from "bun:test";
|
||||
import {
|
||||
clearChecks,
|
||||
getCheck,
|
||||
getAllChecks,
|
||||
registerCheck,
|
||||
type CheckDefinition,
|
||||
} from "../src/checks/registry.js";
|
||||
import { registerPygieniumCommands } from "../src/commands.js";
|
||||
import { buildPygieniumHelpLines } from "../src/help.js";
|
||||
|
||||
/** A synthetic check registered only for this suite. */
|
||||
const witnessCheck: CheckDefinition = {
|
||||
name: "witness",
|
||||
label: "Witness",
|
||||
description: "Test-only check proving zero-wiring extensibility.",
|
||||
agentName: "scanner",
|
||||
fixAgentName: "fixer",
|
||||
phaseId: "witness",
|
||||
buildScanTask: () =>
|
||||
"# Task: witness scan\nwrite findings.md: witness: 0 issues",
|
||||
buildFixTask: () => "# Task: witness fix\nwrite changes.md: witness: 0 edits",
|
||||
gate: () => undefined,
|
||||
};
|
||||
|
||||
describe("registry extensibility (task 14)", () => {
|
||||
it("the noop check file self-registers (no index.ts edits)", () => {
|
||||
// Importing checks/noop.ts ran its top-level registerCheck(noopCheck).
|
||||
expect(getCheck("noop")).toBeDefined();
|
||||
expect(getAllChecks().some((c) => c.name === "noop")).toBe(true);
|
||||
afterEach(() => clearChecks());
|
||||
|
||||
it("a registered check is visible via getCheck/getAllChecks", () => {
|
||||
registerCheck(witnessCheck);
|
||||
expect(getCheck("witness")).toBe(witnessCheck);
|
||||
expect(getAllChecks().some((c) => c.name === "witness")).toBe(true);
|
||||
});
|
||||
|
||||
it("registerPygieniumCommands exposes /pygienium-noop (zero wiring)", () => {
|
||||
it("registerPygieniumCommands exposes /pygienium-<name> (zero wiring)", () => {
|
||||
registerCheck(witnessCheck);
|
||||
const names: string[] = [];
|
||||
registerPygieniumCommands((name) => names.push(name));
|
||||
expect(names).toContain("pygienium-noop");
|
||||
expect(names).toContain("pygienium-witness");
|
||||
// And the operator commands are still wired.
|
||||
expect(names).toContain("pygienium-help");
|
||||
expect(names).toContain("pygienium-all");
|
||||
});
|
||||
|
||||
it("/pygienium-help lists the noop check", () => {
|
||||
it("/pygienium-help lists a registered check", () => {
|
||||
registerCheck(witnessCheck);
|
||||
const text = buildPygieniumHelpLines().join("\n");
|
||||
expect(text).toContain("/pygienium-noop");
|
||||
expect(text).toContain(getCheck("noop")!.description);
|
||||
expect(text).toContain("/pygienium-witness");
|
||||
expect(text).toContain(witnessCheck.description);
|
||||
});
|
||||
});
|
||||
|
||||
337
tests/todos.test.ts
Normal file
337
tests/todos.test.ts
Normal file
@@ -0,0 +1,337 @@
|
||||
/**
|
||||
* todos.test.ts — unit + integration tests for the todos (TODOs & stubs) check.
|
||||
*
|
||||
* Unit: `detectTodoStubs` over a multi-language fixture tree — markers, silent
|
||||
* stubs (placeholder return / empty body / pass-only body), loud stubs, and
|
||||
* the negative cases (a real adder, a `return null` catch handler, in-string
|
||||
* "TODO" flagged for recall, clean code never flagged).
|
||||
*
|
||||
* Integration (deterministic fake agent runner): the scan persists findings.md
|
||||
* with the three sections + machine-readable summary and a new/resolved delta
|
||||
* vs the previous run; --fix converts silent stubs to loud throws, preserves
|
||||
* markers, leaves loud stubs untouched, and records changes.md.
|
||||
*/
|
||||
import { describe, expect, it, beforeEach, afterEach } from "bun:test";
|
||||
import { existsSync } from "node:fs";
|
||||
import { mkdtemp, mkdir, rm, writeFile, readFile } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import {
|
||||
clearChecks,
|
||||
getCheck,
|
||||
registerCheck,
|
||||
type CheckDefinition,
|
||||
} from "../src/checks/registry.js";
|
||||
import {
|
||||
setAgentRunner,
|
||||
resetAgentRunner,
|
||||
fakeAgentRunner,
|
||||
} from "../src/agent-runner.js";
|
||||
import { handleCheckCommand, type PygieniumCtx } from "../src/commands.js";
|
||||
import { loadRunState } from "../src/run-state.js";
|
||||
import {
|
||||
findingsPath,
|
||||
changesPath,
|
||||
todosCheck,
|
||||
detectTodoStubs,
|
||||
todosPriorCounts,
|
||||
buildTodosScanTask,
|
||||
} from "../src/checks/todos.js";
|
||||
|
||||
function stubCtx(cwd: string): PygieniumCtx {
|
||||
return { cwd, mode: "print", hasUI: false, ui: undefined } as PygieniumCtx;
|
||||
}
|
||||
|
||||
/**
|
||||
* Seed `stubs.ts` with the full taxonomy: a TODO marker, a silent stub with a
|
||||
* placeholder return (getPrice), a silent stub with an empty body (notify),
|
||||
* and a loud stub (connect throws "Not implemented").
|
||||
*/
|
||||
async function seedStubs(dir: string): Promise<void> {
|
||||
await mkdir(dir, { recursive: true }).catch(() => {});
|
||||
await writeFile(
|
||||
join(dir, "stubs.ts"),
|
||||
[
|
||||
`// TODO: add pagination`,
|
||||
`export function getPrice(): number {`,
|
||||
` return 0;`,
|
||||
`}`,
|
||||
``,
|
||||
`export function notify(): void {}`,
|
||||
``,
|
||||
`export function connect(): Promise<void> {`,
|
||||
` throw new Error("Not implemented");`,
|
||||
`}`,
|
||||
``,
|
||||
].join("\n"),
|
||||
"utf8",
|
||||
);
|
||||
}
|
||||
|
||||
/** Seed the multi-language fixture tree for the deterministic-detector tests. */
|
||||
async function seedTree(dir: string): Promise<void> {
|
||||
await mkdir(dir, { recursive: true }).catch(() => {});
|
||||
await Promise.all([
|
||||
writeFile(
|
||||
join(dir, "math.ts"),
|
||||
"export function add(a: number, b: number): number {\n return a + b;\n}\n",
|
||||
"utf8",
|
||||
),
|
||||
writeFile(
|
||||
join(dir, "parse.ts"),
|
||||
[
|
||||
`export function parse(input: string) {`,
|
||||
` try {`,
|
||||
` return JSON.parse(input);`,
|
||||
` } catch {`,
|
||||
` return null;`,
|
||||
` }`,
|
||||
`}`,
|
||||
``,
|
||||
].join("\n"),
|
||||
"utf8",
|
||||
),
|
||||
writeFile(join(dir, "stringlit.ts"), 'export const op = "TODO";\n', "utf8"),
|
||||
writeFile(
|
||||
join(dir, "stubs.ts"),
|
||||
[
|
||||
`// TODO: add pagination`,
|
||||
`export function getPrice(): number {`,
|
||||
` return 0;`,
|
||||
`}`,
|
||||
``,
|
||||
`export function notify(): void {}`,
|
||||
``,
|
||||
`export function connect(): Promise<void> {`,
|
||||
` throw new Error("Not implemented");`,
|
||||
`}`,
|
||||
``,
|
||||
].join("\n"),
|
||||
"utf8",
|
||||
),
|
||||
writeFile(
|
||||
join(dir, "repo.py"),
|
||||
[
|
||||
`class Repository:`,
|
||||
` def find(self, uid):`,
|
||||
` raise NotImplementedError # interface method`,
|
||||
``,
|
||||
` def save(self, record):`,
|
||||
` pass`,
|
||||
``,
|
||||
].join("\n"),
|
||||
"utf8",
|
||||
),
|
||||
writeFile(
|
||||
join(dir, "fetch.rs"),
|
||||
"fn fetch() -> Result<u32, String> {\n todo!()\n}\n",
|
||||
"utf8",
|
||||
),
|
||||
]);
|
||||
}
|
||||
|
||||
describe("detectTodoStubs", () => {
|
||||
let dir: string;
|
||||
beforeEach(async () => {
|
||||
dir = await mkdtemp(join(tmpdir(), "pygienium-todos-unit-"));
|
||||
await seedTree(dir);
|
||||
});
|
||||
afterEach(async () => {
|
||||
await rm(dir, { recursive: true, force: true }).catch(() => {});
|
||||
});
|
||||
|
||||
it("flags markers, silent stubs, and loud stubs across languages", async () => {
|
||||
const hits = await detectTodoStubs(dir);
|
||||
const marker = hits.filter((h) => h.kind === "marker");
|
||||
const silent = hits.filter((h) => h.kind === "silent-stub");
|
||||
const loud = hits.filter((h) => h.kind === "loud-stub");
|
||||
|
||||
// Clean code is never flagged: the adder and the `return null` catch
|
||||
// handler (a boundary handler, not a stub) produce zero hits.
|
||||
expect(
|
||||
hits.filter((h) => /(?:math\.ts|parse\.ts)$/.test(h.path)),
|
||||
).toHaveLength(0);
|
||||
|
||||
// Markers: the stubs.ts TODO comment, and the in-string "TODO" (recall —
|
||||
// the scan agent's job is to drop the string-literal noise).
|
||||
expect(
|
||||
marker.some(
|
||||
(h) =>
|
||||
h.path.endsWith("stubs.ts") && h.line === 1 && h.snippet === "TODO",
|
||||
),
|
||||
).toBe(true);
|
||||
expect(
|
||||
marker.some(
|
||||
(h) => h.path.endsWith("stringlit.ts") && h.snippet === "TODO",
|
||||
),
|
||||
).toBe(true);
|
||||
|
||||
// Silent stubs: lone placeholder return + empty body in stubs.ts,
|
||||
// pass-only body in repo.py — each with its enclosing function.
|
||||
const stubsSilent = silent.filter((h) => h.path.endsWith("stubs.ts"));
|
||||
expect(
|
||||
stubsSilent.some(
|
||||
(h) => h.snippet === "placeholder-return" && h.context === "getPrice",
|
||||
),
|
||||
).toBe(true);
|
||||
expect(
|
||||
stubsSilent.some(
|
||||
(h) => h.snippet === "empty-body" && h.context === "notify",
|
||||
),
|
||||
).toBe(true);
|
||||
expect(
|
||||
silent.some(
|
||||
(h) =>
|
||||
h.path.endsWith("repo.py") &&
|
||||
h.snippet === "pass-only" &&
|
||||
h.context === "save",
|
||||
),
|
||||
).toBe(true);
|
||||
|
||||
// Loud stubs: "Not implemented" throw, raise NotImplementedError,
|
||||
// rust todo!() — reported as tracked debt.
|
||||
expect(
|
||||
loud.some(
|
||||
(h) => h.path.endsWith("stubs.ts") && h.snippet === "Not implemented",
|
||||
),
|
||||
).toBe(true);
|
||||
expect(
|
||||
loud.some(
|
||||
(h) =>
|
||||
h.path.endsWith("repo.py") && h.snippet === "NotImplementedError",
|
||||
),
|
||||
).toBe(true);
|
||||
expect(
|
||||
loud.some((h) => h.path.endsWith("fetch.rs") && h.snippet === "todo!("),
|
||||
).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("todos check", () => {
|
||||
let cwd: string;
|
||||
|
||||
beforeEach(async () => {
|
||||
clearChecks();
|
||||
setAgentRunner(fakeAgentRunner);
|
||||
// Re-register explicitly: the module's import-time registerCheck only
|
||||
// runs once (module cache), so clearChecks + registerCheck restores it
|
||||
// deterministically for each test.
|
||||
registerCheck(todosCheck);
|
||||
cwd = await mkdtemp(join(tmpdir(), "pygienium-todos-"));
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
resetAgentRunner();
|
||||
await rm(cwd, { recursive: true, force: true }).catch(() => {});
|
||||
});
|
||||
|
||||
it("is registered and uses the todos scanner agent", () => {
|
||||
const check = getCheck("todos");
|
||||
expect(check).toBeDefined();
|
||||
expect((check as CheckDefinition)?.agentName).toBe("todos");
|
||||
});
|
||||
|
||||
it("scan-only writes findings.md with the three sections and leaves sources untouched", async () => {
|
||||
await seedStubs(cwd);
|
||||
const check = getCheck("todos")!;
|
||||
|
||||
await handleCheckCommand(check, "", stubCtx(cwd));
|
||||
|
||||
const findings = await readFile(findingsPath(cwd), "utf8");
|
||||
expect(findings).toContain(
|
||||
"summary: 1 marker(s), 2 silent stub(s), 1 loud stub(s)",
|
||||
);
|
||||
expect(findings).toContain("new: 4"); // first run: everything is new
|
||||
expect(findings).toContain("## TODO markers");
|
||||
expect(findings).toContain("## Silent stubs (actionable)");
|
||||
expect(findings).toContain("placeholder-return");
|
||||
expect(findings).toContain("Not implemented");
|
||||
|
||||
// Scan-only: no changes.md, sources untouched.
|
||||
expect(existsSync(changesPath(cwd))).toBe(false);
|
||||
const untouched = await readFile(join(cwd, "stubs.ts"), "utf8");
|
||||
expect(untouched).toContain("return 0;");
|
||||
|
||||
// Run state records the scan summary as findings text.
|
||||
const state = await loadRunState(cwd);
|
||||
expect(state?.checks["todos"]?.status).toBe("complete");
|
||||
expect(state?.checks["todos"]?.findings).toContain(
|
||||
"todos: 2 silent stub(s), 1 loud stub(s), 1 marker(s)",
|
||||
);
|
||||
});
|
||||
|
||||
it("--fix converts silent stubs to loud throws, preserves markers and loud stubs, records changes.md", async () => {
|
||||
await seedStubs(cwd);
|
||||
const check = getCheck("todos")!;
|
||||
|
||||
await handleCheckCommand(check, "--fix", stubCtx(cwd));
|
||||
|
||||
const cleaned = await readFile(join(cwd, "stubs.ts"), "utf8");
|
||||
// Silent stubs now throw loudly, naming the function.
|
||||
expect(cleaned).toContain('throw new Error("todos: getPrice() is a stub")');
|
||||
expect(cleaned).toContain('throw new Error("todos: notify() is a stub")');
|
||||
// Markers are NEVER deleted and loud stubs are NEVER touched.
|
||||
expect(cleaned).toContain("// TODO: add pagination");
|
||||
expect(cleaned).toContain('throw new Error("Not implemented")');
|
||||
expect(cleaned).toContain("Cleaned by pygienium-todos");
|
||||
// The placeholder bodies are gone.
|
||||
expect(cleaned).not.toContain("return 0;");
|
||||
expect(cleaned).not.toContain("notify(): void {}");
|
||||
|
||||
const changes = await readFile(changesPath(cwd), "utf8");
|
||||
expect(changes).toContain(
|
||||
"summary: 2 silent stub(s) converted to loud, 0 kept",
|
||||
);
|
||||
expect(changes).toContain("getPrice()");
|
||||
expect(changes).toContain("notify()");
|
||||
|
||||
const state = await loadRunState(cwd);
|
||||
expect(state?.checks["todos"]?.fix).toBe(true);
|
||||
expect(state?.checks["todos"]?.status).toBe("complete");
|
||||
});
|
||||
|
||||
it("findings.md and changes.md live under .pygienium/checks/todos/", async () => {
|
||||
await seedStubs(cwd);
|
||||
const check = getCheck("todos")!;
|
||||
await handleCheckCommand(check, "--fix", stubCtx(cwd));
|
||||
expect(findingsPath(cwd)).toBe(
|
||||
join(cwd, ".pygienium", "checks", "todos", "findings.md"),
|
||||
);
|
||||
expect(changesPath(cwd)).toBe(
|
||||
join(cwd, ".pygienium", "checks", "todos", "changes.md"),
|
||||
);
|
||||
});
|
||||
|
||||
it("skips when the target has no source files", async () => {
|
||||
// cwd is a fresh empty tempdir (no seeding) → the gate finds no source
|
||||
// files and skips the check without spawning an agent.
|
||||
const check = getCheck("todos")!;
|
||||
await handleCheckCommand(check, "", stubCtx(cwd));
|
||||
const state = await loadRunState(cwd);
|
||||
expect(state?.checks["todos"]?.status).toBe("skipped");
|
||||
});
|
||||
|
||||
it("reports a new/resolved delta against the previous run's counts", async () => {
|
||||
await seedStubs(cwd);
|
||||
const check = getCheck("todos")!;
|
||||
await handleCheckCommand(check, "", stubCtx(cwd));
|
||||
|
||||
// The previous run's verified counts are parseable from run-state.
|
||||
const prior = await todosPriorCounts(cwd);
|
||||
expect(prior).toEqual({ silent: 2, loud: 1, marker: 1 });
|
||||
|
||||
// Nobody flagged anything new and everything was resolved: the next scan
|
||||
// sees zero candidates → new: 0, resolved: 4 (all prior items).
|
||||
await rm(join(cwd, "stubs.ts"));
|
||||
const task = await buildTodosScanTask(cwd, {
|
||||
cwd,
|
||||
target: cwd,
|
||||
fix: false,
|
||||
rest: [],
|
||||
});
|
||||
expect(task).toContain(
|
||||
"summary: 0 marker(s), 0 silent stub(s), 0 loud stub(s)",
|
||||
);
|
||||
expect(task).toContain("| new: 0 | resolved: 4 |");
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user