From f77b096fe268ad5b085e03c96c06c6a525b255fe Mon Sep 17 00:00:00 2001 From: Dylan Boudro <121908331+starmorph@users.noreply.github.com> Date: Wed, 28 Oct 2026 16:23:49 -0700 Subject: [PATCH] Isolate tests from deployment configuration --- CONTRIBUTING.md | 5 ++++- tests/autopilot.test.ts | 3 +-- tests/cancel.test.ts | 4 +--- tests/claims.test.ts | 5 ++--- tests/fixtures/config.ts | 30 ++++++++++++++++++++++++++++++ tests/gateway.test.ts | 2 +- tests/pr-review.test.ts | 22 +++++++++++++++++----- tests/routes.test.ts | 13 ++++++++++++- tests/sandbox-refresh.test.ts | 5 ++--- tests/setup.ts | 4 ++++ tests/spec-to-pr.test.ts | 21 +++++++++++++++++---- tests/triage.test.ts | 3 +-- vitest.config.ts | 1 + 13 files changed, 93 insertions(+), 25 deletions(-) create mode 100644 tests/fixtures/config.ts create mode 100644 tests/setup.ts diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 81cd57a..7cf3996 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -62,7 +62,10 @@ npm run check ``` It runs Biome, TypeScript, and Vitest. Tests fake GitHub, Render, and model -calls. +calls. Vitest replaces `factory.config.ts` with the explicit configuration in +`tests/fixtures/config.ts`, so customizing your deployment does not change test +subjects, permissions, or expected verification commands. Keep configuration +compatibility cases in that fixture and exercise them through routing and tasks. Run additional checks when relevant: diff --git a/tests/autopilot.test.ts b/tests/autopilot.test.ts index 4d6b1d2..d56edd4 100644 --- a/tests/autopilot.test.ts +++ b/tests/autopilot.test.ts @@ -6,6 +6,7 @@ import type { GitHubClient, IssueComment } from "../app/github/index.js"; import { renderMarker } from "../app/github/index.js"; import type { SpecToPrHandoff } from "../app/workflows/spec-to-pr/index.js"; import type { TriageHandoff } from "../app/workflows/triage/spec.js"; +import { REPO } from "./fixtures/config.js"; const events: FactoryEvent[] = []; vi.mock("../app/events/index.js", () => ({ @@ -20,8 +21,6 @@ const { triageWorkflow } = await import("../app/workflows/triage/index.js"); const { specToPrWorkflow } = await import("../app/workflows/spec-to-pr/index.js"); const { prReviewWorkflow } = await import("../app/workflows/pr-review/index.js"); -const REPO = Object.keys(config.repos)[0]; -if (!REPO) throw new Error("factory.config.ts must configure at least one repository"); const SPEC_HASH = "b".repeat(64); const input = { repo: REPO, installationId: 5, issue: 12, actor: "maintainer" }; diff --git a/tests/cancel.test.ts b/tests/cancel.test.ts index b21902c..3951b7a 100644 --- a/tests/cancel.test.ts +++ b/tests/cancel.test.ts @@ -1,5 +1,6 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import type { FactoryEvent } from "../app/events/index.js"; +import { REPO } from "./fixtures/config.js"; const events: FactoryEvent[] = []; vi.mock("../app/events/index.js", () => ({ @@ -8,7 +9,6 @@ vi.mock("../app/events/index.js", () => ({ }, })); -const { config } = await import("../app/config/index.js"); const { parseMarker, renderMarker } = await import("../app/github/index.js"); const { WORKING_LABEL } = await import("../app/claims/index.js"); const { cancelRun } = await import("../app/workflows/cancel.js"); @@ -16,8 +16,6 @@ type IssueComment = import("../app/github/index.js").IssueComment; type Permission = import("../app/github/index.js").Permission; const BOT = "software-factory[bot]"; -const REPO = Object.keys(config.repos)[0]; -if (!REPO) throw new Error("factory.config.ts must configure at least one repository"); const ctx = { metadata: { taskRunId: "trn-cancel" } }; function fakeGitHub(permission: Permission) { diff --git a/tests/claims.test.ts b/tests/claims.test.ts index 44be6af..a3b5fb9 100644 --- a/tests/claims.test.ts +++ b/tests/claims.test.ts @@ -1,5 +1,6 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import type { FactoryEvent } from "../app/events/index.js"; +import { REPO as REPO_NAME } from "./fixtures/config.js"; const events: FactoryEvent[] = []; vi.mock("../app/events/index.js", () => ({ @@ -9,7 +10,7 @@ vi.mock("../app/events/index.js", () => ({ })); const { AgentError } = await import("../app/agents/index.js"); -const { config, requireRepo } = await import("../app/config/index.js"); +const { requireRepo } = await import("../app/config/index.js"); const { renderMarker, parseMarker } = await import("../app/github/index.js"); type GitHubClient = import("../app/github/index.js").GitHubClient; type IssueComment = import("../app/github/index.js").IssueComment; @@ -17,8 +18,6 @@ type Permission = import("../app/github/index.js").Permission; const { finishStatus, readStatus, takeOver, WORKING_LABEL, withClaim } = await import("../app/claims/index.js"); const BOT = "software-factory[bot]"; -const REPO_NAME = Object.keys(config.repos)[0]; -if (!REPO_NAME) throw new Error("factory.config.ts must configure at least one repository"); const REPO = requireRepo(REPO_NAME, 5); class FakeGitHub { diff --git a/tests/fixtures/config.ts b/tests/fixtures/config.ts new file mode 100644 index 0000000..0e670b2 --- /dev/null +++ b/tests/fixtures/config.ts @@ -0,0 +1,30 @@ +import type { FactoryConfig } from "../../factory.config.js"; + +export const REPO = "acme/widget"; +export const BUILD_COMMAND = "npm run build"; +export const TEST_COMMAND = "npm test"; + +/** Distinct repositories prove tasks use the selected repository's branch and command list. */ +export const REPOSITORY_CASES = [ + { repo: "acme/docs", baseBranch: "docs", verify: [] }, + { repo: "acme/api", baseBranch: "develop", verify: ["npm run check"] }, + { repo: "other/worker", baseBranch: "release/candidate", verify: ["npm run lint", "npm run typecheck", "npm test"] }, +] as const; + +/** Unit tests never read the operator's deployment settings. Keep security-sensitive expectations explicit. */ +export const testConfig: FactoryConfig = { + repos: { + [REPO]: { baseBranch: "main", install: "npm ci", verify: [BUILD_COMMAND, TEST_COMMAND] }, + ...Object.fromEntries(REPOSITORY_CASES.map(({ repo, baseBranch, verify }) => [repo, { baseBranch, verify: [...verify] }])), + }, + labels: { + triage: "factory:triage", + specApproved: "factory:spec-approved", + prReview: "factory:pr-review", + autopilot: "factory:autopilot", + }, + commandPrefix: "/factory", + minimumPermission: "write", + limits: { repairRounds: 2, maxChangedFiles: 200, maxFileBytes: 1_000_000 }, + sandbox: { timeoutSeconds: 4 * 60 * 60 }, +}; diff --git a/tests/gateway.test.ts b/tests/gateway.test.ts index d91b6f7..3d6fc9d 100644 --- a/tests/gateway.test.ts +++ b/tests/gateway.test.ts @@ -3,9 +3,9 @@ import { beforeEach, describe, expect, it, type Mock, vi } from "vitest"; import { config, type TaskName } from "../app/config/index.js"; import { TriageInput } from "../app/schemas/tasks.js"; import { createGateway, type GatewayDeps } from "../app/gateway/index.js"; +import { REPO } from "./fixtures/config.js"; const SECRET = "webhook-secret"; -const REPO = Object.keys(config.repos)[0] as string; const triagePayload = { action: "labeled", diff --git a/tests/pr-review.test.ts b/tests/pr-review.test.ts index 70708c4..823b172 100644 --- a/tests/pr-review.test.ts +++ b/tests/pr-review.test.ts @@ -1,5 +1,6 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import type { FactoryEvent } from "../app/events/index.js"; +import { BUILD_COMMAND, REPO, REPOSITORY_CASES, TEST_COMMAND } from "./fixtures/config.js"; const events: FactoryEvent[] = []; vi.mock("../app/events/index.js", () => ({ @@ -9,7 +10,6 @@ vi.mock("../app/events/index.js", () => ({ })); const { AgentError } = await import("../app/agents/index.js"); -const { config } = await import("../app/config/index.js"); const { GitHubError, parseMarker, renderMarker } = await import("../app/github/index.js"); const { runPrReview } = await import("../app/workflows/pr-review/index.js"); type GitHubClient = import("../app/github/index.js").GitHubClient; @@ -26,10 +26,6 @@ type RunAgent = typeof import("../app/agents/index.js").runAgent; type PrReviewDeps = import("../app/workflows/pr-review/index.js").PrReviewDeps; const BOT = "software-factory[bot]"; -const REPO = Object.keys(config.repos)[0]; -if (!REPO) throw new Error("factory.config.ts must configure at least one repository"); -const [BUILD_COMMAND, TEST_COMMAND] = config.repos[REPO].verify; -if (!BUILD_COMMAND || !TEST_COMMAND) throw new Error("test fixture requires two verify commands"); const HEAD = "a".repeat(40); const MERGE_BASE = "b".repeat(40); const ctx = { metadata: { taskRunId: "trn-run", rootTaskRunId: "trn-root" } }; @@ -384,6 +380,22 @@ describe("runPrReview", () => { expect(gh.status()).toContain("pullrequestreview-9"); }); + it.each(REPOSITORY_CASES)("reviews $repo on $baseBranch with its own verification commands", async ({ repo, baseBranch, verify }) => { + gh.pull = { ...gh.pull, baseRef: baseBranch }; + const result = await runPrReview(ctx, { ...input, repo }, deps()); + + expect(result).toMatchObject({ state: "done", outcome: "reviewed" }); + expect(opened).toEqual([{ kind: "pull", number: 34, headSha: HEAD, baseBranch }]); + expect(sandbox.execs.map((entry) => entry.command)).toEqual([`git merge-base 'refs/remotes/origin/${baseBranch}' HEAD`, ...verify]); + expect(sandbox.execs.every((entry) => entry.opts?.asRunner)).toBe(true); + const message = agents.call("pr-reviewer").message; + expect(message).toContain(`# Pull request #34 in ${repo}`); + if (verify.length === 0) expect(message).toContain("No verify commands are configured."); + for (const command of verify) expect(message).toContain(`\`${command}\`: passed`); + expect(gh.posted[0]).toMatchObject({ commitId: HEAD, event: "COMMENT" }); + expect(sandbox.closed).toBe(1); + }); + it("keeps an autopilot review's status and sandbox on the workflow issue", async () => { await gh.createComment( 12, diff --git a/tests/routes.test.ts b/tests/routes.test.ts index 4e03102..dd0034a 100644 --- a/tests/routes.test.ts +++ b/tests/routes.test.ts @@ -2,8 +2,8 @@ import { describe, expect, it } from "vitest"; import { config } from "../app/config/index.js"; import { AutopilotInput, PrReviewInput, SpecToPrInput, TriageInput } from "../app/schemas/tasks.js"; import { route } from "../app/gateway/routes.js"; +import { REPO, REPOSITORY_CASES } from "./fixtures/config.js"; -const REPO = Object.keys(config.repos)[0] as string; const BASE = config.repos[REPO]?.baseBranch as string; const DELIVERY = "delivery-1"; const SHA = "a".repeat(40); @@ -113,6 +113,17 @@ describe("route: starts", () => { }); }); + it.each(REPOSITORY_CASES)("routes $repo only from its own base branch $baseBranch", ({ repo, baseBranch }) => { + const payload = push({ repository: { full_name: repo }, ref: `refs/heads/${baseBranch}` }); + expect(route("push", payload, DELIVERY)).toEqual({ + kind: "start", + task: "refresh-snapshot", + input: { repo, installationId: 7, sha: SHA }, + idempotencyKey: `${repo}@${SHA}`, + }); + expect(route("push", { ...payload, ref: "refs/heads/main" }, DELIVERY).kind).toBe("ignore"); + }); + it("push to the base branch starts refresh-snapshot keyed by repo@sha", () => { const r = route("push", push(), DELIVERY); expect(r).toEqual({ diff --git a/tests/sandbox-refresh.test.ts b/tests/sandbox-refresh.test.ts index 518e94a..1739570 100644 --- a/tests/sandbox-refresh.test.ts +++ b/tests/sandbox-refresh.test.ts @@ -1,5 +1,6 @@ import type { TaskContext } from "@renderinc/sdk/workflows"; import { describe, expect, it, vi } from "vitest"; +import { REPO } from "./fixtures/config.js"; const buildSnapshot = vi.fn(async (_client: unknown, repo: { fullName: string }, sha: string) => ({ id: "snp-1", @@ -10,12 +11,10 @@ const buildSnapshot = vi.fn(async (_client: unknown, repo: { fullName: string }, vi.mock("../app/sandbox/index.js", () => ({ buildSnapshot, sandboxClient: async () => ({ fake: true }) })); const { refreshSnapshot } = await import("../app/workflows/refresh-snapshot.js"); -const { config, TASKS } = await import("../app/config/index.js"); +const { TASKS } = await import("../app/config/index.js"); const ctx = { metadata: {}, run: async () => undefined } as unknown as TaskContext; const SHA = "a".repeat(40); -const REPO = Object.keys(config.repos)[0]; -if (!REPO) throw new Error("factory.config.ts must configure at least one repository"); describe("refresh-snapshot task", () => { it("registers under its task name and builds the configured repository at the pushed commit", async () => { diff --git a/tests/setup.ts b/tests/setup.ts new file mode 100644 index 0000000..61fb8a5 --- /dev/null +++ b/tests/setup.ts @@ -0,0 +1,4 @@ +import { vi } from "vitest"; + +// Mock the deployment file, not app/config: repository resolution and permission checks remain real. +vi.mock("../factory.config.js", async () => ({ default: (await import("./fixtures/config.js")).testConfig })); diff --git a/tests/spec-to-pr.test.ts b/tests/spec-to-pr.test.ts index 32e64d3..def3413 100644 --- a/tests/spec-to-pr.test.ts +++ b/tests/spec-to-pr.test.ts @@ -1,5 +1,6 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import type { FactoryEvent } from "../app/events/index.js"; +import { BUILD_COMMAND, REPO, REPOSITORY_CASES, TEST_COMMAND } from "./fixtures/config.js"; const events: FactoryEvent[] = []; vi.mock("../app/events/index.js", () => ({ @@ -26,10 +27,6 @@ type PublishResult = import("../app/changeset/index.js").PublishResult; const BOT = "software-factory[bot]"; const BOT_ID = 4242; -const REPO = Object.keys(config.repos)[0]; -if (!REPO) throw new Error("factory.config.ts must configure at least one repository"); -const [BUILD_COMMAND, TEST_COMMAND] = config.repos[REPO].verify; -if (!BUILD_COMMAND || !TEST_COMMAND) throw new Error("test fixture requires two verify commands"); const BASE = "b".repeat(40); const PR_HEAD = "c".repeat(40); const ISSUE = 12; @@ -292,6 +289,22 @@ describe("implement, verify, and publish", () => { expect(t.sandbox.close).toHaveBeenCalledOnce(); }); + it.each(REPOSITORY_CASES)("publishes $repo against $baseBranch with its own verification commands", async ({ repo, baseBranch, verify }) => { + const t = setup({ input: { ...input, repo } }); + const outcome = await t.run(); + + expect(outcome).toMatchObject({ state: "done", outcome: "pr_opened" }); + expect(t.openSandbox).toHaveBeenCalledWith(expect.objectContaining({ fullName: repo }), { kind: "branch", name: baseBranch }); + expect(verifyCommands(t.execs)).toEqual(verify); + expect(t.publish).toHaveBeenCalledWith(expect.objectContaining({ repo: expect.objectContaining({ fullName: repo }) })); + expect(t.gh.ensurePull).toHaveBeenCalledWith(expect.objectContaining({ base: baseBranch, draft: true })); + const message = t.calls[0]?.message ?? ""; + expect(message).toContain(`issue #${ISSUE} in ${repo}`); + if (verify.length === 0) expect(message).toContain("none configured"); + for (const command of verify) expect(message).toContain(`- \`${command}\``); + expect(t.sandbox.close).toHaveBeenCalledOnce(); + }); + it("repairs a failing verify and publishes once it passes", async () => { const t = setup({ sandbox: { verify: { [TEST_COMMAND]: [fail("FAIL src/server.test.ts\nexpected 200, got 404"), { exitCode: 0, stdout: "", stderr: "" }] } }, diff --git a/tests/triage.test.ts b/tests/triage.test.ts index f0a5162..83bec37 100644 --- a/tests/triage.test.ts +++ b/tests/triage.test.ts @@ -1,5 +1,6 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import type { FactoryEvent } from "../app/events/index.js"; +import { REPO } from "./fixtures/config.js"; const events: FactoryEvent[] = []; vi.mock("../app/events/index.js", () => ({ @@ -24,8 +25,6 @@ type ExecOptions = import("../app/sandbox/index.js").ExecOptions; type SandboxRef = import("../app/sandbox/index.js").SandboxRef; const BOT = "software-factory[bot]"; -const REPO = Object.keys(config.repos)[0]; -if (!REPO) throw new Error("factory.config.ts must configure at least one repository"); const APPROVED = config.labels.specApproved; const ctx = { metadata: { taskRunId: "trn-triage", rootTaskRunId: "trn-triage" } }; const input = { repo: REPO, installationId: 5, issue: 12, actor: "maintainer" }; diff --git a/vitest.config.ts b/vitest.config.ts index 5bdee00..a77db7e 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -3,5 +3,6 @@ import { defineConfig } from "vitest/config"; export default defineConfig({ test: { include: ["tests/**/*.test.ts"], + setupFiles: ["tests/setup.ts"], }, });