From 6444ddb87e634c1ddd79ec905ca6c43d049a7143 Mon Sep 17 00:00:00 2001 From: Dotta Date: Thu, 10 Sep 2026 08:03:20 -0500 Subject: [PATCH] test(server): install route mocks before loading services Keep service mocks hoisted and reuse the loaded route modules across cases. Include captured route exceptions in failed status assertions so intermittent HTTP 500s are diagnosable. Co-authored-by: Paperclip --- .../__tests__/external-object-routes.test.ts | 121 +++++++++--------- .../sidebar-preferences-routes.test.ts | 34 +++-- 2 files changed, 75 insertions(+), 80 deletions(-) diff --git a/server/src/__tests__/external-object-routes.test.ts b/server/src/__tests__/external-object-routes.test.ts index 9523fe66d0..7f5ef38e88 100644 --- a/server/src/__tests__/external-object-routes.test.ts +++ b/server/src/__tests__/external-object-routes.test.ts @@ -32,54 +32,52 @@ const mockInstanceSettingsService = vi.hoisted(() => ({ getExperimental: vi.fn(), })); -function registerRouteMocks() { - vi.doMock("../services/external-objects.js", () => ({ - externalObjectService: () => mockExternalObjectsService, - })); +vi.mock("../services/external-objects.js", () => ({ + externalObjectService: () => mockExternalObjectsService, +})); - vi.doMock("../services/instance-settings.js", () => ({ - instanceSettingsService: () => mockInstanceSettingsService, - })); +vi.mock("../services/instance-settings.js", () => ({ + instanceSettingsService: () => mockInstanceSettingsService, +})); - vi.doMock("../services/task-watchdog-scope.js", () => ({ - TASK_WATCHDOG_ORIGIN_KIND: "task_watchdog", - resolveTaskWatchdogMutationScope: vi.fn(async () => ({ kind: "none" })), - taskWatchdogScopeAllowsIssueMutation: vi.fn(async () => ({ kind: "none" })), - })); +vi.mock("../services/task-watchdog-scope.js", () => ({ + TASK_WATCHDOG_ORIGIN_KIND: "task_watchdog", + resolveTaskWatchdogMutationScope: vi.fn(async () => ({ kind: "none" })), + taskWatchdogScopeAllowsIssueMutation: vi.fn(async () => ({ kind: "none" })), +})); - vi.doMock("../services/index.js", () => ({ - accessService: () => mockAccessService, - agentService: () => mockAgentService, - companySkillService: () => ({}), - companyService: () => ({ - getById: vi.fn(async () => null), - }), - companySearchService: () => ({}), - documentAnnotationService: () => ({}), - documentService: () => ({}), - executionWorkspaceService: () => ({}), - feedbackService: () => ({}), - goalService: () => ({}), - heartbeatService: () => ({ - wakeup: vi.fn(async () => undefined), - reportRunActivity: vi.fn(async () => undefined), - getRun: vi.fn(async () => null), - getActiveRunForAgent: vi.fn(async () => null), - cancelRun: vi.fn(async () => null), - }), - issueApprovalService: () => ({}), - issueRecoveryActionService: () => ({}), - issueReferenceService: () => ({ - listIssueReferenceSummary: async () => ({ outbound: [], inbound: [] }), - }), - issueService: () => mockIssueService, - issueThreadInteractionService: () => ({}), - logActivity: vi.fn(async () => undefined), - projectService: () => ({}), - routineService: () => ({}), - workProductService: () => ({}), - })); -} +vi.mock("../services/index.js", () => ({ + accessService: () => mockAccessService, + agentService: () => mockAgentService, + companySkillService: () => ({}), + companyService: () => ({ + getById: vi.fn(async () => null), + }), + companySearchService: () => ({}), + documentAnnotationService: () => ({}), + documentService: () => ({}), + executionWorkspaceService: () => ({}), + feedbackService: () => ({}), + goalService: () => ({}), + heartbeatService: () => ({ + wakeup: vi.fn(async () => undefined), + reportRunActivity: vi.fn(async () => undefined), + getRun: vi.fn(async () => null), + getActiveRunForAgent: vi.fn(async () => null), + cancelRun: vi.fn(async () => null), + }), + issueApprovalService: () => ({}), + issueRecoveryActionService: () => ({}), + issueReferenceService: () => ({ + listIssueReferenceSummary: async () => ({ outbound: [], inbound: [] }), + }), + issueService: () => mockIssueService, + issueThreadInteractionService: () => ({}), + logActivity: vi.fn(async () => undefined), + projectService: () => ({}), + routineService: () => ({}), + workProductService: () => ({}), +})); function makeIssue(overrides: Record = {}) { return { @@ -118,6 +116,12 @@ async function createApp(actor: Express.Request["actor"]) { next(); }); app.use("/api", issueRoutes(routeDb as any, { provider: "local_disk" } as any)); + // Keep unexpected route exceptions visible when a status assertion fails. + app.locals.routeErrors = [] as string[]; + app.use((error: unknown, _req: express.Request, _res: express.Response, next: express.NextFunction) => { + app.locals.routeErrors.push(error instanceof Error ? error.stack ?? error.message : String(error)); + next(error); + }); app.use(errorHandler); return app; } @@ -158,21 +162,14 @@ function peerActor(): Express.Request["actor"] { } describe("external object routes", () => { - // Load the real route and middleware modules once before the tests run. The - // first import transforms a large module graph. Under the loaded serial shard - // (maxWorkers=1) that cold cost crossed the 5s testTimeout of the first test. - // The hook has a 30s budget, so it absorbs the transform cost and every later - // createApp() call hits the cached modules. + // Hoisted service mocks must be installed before this import. Warm the real + // route module once within the hook budget; individual tests reset service + // behavior without rebuilding the dependency graph or starting real services. beforeAll(async () => { await createApp(boardActor()); }); beforeEach(() => { - vi.resetModules(); - vi.doUnmock("../routes/issues.js"); - vi.doUnmock("../services/index.js"); - vi.doUnmock("../services/external-objects.js"); - registerRouteMocks(); vi.resetAllMocks(); mockIssueService.getById.mockResolvedValue(makeIssue()); mockIssueService.assertCheckoutOwner.mockResolvedValue({ adoptedFromRunId: null }); @@ -204,7 +201,7 @@ describe("external object routes", () => { const res = await request(app).get(`/api/issues/${issueId}/external-object-summary`); // Uniform 404 so cross-tenant ids are indistinguishable from missing ones. - expect(res.status).toBe(404); + expect(res.status, app.locals.routeErrors.join("\n")).toBe(404); expect(res.body.error).toBe("Issue not found"); expect(mockExternalObjectsService.getIssueSummary).not.toHaveBeenCalled(); }); @@ -214,7 +211,7 @@ describe("external object routes", () => { const res = await request(app).get(`/api/issues/${issueId}/external-object-summary`); - expect(res.status).toBe(200); + expect(res.status, app.locals.routeErrors.join("\n")).toBe(200); expect(res.body.total).toBe(1); expect(mockExternalObjectsService.getIssueSummary).toHaveBeenCalledWith(issueId); }); @@ -242,7 +239,7 @@ describe("external object routes", () => { .post(`/api/companies/${companyId}/issues/external-object-summaries`) .send({ issueIds: [issueId] }); - expect(res.status).toBe(200); + expect(res.status, app.locals.routeErrors.join("\n")).toBe(200); expect(res.body.summaries[issueId].total).toBe(1); expect(mockExternalObjectsService.getIssueSummaries).toHaveBeenCalledWith(companyId, [issueId]); }); @@ -258,7 +255,7 @@ describe("external object routes", () => { .post(`/api/companies/${companyId}/issues/external-object-summaries`) .send({ issueIds: [issueId] }); - expect(res.status).toBe(200); + expect(res.status, app.locals.routeErrors.join("\n")).toBe(200); expect(res.body.summaries).toEqual({}); expect(mockExternalObjectsService.getIssueSummaries).toHaveBeenCalledWith(companyId, []); }); @@ -270,7 +267,7 @@ describe("external object routes", () => { .post(`/api/companies/${companyId}/issues/external-object-summaries`) .send({ issueIds: [issueId] }); - expect(res.status).toBe(403); + expect(res.status, app.locals.routeErrors.join("\n")).toBe(403); expect(mockExternalObjectsService.getIssueSummaries).not.toHaveBeenCalled(); }); @@ -281,7 +278,7 @@ describe("external object routes", () => { .post(`/api/issues/${issueId}/external-objects/refresh`) .send({}); - expect(res.status).toBe(409); + expect(res.status, app.locals.routeErrors.join("\n")).toBe(409); expect(res.body.details.code).toBe("issue_write_assignee_run_lock"); expect(mockExternalObjectsService.refreshIssueObjects).not.toHaveBeenCalled(); }); @@ -293,7 +290,7 @@ describe("external object routes", () => { .post(`/api/issues/${issueId}/external-objects/refresh`) .send({}); - expect(res.status).toBe(200); + expect(res.status, app.locals.routeErrors.join("\n")).toBe(200); expect(mockIssueService.assertCheckoutOwner).toHaveBeenCalledWith(issueId, ownerAgentId, ownerRunId); expect(mockExternalObjectsService.refreshIssueObjects).toHaveBeenCalledWith(issueId, expect.objectContaining({ companyId, diff --git a/server/src/__tests__/sidebar-preferences-routes.test.ts b/server/src/__tests__/sidebar-preferences-routes.test.ts index f426b30509..b828b9ce46 100644 --- a/server/src/__tests__/sidebar-preferences-routes.test.ts +++ b/server/src/__tests__/sidebar-preferences-routes.test.ts @@ -10,12 +10,10 @@ const mockSidebarPreferenceService = vi.hoisted(() => ({ })); const mockLogActivity = vi.hoisted(() => vi.fn()); -function registerModuleMocks() { - vi.doMock("../services/index.js", () => ({ - sidebarPreferenceService: () => mockSidebarPreferenceService, - logActivity: mockLogActivity, - })); -} +vi.mock("../services/index.js", () => ({ + sidebarPreferenceService: () => mockSidebarPreferenceService, + logActivity: mockLogActivity, +})); async function createApp(actor: Record) { const [{ sidebarPreferenceRoutes }, { errorHandler }] = await Promise.all([ @@ -29,6 +27,12 @@ async function createApp(actor: Record) { next(); }); app.use("/api", sidebarPreferenceRoutes({} as never)); + // Keep unexpected route exceptions visible when a status assertion fails. + app.locals.routeErrors = [] as string[]; + app.use((error: unknown, _req: express.Request, _res: express.Response, next: express.NextFunction) => { + app.locals.routeErrors.push(error instanceof Error ? error.stack ?? error.message : String(error)); + next(error); + }); app.use(errorHandler); return app; } @@ -40,12 +44,6 @@ const ORDERED_IDS = [ describe("sidebar preference routes", () => { beforeEach(() => { - vi.resetModules(); - vi.doUnmock("../services/index.js"); - vi.doUnmock("../routes/sidebar-preferences.js"); - vi.doUnmock("../routes/authz.js"); - vi.doUnmock("../middleware/index.js"); - registerModuleMocks(); vi.clearAllMocks(); mockSidebarPreferenceService.getCompanyOrder.mockResolvedValue({ orderedIds: ORDERED_IDS, @@ -76,7 +74,7 @@ describe("sidebar preference routes", () => { const res = await request(app).get("/api/sidebar-preferences/me"); - expect(res.status).toBe(200); + expect(res.status, app.locals.routeErrors.join("\n")).toBe(200); expect(res.body).toEqual({ orderedIds: ORDERED_IDS, updatedAt: null, @@ -97,7 +95,7 @@ describe("sidebar preference routes", () => { .put("/api/sidebar-preferences/me") .send({ orderedIds: ORDERED_IDS }); - expect(res.status).toBe(200); + expect(res.status, app.locals.routeErrors.join("\n")).toBe(200); expect(mockSidebarPreferenceService.upsertCompanyOrder).toHaveBeenCalledWith("user-1", ORDERED_IDS); }); @@ -112,7 +110,7 @@ describe("sidebar preference routes", () => { const res = await request(app).get("/api/companies/company-1/sidebar-preferences/me"); - expect(res.status).toBe(200); + expect(res.status, app.locals.routeErrors.join("\n")).toBe(200); expect(mockSidebarPreferenceService.getProjectOrder).toHaveBeenCalledWith("company-1", "user-1"); }); @@ -130,7 +128,7 @@ describe("sidebar preference routes", () => { .put("/api/companies/company-1/sidebar-preferences/me") .send({ orderedIds: ORDERED_IDS }); - expect(res.status).toBe(200); + expect(res.status, app.locals.routeErrors.join("\n")).toBe(200); expect(mockSidebarPreferenceService.upsertProjectOrder).toHaveBeenCalledWith("company-1", "user-1", ORDERED_IDS); expect(mockLogActivity).toHaveBeenCalledWith( {} as never, @@ -156,7 +154,7 @@ describe("sidebar preference routes", () => { const res = await request(app).get("/api/companies/company-1/sidebar-preferences/me"); - expect(res.status).toBe(403); + expect(res.status, app.locals.routeErrors.join("\n")).toBe(403); expect(mockSidebarPreferenceService.getProjectOrder).not.toHaveBeenCalled(); }); @@ -170,7 +168,7 @@ describe("sidebar preference routes", () => { const res = await request(app).get("/api/sidebar-preferences/me"); - expect(res.status).toBe(403); + expect(res.status, app.locals.routeErrors.join("\n")).toBe(403); expect(mockSidebarPreferenceService.getCompanyOrder).not.toHaveBeenCalled(); }); });