From 6236ee9ef913d4b1c66ba2ccdb9d57d87fe640ce Mon Sep 17 00:00:00 2001 From: Ali Waseem Date: Thu, 28 May 2026 06:58:50 -0600 Subject: [PATCH] POC: bring back MSW to remove the pattern of vi.mock (#46439) ## I have read the [CONTRIBUTING.md](https://github.com/supabase/supabase/blob/master/CONTRIBUTING.md) file. YES ## What kind of change does this PR introduce? Right now our tests for API mocking is using vi.mock and mocking that query or fetch handler. This is not the right approach IMO, 2 years ago @jordienr added MSW with some very powerful helpers. The idea is to move component test that rely on API using MSW within ViteTest. Principles are simple: - Mock API responses - Mount your component that uses API responses - Tests and assert on UI - Added Skill for Clanker This pattern is 100 times better than what we have ## Summary by CodeRabbit * **Tests** * Expanded and strengthened test suites for secrets, org lookup, support flows, OAuth auth, and onboarding; mocks now use contract-backed responses for more realistic coverage. * **Documentation** * Added a comprehensive guide describing a standardized pattern for component tests that mock network requests. * **Chores** * Improved test helpers, typing for API mocks, and test runner configuration for more reliable and maintainable tests. [![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/supabase/supabase/pull/46439?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) --------- Co-authored-by: Alaister Young Co-authored-by: Alaister Young <10985857+alaister@users.noreply.github.com> --- .claude/skills/studio-mock-api-tests/SKILL.md | 294 ++++++++++++++++++ .../EditSecretSheet.test.tsx | 69 ++++ .../__tests__/EditSecretModal.test.tsx | 1 - .../Organization/OrgNotFound.test.tsx | 92 ++++++ .../RedeemCredits/RedeemCredits.test.tsx | 13 +- .../__tests__/SupportFormPage.test.tsx | 94 ++++-- .../components/ApiAuthorization.test.tsx | 51 ++- apps/studio/tests/helpers.tsx | 37 +++ apps/studio/tests/lib/custom-render.tsx | 5 +- apps/studio/tests/lib/msw.ts | 23 +- .../pages/aws-marketplace-onboarding.test.tsx | 32 +- 11 files changed, 645 insertions(+), 66 deletions(-) create mode 100644 .claude/skills/studio-mock-api-tests/SKILL.md create mode 100644 apps/studio/components/interfaces/Functions/EdgeFunctionSecrets/EditSecretSheet.test.tsx create mode 100644 apps/studio/components/interfaces/Organization/OrgNotFound.test.tsx diff --git a/.claude/skills/studio-mock-api-tests/SKILL.md b/.claude/skills/studio-mock-api-tests/SKILL.md new file mode 100644 index 00000000000..bd09ea125f6 --- /dev/null +++ b/.claude/skills/studio-mock-api-tests/SKILL.md @@ -0,0 +1,294 @@ +--- +name: studio-mock-api-tests +description: Component tests for Supabase Studio that mock API requests at the + network layer with MSW. Use when writing or reviewing a component test that + exercises a React Query hook or mutation, or when migrating an existing + test away from vi.mock('@/data/...'). Covers the customRender + addAPIMock + template and the jsdom/MSW gotchas that cost real debugging time. +--- + +# Studio MSW component tests + +Mount a Studio component, intercept its network calls with MSW, assert +what renders and what gets sent. The infrastructure is already wired up — +this skill is the working template plus the gotchas. + +## When to use + +- The component (or any descendant it renders) calls a React Query hook + or mutation that hits `/platform/...`, `/v1/...`, or another endpoint + in `apps/studio/data/api.d.ts`. +- You'd otherwise be tempted to write `vi.mock('@/data/some-query', ...)`. + **Don't.** Mock the network instead — see "Why not vi.mock" below. + +If the component is purely presentational with no data fetching, you +don't need MSW; render and assert directly. + +## The template + +```tsx +import { fireEvent, screen, waitFor } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { mockAnimationsApi } from 'jsdom-testing-mocks' +import { HttpResponse } from 'msw' +import { describe, expect, test, vi } from 'vitest' + +import { MyComponent } from './MyComponent' +import { customRender } from '@/tests/lib/custom-render' +import { addAPIMock } from '@/tests/lib/msw' + +// Needed if the component renders inside a Sheet, Modal, Popover, or +// anything else built on Radix that uses Web Animations. +mockAnimationsApi() + +describe('MyComponent', () => { + test('renders rows from the API', async () => { + addAPIMock({ + method: 'get', + path: '/platform/organizations', + response: () => + HttpResponse.json([ + { + /* ... */ + }, + ]), + }) + + customRender() + + expect(await screen.findByText('Acme')).toBeInTheDocument() + }) +}) +``` + +That's the whole pattern. Server lifecycle (`listen`/`resetHandlers`/ +`close`) is handled by `apps/studio/tests/vitestSetup.ts` — handlers +registered via `addAPIMock` are scoped to the current test. + +## Gotchas that will eat your afternoon + +### 1. Path params use `:slug`, not `{slug}` + +`addAPIMock` is typed from the OpenAPI `paths`, but path params are +remapped to MSW's `:param` format. Autocomplete will guide you, but if +typecheck reports the path isn't assignable, you're using the OpenAPI +`{slug}` form. + +```ts +// ❌ TypeScript error, MSW won't match +path: '/platform/organizations/{slug}/projects' + +// ✅ Correct +path: '/platform/organizations/:slug/projects' +``` + +### 2. Use `HttpResponse.json`, not `new HttpResponse` + +For success responses, always go through `HttpResponse.json` — even for +204/201-no-content endpoints. A raw `new HttpResponse(null, { status: 201 })` +returns no content-type, and `openapi-fetch` can hang the mutation flow, +which silently breaks `onSuccess` callbacks. + +```ts +// ❌ Mutation onSuccess silently never fires +response: () => new HttpResponse(null, { status: 201 }) + +// ✅ Works (pass the OpenAPI body shape explicitly — see gotcha #8) +response: () => HttpResponse.json({}, { status: 201 }) +``` + +### 3. Submit buttons in Sheets/Modals need `fireEvent.click` + +The convention `