Skip to content

Conversation

@pyramation
Copy link
Contributor

@pyramation pyramation commented Jan 21, 2026

Summary

Re-implements the changes from PR #633 which was closed due to test hanging issues (now fixed in PR #634). This adds 11 packages with tests that were missing from the CI test matrix:

  • graphql/server-test
  • graphql/env
  • graphql/server
  • graphql/test
  • graphql/playwright-test
  • packages/csv-to-pg
  • packages/smtppostmaster
  • postgres/drizzle-orm-test
  • uploads/etag-hash
  • uploads/etag-stream
  • uploads/stream-to-etag

Also updates the graphql/env test snapshot to include new smtp configuration fields that were added to the codebase.

Note: jobs/knative-job-worker is commented out (temporarily disabled per user request).

Updates since last revision

  • Fixed IPv6/IPv4 mismatch issue: The server was binding to localhost which resolves to ::1 (IPv6) on some systems, but supertest connects to 127.0.0.1 (IPv4), causing ECONNREFUSED errors. Now explicitly binds to 127.0.0.1.
  • Fixed race condition: Added wait for 'listening' event before accessing httpServer.address() (which was returning null).
  • Fixed GraphQL query field names: Updated test queries from allUsers/allPosts to users/posts to match the actual PostGraphile-generated schema.
  • Updated test assertions: URL pattern expectations now match 127.0.0.1 instead of localhost.
  • Clarified rollback test: The pg client (superuser) doesn't participate in the db client's transaction rollback - test now correctly demonstrates this behavior.

Review & Testing Checklist for Human

  • Verify IPv6/IPv4 fix works in CI environment - The change from localhost to 127.0.0.1 is the core fix; confirm tests pass in CI
  • Review graphql/server-test/src/server.ts - verify the listening event wait logic and IPv4 binding are correct
  • Verify the GraphQL field name changes (users vs allUsers) match the actual schema generated by PostGraphile
  • Review the rollback test change - it now asserts data persists (count=3) rather than rolls back (count=2); confirm this is the intended behavior for testing pg client vs db client differences
  • Verify all 11 new CI jobs run and pass

Suggested test plan: Wait for CI to complete and verify all 11 new jobs pass, particularly graphql/server-test. If tests fail with connection errors, the IPv6/IPv4 fix may need adjustment for that environment.

Notes

Link to Devin run: https://app.devin.ai/sessions/3cb2368e9a0d4516a01fbf7cbcd33e94
Requested by: Dan Lynch (@pyramation)

Adds 11 packages with tests that were missing from the CI test matrix:
- graphql/server-test
- graphql/env
- graphql/server
- graphql/test
- graphql/playwright-test
- packages/csv-to-pg
- packages/smtppostmaster
- postgres/drizzle-orm-test
- uploads/etag-hash
- uploads/etag-stream
- uploads/stream-to-etag

Also updates graphql/env snapshot to include new smtp configuration fields.

Note: jobs/knative-job-worker is commented out (temporarily disabled per user request).
@devin-ai-integration
Copy link
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

…ing address

The server.listen() call returns immediately but the server isn't actually
listening yet. This caused httpServer.address() to return null, resulting in
'Cannot read properties of null (reading port)' errors.

Now we wait for the 'listening' event before accessing the address.
@pyramation pyramation closed this Jan 21, 2026
@pyramation pyramation reopened this Jan 21, 2026
- Bind server to 127.0.0.1 instead of localhost to avoid IPv6/IPv4 mismatch
  (localhost resolves to ::1 on some systems but supertest connects to 127.0.0.1)
- Update findAvailablePort to accept host parameter
- Update test assertions to expect 127.0.0.1 in URL patterns
- Fix GraphQL query field names (allUsers -> users, allPosts -> posts)
- Update snapshot with correct query results
- Fix rollback test to correctly demonstrate pg client behavior
@pyramation pyramation merged commit 992cb99 into main Jan 21, 2026
48 checks passed
@pyramation pyramation deleted the devin/1768982377-add-missing-packages-to-ci branch January 21, 2026 10:37
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.

2 participants