Skip to content

feat(graphile-test): add withTransaction to pgClient in test context#1087

Open
pyramation wants to merge 1 commit intomainfrom
feat/graphile-test-with-transaction
Open

feat(graphile-test): add withTransaction to pgClient in test context#1087
pyramation wants to merge 1 commit intomainfrom
feat/graphile-test-with-transaction

Conversation

@pyramation
Copy link
Copy Markdown
Contributor

Summary

Adds a savepoint-based withTransaction implementation to the pgClient provided inside the withPgClient callback in graphile-test's execution context.

In production, @dataplan/pg provides withTransaction on the pgClient. In tests, the raw pg.Client from pgsql-test lacks this method, causing any Graphile plugin that calls pgClient.withTransaction() (e.g. graphile-presigned-url-plugin) to crash at runtime unless each test file manually monkey-patches the method.

This fix augments the client lazily (guarded by typeof check) with a savepoint-based implementation that nests cleanly inside the test harness's outer transaction. Downstream tests in constructive-db can now remove their per-file monkey patches.

Review & Testing Checklist for Human

  • Verify the savepoint-based withTransaction behaves correctly when the callback throws (should ROLLBACK TO SAVEPOINT and re-throw, not leave the transaction in a broken state)
  • Confirm that mutating pgClient (attaching withTransaction permanently) doesn't cause issues across multiple withPgClient calls in the same test — the if guard prevents re-assignment but the method stays attached
  • After publishing, verify downstream: remove the monkey patch from constructive-db's minio-multi-scope-graphql.integration.test.ts and confirm the storage upload tests still pass

Notes

  • No unit test added in this PR. The method is exercised by the presigned-url plugin's upload flow in constructive-db's integration tests. A focused unit test could be added as a follow-up.
  • The eslint-disable for @typescript-eslint/no-explicit-any is required because the raw pg.Client type doesn't declare withTransaction.

Link to Devin session: https://app.devin.ai/sessions/7903d2a3e7a34c6daa605e12d6b80d9e
Requested by: @pyramation

In production, @dataplan/pg provides withTransaction on the pgClient.
In tests, the raw pg Client lacks it, causing plugins that call
pgClient.withTransaction() (e.g. graphile-presigned-url-plugin) to fail
unless tests manually monkey-patch the method.

This adds a savepoint-based withTransaction implementation to the client
inside the withPgClient callback. Savepoints nest cleanly inside the
test harness's outer transaction, matching production behavior.

Eliminates the need for per-test monkey patches.
@devin-ai-integration
Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

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