Skip to content

test(integration): un-skip 11 of 13 stale integration files (#3289)#3313

Merged
bokelley merged 1 commit into
mainfrom
bokelley/cleanup-3289
Apr 26, 2026
Merged

test(integration): un-skip 11 of 13 stale integration files (#3289)#3313
bokelley merged 1 commit into
mainfrom
bokelley/cleanup-3289

Conversation

@bokelley

Copy link
Copy Markdown
Contributor

Summary

Walks through the umbrella in #3289 and un-skips everything that doesn't need a deeper architectural shift. +11 files un-skipped, +131 tests now run in CI, 0 failures.

Before After
Files green 35 46
Files file-skipped 11 2 (mcp-protocol, revenue-tracking)*
Files excluded via script 2 0 (script no longer needs --exclude)
Tests passing 465 596
Tests skipped 139 71

* Plus user-context (file-skip) and impersonation-audit (only the chat-driven describe is skipped — the schema describe runs).

What's in this PR

Auth-mock cluster (8 files). Old pattern:

vi.mock('../../src/middleware/auth.js', () => ({
  requireAuth: ..., requireAdmin: ...,
}));

…silently dropped every other auth.js export. HTTPServer setup imports optionalAuth and crashes during route registration → all 28 tests in admin-endpoints (etc.) get reported as skipped under a failed suite. Switched to:

vi.mock('../../src/middleware/auth.js', async (importOriginal) => ({
  ...(await importOriginal<typeof import('../../src/middleware/auth.js')>()),
  requireAuth: ..., requireAdmin: ...,
}));

Future auth additions don't re-break tests. CSRF middleware also gets a passthrough mock so POST/PUT/DELETE stop hitting 403 forbidden on the missing X-CSRF-Token cookie.

vi.hoisted for the excluded files. admin-sync-revenue-backfill and self-service-delete errored at module-eval time because vi.mock factories referenced top-level vars that aren't live until after factory hoisting. Shared mock state moves into vi.hoisted({...}). The WorkOS class mock now wires every new WorkOS() to the same shared instance methods, so per-test mockImplementation overrides retarget the right spy. --exclude flags are removed from test:server-integration.

health.test.ts. Dropped the stale (server as any).registry.initialize() beforeAllHTTPServer no longer exposes .registry. The two failing-init tests went with it; the GET /health checks still match the live response shape.

mcp-protocol.test.ts. Bearer-auth gate cleared via MCP_AUTH_DISABLED=true at module-load. Stays describe.skip pending an SSE-format response-parser update — only blocker now is Content-Type: text/event-stream on the StreamableHTTP transport.

schema-routing.test.ts. Replaced localeCompare(numeric: true) with semver.rcompare so the test's "latest in 3.x" matches the alias-rewrite middleware (which prefers stable over prerelease at the same X.Y.Z).

digest-email-recipients.test.ts. Track-B counts were hardcoded to 3; the cert curriculum grew to 4. Reads the live count out of certification_modules so the test tracks curriculum changes instead of breaking on them.

member-db.test.ts. logo_url / brand_color etc. aren't part of CreateMemberProfileInput anymore and the create handler doesn't persist them. Test reduced to the documented input contract.

agent-visibility-e2e.test.ts. Fixed src/db/migrations/... path → server/src/db/migrations/....

Test env. WORKOS_COOKIE_PASSWORD set in revenue-tracking-env.ts setup so AUTH_ENABLED resolves true and workos!.userManagement is non-null in tests that reach for it.

Remaining (still tracked in #3289)

  • mcp-protocol.test.ts — needs SSE-format parser
  • revenue-tracking.test.ts — needs fuller stripe-client mock (webhooks.constructEvent + invoice/customer fixtures)
  • user-context.test.ts (WorkOS-path tests) — needs richer WorkOS user-lookup mock
  • impersonation-audit.test.ts (chat-driven describe) — needs working AddieClaudeClient init or refactor to unit-level coverage of audit-log helpers
  • join-request-approval and personal-workspace-restrictions — 9 individual it.skip tests that need a per-file @workos-inc/node class mock so handlers' direct workos!.userManagement.* calls don't 401 against real WorkOS

Per-test skips now have inline comments describing exactly what blocks them, so the next person picking up #3289 can jump straight in.

Test plan

  • CI: Server integration tests job goes green on this PR.
  • Spot-check: npm run test:server-integration against a local Postgres reproduces 596 passed / 71 skipped.

🤖 Generated with Claude Code

The integration suite landed in #3292 with 13 files dark behind
describe.skip / --exclude, surfacing as #3289. This walks through
every umbrella item:

* auth-mock cluster (8 files) — vi.mock factory was returning a
  partial auth.js shape, so HTTPServer setup tripped on the missing
  optionalAuth import. Switched to importOriginal + override; spread
  the real exports so future auth additions don't re-break tests.
  Added matching csrfProtection mocks so POST/PUT/DELETE no longer
  403 on missing X-CSRF-Token.

* admin-sync-revenue-backfill, self-service-delete — were excluded
  from the script because vi.mock factories referenced top-level vars
  that aren't live until after factory hoisting. Moved shared state
  into vi.hoisted blocks; rewrote the WorkOS class mock so every
  `new WorkOS()` returns the same shared instance methods, so
  per-test mockImplementation overrides actually retarget the right
  spy. Removed --exclude flags from test:server-integration.

* health.test.ts — dropped the stale (server as any).registry.initialize()
  beforeAll (HTTPServer no longer exposes registry) and the two
  database-init tests that exercised it; kept the GET /health checks
  that still match the live response shape.

* mcp-protocol — bearer-auth gate cleared via MCP_AUTH_DISABLED at
  module-load time; the file now stays describe.skip pending an SSE-
  format response-parser update (the only remaining blocker is the
  StreamableHTTP transport returning text/event-stream by default).

* schema-routing — replaced localeCompare(numeric: true) with
  semver.rcompare so the test's idea of "latest in 3.x" matches the
  alias-rewrite middleware (which prefers stable over prerelease at
  the same X.Y.Z).

* digest-email-recipients — counts were hardcoded to a track-B size
  of 3; the cert curriculum grew to 4. Read the live count out of
  certification_modules so the test tracks curriculum changes.

* member-db — logo_url / brand_color / etc. aren't part of
  CreateMemberProfileInput anymore; the create handler doesn't
  persist them. Test reduced to the documented input contract.

* agent-visibility-e2e — fixed `src/db/migrations/...` path to
  `server/src/db/migrations/...` so the legacy-shape test reads the
  migration file CI ships.

* WORKOS_COOKIE_PASSWORD added to revenue-tracking-env.ts setup so
  AUTH_ENABLED resolves true in tests where production code reaches
  for `workos!.userManagement` and would otherwise hit a null deref.

Result: 46 / 50 integration files green (was 35), 596 tests pass
(was 465), 71 still skipped (was 139), 0 failures. Remaining
file-level skips and per-test skips have richer comments pointing
to #3289 with concrete next-step descriptions.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
};
});

