refactor: deduplicate remaining seed fixtures across server-test#1222
Merged
Conversation
Extract shared base/setup.sql (roles, uuid-ossp, stamps) from the repeated setup.sql files in simple-seed/, schema-snapshot/, and search-seed/. Changes: - Add __fixtures__/seed/base/setup.sql — minimal base layer - Delete simple-seed/ entirely — tests now compose from base/setup.sql + root app-schemas/simple-pets/ (schema + test-data) - Delete schema-snapshot/setup.sql — tests now use base/setup.sql - Replace search-seed/setup.sql with search-seed/extensions.sql containing only pg_trgm + optional pgvector (unique content) Updated test files: - server.integration.test.ts — simple-seed scenarios use shared base - cli-e2e.test.ts — both simple-seed and search-seed use shared base - search.integration.test.ts — uses shared base + local extensions - schema-snapshot.test.ts — uses shared base Remaining local fixtures (unique content, not shareable): - search-seed/ — extensions.sql, schema.sql, test-data.sql - schema-snapshot/ — schema.sql, test-data.sql - simple-seed-storage/ — storage module setup, schema, test-data
Contributor
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Remove all inline CREATE ROLE blocks from seed SQL fixtures. Roles (administrator, authenticated, anonymous) are now created by pgsql-test's getConnections() via DbAdmin.createBaseRoles(), which delegates to generateCreateBaseRolesSQL() from @pgpmjs/core. This eliminates the DO $$ ... IF NOT EXISTS ... CREATE ROLE pattern that can cause lock issues, and aligns with the pgpm role management workflow.
This comment has been minimized.
This comment has been minimized.
When parallel CI test suites call createBaseRoles simultaneously, the ALTER ROLE in generateCreateBaseRolesSQL can race and fail with 'tuple concurrently updated'. Add retry loop (3 attempts) to handle this transient concurrency error.
…otstrap The CI workflow already runs 'pgpm admin-users bootstrap' before tests, which creates the base roles (administrator, authenticated, anonymous). No code changes needed in pgsql-client or pgsql-test — the seed SQL just needed to stop duplicating what the workflow already does.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Consolidates remaining duplicated seed fixtures in
graphql/server-testand removes all inlineCREATE ROLEblocks from seed SQL.Changes:
simple-seed/entirely — tests now compose frombase/setup.sql+ rootapp-schemas/simple-pets/schema-snapshot/setup.sqlandsearch-seed/setup.sql— replaced with sharedbase/setup.sqlbase/setup.sql— minimal shared layer (extensions + stamps schema)search-seed/extensions.sql— search-specific extensions only (pg_trgm + pgvector)DO $$ ... CREATE ROLEblocks from seed SQL — roles are created upstream by the CI workflow'spgpm admin-users bootstrap --yesstep (and locally by running the same command before tests)No changes to
pgsql-clientorpgsql-test— seed SQL just needed to stop duplicating whatpgpm admin-users bootstrapalready does.Net: ~100 lines of duplicated SQL removed, zero
CREATE ROLEin any seed file.Review & Testing Checklist for Human
CREATE ROLEremains in any__fixtures__/seed/filepgpm admin-users bootstrap --yesbefore the test steps (lines 226 and 334 ofrun-tests.yaml)pgpm admin-users bootstrap --yes && pgpm admin-users add --test --yes && cd graphql/server-test && pnpm testNotes
services/setup.sqlpreviously hadapp_user/app_adminroles that were never referenced — removed along with the other role creation blocksLink to Devin session: https://app.devin.ai/sessions/2b5a29d83d3f478e8d3d972653b4879c
Requested by: @pyramation