Assorted test fixes and cleanup#3672
Merged
Merged
Conversation
because of the `SELECTR` typo.
…emove unneeded function wrappers
There was a problem hiding this comment.
Pull request overview
This PR updates pg’s test suite to make async rejection assertions more reliable and to clean up promise API coverage, helping prevent false positives/negatives in CI.
Changes:
- Convert several
assert.rejectsusages toasync/awaitpatterns in SASL/SCRAM unit tests. - Strengthen GH issue #3174 integration assertions by checking the client “not queryable” error on a valid SQL query.
- Remove duplicate and legacy callback-based promise API integration tests, simplifying the file.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/pg/test/unit/client/sasl-scram-tests.js | Awaits assert.rejects calls so async failures are properly observed by the test runner. |
| packages/pg/test/integration/gh-issues/3174-tests.js | Ensures post-error query rejection is validated against the intended “not queryable” state, not SQL syntax errors. |
| packages/pg/test/integration/client/promise-api-tests.js | Removes duplicate/older tests and modernizes the invalid-connection test to async/await. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
avallete
added a commit
to supabase/node-postgres
that referenced
this pull request
May 13, 2026
* test: Ensure failure to throw at all doesn’t pass (brianc#3671) * Assorted test fixes and cleanup (brianc#3672) * cleanup: Remove duplicate test * cleanup: Remove nonsense test * cleanup: Simplify promise rejection test * test: Fix and tighten assertion that would always pass because of the `SELECTR` typo. * cleanup: Add missing `await`s when using `assert.rejects` in tests; remove unneeded function wrappers * ci: Node 26 followup (brianc#3670) * Revert unneeded pg-native→libpq dependency range adjustment This reverts part of commit 1025d12. * dev: Upgrade libpq/nan in lockfile for Node 26 compatibility * fix: apply SASLprep (RFC 4013) to passwords before SCRAM-SHA-256 PBKDF2 (brianc#3669) * fix: apply SASLprep (RFC 4013) to passwords before SCRAM-SHA-256 PBKDF2 `pg`'s SCRAM-SHA-256 client passes the raw password into PBKDF2 with no normalization, while PostgreSQL's server (and libpq) apply SASLprep (B.1 mapping -> NFKC -> prohibition + bidi check) when computing the stored verifier. Passwords whose NFKC form differs from themselves (e.g. containing U+00A8 dieresis, U+2011 non-breaking hyphen, U+00BC vulgar one quarter, NBSP, soft hyphen) authenticate with psql/libpq but fail against pg with `28P01`. Wire `@mongodb-js/saslprep` (the maintained fork used by mongodb's official Node driver) into `continueSession` before `crypto.deriveKey`, with a try/catch fallback to the raw password on prohibited / bidi violations to match `libpq`'s `pg_saslprep` behavior. Also adds: - Unit tests covering the soft-hyphen B.1 mapping equivalence, the Roman-numeral-IX NFKC asymmetry, the prohibited-char fallback, and a deterministic snapshot for the original bug-report password. - A gated integration test block (SCRAM_TEST_PGUSER_UNICODE / SCRAM_TEST_PGPASSWORD_UNICODE) covering raw + NFKC-equivalent + wrong password. - A `scram_unicode_test` role (password `U&'IX-\2168'`) provisioned in CI plus matching env vars so the new integration tests run on every Node version. - A Cloudflare Workers regression guard that exercises `sasl.continueSession` to ensure `@mongodb-js/saslprep` resolves cleanly under workerd. - A `pg@8.21.0` CHANGELOG entry. * fix: inline SASLprep, drop @mongodb-js/saslprep dependency Per review feedback on brianc#3669: ship the SASLprep step as a small in-tree function instead of pulling a runtime dep with an unpinned transitive. The function performs only the three byte-changing steps from RFC 4013 (Table C.1.2 -> SPACE, Table B.1 -> empty, NFKC) and skips the prohibition (RFC 4013 section 2.3) and bidi (RFC 3454 section 6) checks, since libpq is forgiving on those paths and Postgres's own SASLprep is similarly lenient. Removes the try/catch fallback (no code path throws). The deterministic snapshot tests stay byte-for-byte valid because none of them touch U+200B, the only edge case where the inline impl diverges from `@mongodb-js/saslprep`. RFC 3454 places U+200B in Table B.1 (mapped to nothing); the dep maps it to SPACE. PostgreSQL's saslprep.c follows the RFC, so the inline impl matches libpq more closely on that codepoint. The B.1 unit-test rename ("passes ASCII control characters through normalization unchanged") keeps the same snapshot bytes since BEL is unchanged by all three steps. Co-authored-by: charmander <charmander@noreply.github.com> * Revert unrelated no-op changes to yarn.lock now that the associated dependency isn’t being added. * cleanup: Allow Prettier to format some lines * cleanup: Remove changelog entry for unreleased pg version normally added as part of the release process * refactor: Simplify comments in sasl.js and remove unused test cases Updated comments in sasl.js to clarify the password normalization process and removed redundant test cases from vitest-cf.test.ts, streamlining the codebase. * Remove redundant NFKC-only SASLprep test Confirmed in pull request comments that the “macOS/iOS” thing was an AI inventing an unneeded justification, and NFKC is already covered by another test. * fix: SASLprep zero-width space the same way PostgreSQL does As mentioned in the test comment, RFC 3454 defines appendix B for mapping tables and appendix C for prohibition tables. RFC 4013 SASLprep is probably misusing that list of non-ASCII spaces, and says nothing about the overlap. (At least it’s obsoleted.) * cleanup: Simplify regex character classes with ranges --------- Co-authored-by: charmander <charmander@noreply.github.com> Co-authored-by: Charmander <~@charmander.me> --------- Co-authored-by: Charmander <~@charmander.me> Co-authored-by: charmander <charmander@noreply.github.com>
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
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.
No description provided.