-
Notifications
You must be signed in to change notification settings - Fork 0
Implement local testing with supabase #53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements local testing infrastructure with Supabase, enabling developers to run integration tests against a local Supabase instance. The changes standardize environment variable naming, set up Vitest configuration with test helpers, add database migrations, and update CI workflows to support automated testing.
Key Changes
- Environment variable standardization: Migrated from
NEXT_PUBLIC_SUPABASE_*to simplerAPI_URLandPUBLISHABLE_KEYconventions - Test infrastructure setup: Added Vitest configuration with setup files, test helpers for Supabase client creation, and fixture utilities for creating test data
- Local development tooling: Added Supabase configuration and migration files for local database setup, plus npm script for database initialization
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.config.ts | Configures Vitest to use test setup file for initialization |
| tests/setup.ts | Implements global test setup/teardown with database cleanup hooks |
| tests/helpers/supabase.ts | Provides test client creation and database cleanup utilities |
| tests/helpers/fixtures.ts | Adds fixture functions for creating test users and volunteers |
| tests/helpers/index.ts | Exports test helper modules |
| tests/lib/api/getExample.integration.test.ts | Demonstrates integration testing pattern with Supabase |
| supabase/migrations/20251210030724_remote_schema.sql | Defines database schema with tables, constraints, and RLS policies |
| supabase/config.toml | Configures local Supabase instance settings |
| supabase/.gitignore | Excludes Supabase temporary files from version control |
| src/lib/client/supabase/server.ts | Updates environment variable names for Supabase client creation |
| package.json | Adds dotenv dependency and db initialization script |
| package-lock.json | Updates dependency lock file with new and upgraded packages |
| .vscode/settings.json | Configures VS Code for Deno support in supabase/functions |
| .vscode/extensions.json | Updates recommended extensions for Deno development |
| .github/workflows/ci.yml | Adds database startup step to CI workflow |
Comments suppressed due to low confidence (1)
supabase/migrations/20251210030724_remote_schema.sql:293
- The Row Level Security (RLS) policies grant full permissions (
USING (true) WITH CHECK (true)) to anonymous users for all tables. While this may be intentional for the "development phase" as noted in the policy names, this configuration is extremely dangerous and should:
- Never be deployed to production - This allows anyone to read, insert, update, and delete all data
- Include prominent documentation warnings about these permissive policies
- Have a plan/timeline for implementing proper RLS policies before production deployment
Consider adding comments in the migration file warning about these development-only policies and creating a tracking issue for implementing proper security policies.
CREATE POLICY "full permission for development phase" ON "public"."Cohorts" TO "anon" USING (true) WITH CHECK (true);
CREATE POLICY "full permission for development phase" ON "public"."Roles" TO "anon" USING (true) WITH CHECK (true);
CREATE POLICY "full permission for development phase" ON "public"."VolunteerCohorts" TO "anon" USING (true) WITH CHECK (true);
CREATE POLICY "full permission for development phase" ON "public"."VolunteerRoles" TO "anon" USING (true) WITH CHECK (true);
CREATE POLICY "full permissions for development phase" ON "public"."Volunteers" TO "anon" USING (true) WITH CHECK (true);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export function createAnonTestClient(): SupabaseClient<Database> { | ||
| const { apiUrl, key } = getTestConfig(); | ||
|
|
||
| return createSupabaseClient<Database>(apiUrl, key, { | ||
| auth: { | ||
| autoRefreshToken: false, | ||
| persistSession: false, | ||
| }, | ||
| }); | ||
| } | ||
|
|
Copilot
AI
Dec 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The createAnonTestClient function is identical to createTestClient. Both create clients with the same authentication configuration (autoRefreshToken: false, persistSession: false). Consider either:
- Removing the duplicate function if they serve the same purpose
- Differentiating them if they should have different behavior (e.g., different auth configs or keys)
| export function createAnonTestClient(): SupabaseClient<Database> { | |
| const { apiUrl, key } = getTestConfig(); | |
| return createSupabaseClient<Database>(apiUrl, key, { | |
| auth: { | |
| autoRefreshToken: false, | |
| persistSession: false, | |
| }, | |
| }); | |
| } |
| "[typescript]": { | ||
| "editor.defaultFormatter": "denoland.vscode-deno" | ||
| }, |
Copilot
AI
Dec 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The TypeScript default formatter is set to Deno for all TypeScript files, but Deno should only be used for files in supabase/functions (as configured in line 5). This will cause formatting inconsistencies since the rest of the codebase uses Prettier. Consider either:
- Removing this global TypeScript formatter override and letting it default to Prettier
- Using a more specific configuration that only applies Deno formatting to files within
supabase/functions
| "[typescript]": { | |
| "editor.defaultFormatter": "denoland.vscode-deno" | |
| }, | |
| }, | ||
| }); | ||
| } | ||
|
|
Copilot
AI
Dec 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The deleteAllFromTables function lacks documentation explaining:
- Why the specific deletion order is critical (foreign key constraints)
- That it's used in test setup/teardown
- Potential errors that could occur (permissions, constraints)
- That it silently ignores errors
Adding JSDoc comments would help maintainers understand the function's purpose and behavior.
| /** | |
| * Deletes all rows from a set of tables in a specific order. | |
| * | |
| * The deletion order is critical due to foreign key constraints between tables. | |
| * This function is intended for use in test setup/teardown to ensure a clean database state. | |
| * Potential errors (such as permission issues or constraint violations) are silently ignored; | |
| * the function does not throw or log errors if deletions fail. | |
| * | |
| * @param client Supabase client instance connected to the test database. | |
| * @returns Promise that resolves when all deletions have been attempted. | |
| */ |
package.json
Outdated
| "check": "npm run format:check && npm run lint && npm run typecheck", | ||
| "fix": "npm run format && npm run lint:fix" | ||
| "fix": "npm run format && npm run lint:fix", | ||
| "db": "npx supabase start && npx supabase migration up && rm .env && npx supabase status -o env >> .env" |
Copilot
AI
Dec 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The db script uses rm .env which will fail on Windows systems (should use rimraf or a cross-platform alternative). Additionally, this script unconditionally removes the .env file, which could delete important environment variables that aren't generated by supabase status. Consider:
- Using a cross-platform file removal tool or Node.js script
- Backing up existing
.envcontent or merging the output instead of overwriting - Creating a
.env.localfile specifically for local development
| "db": "npx supabase start && npx supabase migration up && rm .env && npx supabase status -o env >> .env" | |
| "db": "npx supabase start && npx supabase migration up && npx rimraf .env.local && npx supabase status -o env >> .env.local" |
| @@ -1,3 +1,3 @@ | |||
| { | |||
| "recommendations": ["dbaeumer.vscode-eslint", "esbenp.prettier-vscode"] | |||
| "recommendations": ["denoland.vscode-deno"] | |||
Copilot
AI
Dec 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The VS Code recommended extensions were replaced, removing ESLint and Prettier extensions and adding only Deno. However, the project still uses ESLint and Prettier (based on package.json and other config files). The Deno extension is only needed for supabase/functions directory. Consider keeping the ESLint and Prettier extensions in the recommendations alongside Deno, as they're still actively used in this project.
| "recommendations": ["denoland.vscode-deno"] | |
| "recommendations": [ | |
| "denoland.vscode-deno", | |
| "dbaeumer.vscode-eslint", | |
| "esbenp.prettier-vscode" | |
| ] |
| beforeAll(async () => { | ||
| const client = createTestClient(); | ||
| await deleteAllFromTables(client); | ||
| }); | ||
|
|
||
| afterAll(async () => { | ||
| const client = createTestClient(); |
Copilot
AI
Dec 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Creating a new Supabase client for each beforeAll and afterAll hook may be inefficient. Consider creating a single client instance and reusing it, or at least document why separate clients are needed for setup and teardown.
| beforeAll(async () => { | |
| const client = createTestClient(); | |
| await deleteAllFromTables(client); | |
| }); | |
| afterAll(async () => { | |
| const client = createTestClient(); | |
| const client = createTestClient(); | |
| beforeAll(async () => { | |
| await deleteAllFromTables(client); | |
| }); | |
| afterAll(async () => { |
| const client = createTestClient(); | ||
|
|
Copilot
AI
Dec 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Creating the test client at the module level (outside of test cases) means the client is initialized when the module loads, before the environment variables may be loaded by dotenv. This could cause initialization errors. Consider creating the client within each test or in a beforeEach hook to ensure proper initialization timing.
| const client = createTestClient(); | |
| let client; | |
| beforeEach(() => { | |
| client = createTestClient(); | |
| }); |
| const rows = allRows as unknown as Array<{ id: number }>; | ||
| const ids = rows.map((row) => row.id); | ||
| const filteredIds = ids.filter( | ||
| (id): id is number => id !== undefined && id !== null |
Copilot
AI
Dec 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The type assertion as unknown as Array<{ id: number }> on line 56 followed by a filter to ensure IDs are not undefined/null (lines 58-60) suggests a potential type mismatch. Consider using proper typing from the Database schema to avoid the need for these defensive checks, or at least add a comment explaining why the type assertion is necessary and what edge cases the filter is handling.
| const rows = allRows as unknown as Array<{ id: number }>; | |
| const ids = rows.map((row) => row.id); | |
| const filteredIds = ids.filter( | |
| (id): id is number => id !== undefined && id !== null | |
| // Use the correct type from the Database schema for rows | |
| type RowType = Database["public"]["Tables"][typeof tableName]["Row"]; | |
| // The select("id") call returns Array<Pick<RowType, "id">> | |
| const ids = (allRows as Array<Pick<RowType, "id">>).map((row) => row.id); | |
| // If the id type is not guaranteed to be number, filter out undefined/null | |
| // (This filter is defensive; remove if schema guarantees id is always number) | |
| const filteredIds = ids.filter( | |
| (id): id is number => typeof id === "number" |
| export async function createTestUser( | ||
| client: SupabaseClient<Database>, | ||
| overrides?: Partial<TablesInsert<"Users">> | ||
| ): Promise<TablesInsert<"Users"> & { id: string }> { | ||
| const user: TablesInsert<"Users"> = { | ||
| id: overrides?.id || `test-user-${Date.now()}`, | ||
| email: overrides?.email || `test-${Date.now()}@example.com`, | ||
| role: overrides?.role || null, | ||
| ...overrides, | ||
| }; | ||
|
|
||
| const { data, error } = await client | ||
| .from("Users") | ||
| .insert(user) | ||
| .select() | ||
| .single(); | ||
|
|
||
| if (error) { | ||
| throw new Error(`Failed to create test user: ${error.message}`); | ||
| } | ||
|
|
||
| return data; | ||
| } |
Copilot
AI
Dec 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test fixture functions createTestUser and createTestVolunteer lack JSDoc documentation. Consider adding documentation to explain:
- The purpose of each function
- Parameters and their types
- What the
overridesparameter does - Return value structure
- Potential errors that might be thrown
This would improve developer experience when using these fixtures in tests.
| CREATE POLICY "full permission for development phase" ON "public"."Cohorts" TO "anon" USING (true) WITH CHECK (true); | ||
|
|
||
|
|
||
|
|
||
| CREATE POLICY "full permission for development phase" ON "public"."Roles" TO "anon" USING (true) WITH CHECK (true); | ||
|
|
||
|
|
||
|
|
||
| CREATE POLICY "full permission for development phase" ON "public"."VolunteerCohorts" TO "anon" USING (true) WITH CHECK (true); | ||
|
|
||
|
|
||
|
|
||
| CREATE POLICY "full permission for development phase" ON "public"."VolunteerRoles" TO "anon" USING (true) WITH CHECK (true); | ||
|
|
||
|
|
||
|
|
||
| CREATE POLICY "full permissions for development phase" ON "public"."Volunteers" TO "anon" USING (true) WITH CHECK (true); | ||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
Copilot
AI
Dec 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These row-level security policies grant the anon role USING (true) WITH CHECK (true) on Cohorts, Roles, VolunteerCohorts, VolunteerRoles, and Volunteers, which effectively gives any anonymous client full read/write access to all rows via the public anon key. An attacker who can call the Supabase REST/GraphQL endpoints anonymously can exfiltrate and arbitrarily modify sensitive volunteer-related data. Replace these policies with least-privilege RLS rules (e.g., restricted to authenticated users and/or row ownership) and, if you need unrestricted access for development, use a separate dev project or a non-anon role that is never exposed to untrusted clients.
| CREATE POLICY "full permission for development phase" ON "public"."Cohorts" TO "anon" USING (true) WITH CHECK (true); | |
| CREATE POLICY "full permission for development phase" ON "public"."Roles" TO "anon" USING (true) WITH CHECK (true); | |
| CREATE POLICY "full permission for development phase" ON "public"."VolunteerCohorts" TO "anon" USING (true) WITH CHECK (true); | |
| CREATE POLICY "full permission for development phase" ON "public"."VolunteerRoles" TO "anon" USING (true) WITH CHECK (true); | |
| CREATE POLICY "full permissions for development phase" ON "public"."Volunteers" TO "anon" USING (true) WITH CHECK (true); | |
| -- Remove permissive anon policies and replace with least-privilege policies | |
| -- Cohorts: Only authenticated users can read and write | |
| CREATE POLICY "select_cohorts_authenticated" ON "public"."Cohorts" | |
| FOR SELECT TO authenticated | |
| USING (true); | |
| CREATE POLICY "modify_cohorts_authenticated" ON "public"."Cohorts" | |
| FOR ALL TO authenticated | |
| USING (true) WITH CHECK (true); | |
| -- Roles: Only authenticated users can read and write | |
| CREATE POLICY "select_roles_authenticated" ON "public"."Roles" | |
| FOR SELECT TO authenticated | |
| USING (true); | |
| CREATE POLICY "modify_roles_authenticated" ON "public"."Roles" | |
| FOR ALL TO authenticated | |
| USING (true) WITH CHECK (true); | |
| -- VolunteerCohorts: Only authenticated users can read and write | |
| CREATE POLICY "select_volunteercohorts_authenticated" ON "public"."VolunteerCohorts" | |
| FOR SELECT TO authenticated | |
| USING (true); | |
| CREATE POLICY "modify_volunteercohorts_authenticated" ON "public"."VolunteerCohorts" | |
| FOR ALL TO authenticated | |
| USING (true) WITH CHECK (true); | |
| -- VolunteerRoles: Only authenticated users can read and write | |
| CREATE POLICY "select_volunteerroles_authenticated" ON "public"."VolunteerRoles" | |
| FOR SELECT TO authenticated | |
| USING (true); | |
| CREATE POLICY "modify_volunteerroles_authenticated" ON "public"."VolunteerRoles" | |
| FOR ALL TO authenticated | |
| USING (true) WITH CHECK (true); | |
| -- Volunteers: Only authenticated users can read and write | |
| CREATE POLICY "select_volunteers_authenticated" ON "public"."Volunteers" | |
| FOR SELECT TO authenticated | |
| USING (true); | |
| CREATE POLICY "modify_volunteers_authenticated" ON "public"."Volunteers" | |
| FOR ALL TO authenticated | |
| USING (true) WITH CHECK (true); |
da65241 to
c7ef165
Compare
|
Superseded by #64 |
No description provided.