const { FAKE_INVOICE, mockInvoicesList, mockCustomersRetrieve, mockProductsRetrieve } = mocks;
};
});

const { FAKE_INVOICE, mockInvoicesList, mockCustomersRetrieve, mockProductsRetrieve } = mocks;
@bokelley bokelley merged commit f541e4d into main Apr 26, 2026
17 checks passed
@bokelley bokelley deleted the bokelley/cleanup-3289 branch April 26, 2026 22:22
bokelley added a commit that referenced this pull request Apr 27, 2026
…mock (#3327)

* test(integration): un-skip revenue-tracking via fuller stripe-client mock

Adds vi.hoisted + vi.mock for stripe-client following the pattern from
admin-sync-revenue-backfill.test.ts (#3313). The webhook route guard
requires both stripe and STRIPE_WEBHOOK_SECRET to be non-null; the mock
satisfies both. products.retrieve rejects so the handler exercises its
description-fallback path.

Removes the stale dead-code block in beforeAll that attempted to patch the
live stripe object at runtime — this was a no-op since stripe was null,
and became a mock-overwrite bug after the vi.mock was added.

Refs #3318. Part of #3289 integration-test restoration.

https://claude.ai/code/session_01DZZ43na714gw4EYgJ5dJUQ

* test(integration): align revenue-tracking stats assertions with current behavior

formatCurrency in server/src/routes/admin/stats.ts rounds to whole dollars
for dashboard display (10998¢ → \$110, 5998¢ → \$60). Test was asserting
cent-precision strings ('\$109.98', '\$59.98') which no longer match.
Update expectations.

MRR test: live MRR query reads from revenue_events (DISTINCT ON
stripe_subscription_id, period_end > NOW(), revenue_type IN
subscription_initial/recurring), not from columns on the organizations
table. Replace the UPDATE with a properly-shaped revenue_event INSERT.

Refunds + product-breakdown tests also pick up the formatter rounding.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant