diff --git a/.agents/skills/constructive-testing/SKILL.md b/.agents/skills/constructive-testing/SKILL.md index 68f7cabbe0..cd05e9bbd4 100644 --- a/.agents/skills/constructive-testing/SKILL.md +++ b/.agents/skills/constructive-testing/SKILL.md @@ -76,6 +76,7 @@ Choose the **highest-level framework** that fits your test scenario: 2. **Never create `new pg.Pool()` or `new pg.Client()` in tests** — use `getConnections()` 3. **Never manually create/drop databases** — the framework handles this 4. **Never skip hooks** — tests will leak state +5. **Assert expected failures with `db.expectError(() => db.query(...), /pattern/)`** — it wraps the statement in a savepoint so the transaction stays usable. Never write a local `expectRejects`-style helper or raw `SAVEPOINT` / `ROLLBACK TO SAVEPOINT` SQL ([pgsql-test-exceptions.md](./references/pgsql-test-exceptions.md)) ## Reference Guide @@ -86,7 +87,7 @@ Choose the **highest-level framework** that fits your test scenario: | [pgsql-test.md](./references/pgsql-test.md) | pgsql-test overview and setup | Getting started with PostgreSQL testing | | [pgsql-test-rls.md](./references/pgsql-test-rls.md) | RLS policy testing | Testing row-level security, user isolation | | [pgsql-test-seeding.md](./references/pgsql-test-seeding.md) | Test data seeding | loadJson, loadSql, loadCsv fixtures | -| [pgsql-test-exceptions.md](./references/pgsql-test-exceptions.md) | Exception handling | Testing operations that should fail | +| [pgsql-test-exceptions.md](./references/pgsql-test-exceptions.md) | `db.expectError()` | Testing operations that should fail (RLS denials, constraint/permission errors, raised errors) | | [pgsql-test-snapshot.md](./references/pgsql-test-snapshot.md) | Snapshot testing | pruneIds, pruneDates, deterministic assertions | | [pgsql-test-helpers.md](./references/pgsql-test-helpers.md) | Helper utilities | Common test helper functions | | [pgsql-test-jwt-context.md](./references/pgsql-test-jwt-context.md) | JWT context testing | Setting JWT claims, testing authenticated queries | diff --git a/.agents/skills/constructive-testing/references/drizzle-orm-test.md b/.agents/skills/constructive-testing/references/drizzle-orm-test.md index b118e674d3..ffc69864d4 100644 --- a/.agents/skills/constructive-testing/references/drizzle-orm-test.md +++ b/.agents/skills/constructive-testing/references/drizzle-orm-test.md @@ -284,19 +284,16 @@ it('user cannot insert for another user', async () => { 'request.jwt.claim.sub': alice }); - const point = 'insert_other'; - await db.savepoint(point); - - await expect( - drizzleDb + // drizzleDb runs on db's connection, so db.expectError covers it + await db.expectError( + () => drizzleDb .insert(posts) .values({ title: 'Hacked Post', ownerId: bob - }) - ).rejects.toThrow(/permission denied|violates row-level security/); - - await db.rollback(point); + }), + /permission denied|violates row-level security/ + ); }); ``` @@ -369,22 +366,16 @@ it('user can delete own post', async () => { ## Handling Expected Failures -Use savepoint pattern with Drizzle: +Use `db.expectError()` with Drizzle: ```typescript it('anonymous cannot insert', async () => { db.setContext({ role: 'anonymous' }); - const point = 'anon_insert'; - await db.savepoint(point); - - await expect( - drizzleDb - .insert(posts) - .values({ title: 'Hacked' }) - ).rejects.toThrow(/permission denied/); - - await db.rollback(point); + await db.expectError( + () => drizzleDb.insert(posts).values({ title: 'Hacked' }), + /permission denied/ + ); }); ``` diff --git a/.agents/skills/constructive-testing/references/pgsql-test-exceptions.md b/.agents/skills/constructive-testing/references/pgsql-test-exceptions.md index c8a3977836..3270786043 100644 --- a/.agents/skills/constructive-testing/references/pgsql-test-exceptions.md +++ b/.agents/skills/constructive-testing/references/pgsql-test-exceptions.md @@ -1,222 +1,174 @@ --- name: pgsql-test-exceptions -description: Handle PostgreSQL aborted transactions when testing operations that should fail. Use when testing RLS policy violations, constraint errors, permission denied errors, or any expected database exceptions. Essential for security testing. +description: Assert that a statement fails without poisoning the test transaction — use db.expectError() from pgsql-test. Use when testing RLS policy violations, constraint errors, permission denied errors, raised application errors, or any expected database exception. Essential for security testing. compatibility: Node.js 18+, pgsql-test package, PostgreSQL metadata: author: constructive-io - version: "1.0.0" + version: "2.0.0" --- -# Testing Exceptions and Aborted Transactions +# Testing Exceptions: `db.expectError()` -Handle PostgreSQL's transaction abort behavior when testing operations that should fail. This is essential for security testing where you verify that unauthorized operations are rejected. +**Use `db.expectError()` for every statement that is supposed to fail.** Do not +write your own `expectRejects` / `expectRefused` / `expectRaise` helper, and do +not hand-roll `SAVEPOINT` / `ROLLBACK TO SAVEPOINT` around `expect(...).rejects`. +The helper already exists on every `PgTestClient` (`pg`, `db`, and the clients +handed out by `supabase-test`, `drizzle-orm-test`, `graphile-test`, …). -## When to Apply +## The Problem It Solves -Use this skill when: -- Testing RLS policy violations (user can't access other users' data) -- Testing constraint violations (unique, foreign key, check) -- Testing permission denied errors -- Testing any operation that should throw an error -- Verifying database state after a failed operation - -## The Problem - -When PostgreSQL encounters an error inside a transaction, it aborts the entire transaction. The connection rejects all further commands until you explicitly end the transaction: +When a statement fails inside a transaction, PostgreSQL aborts the whole +transaction. Every test runs inside one (`db.beforeEach()`), so after a naive +`await expect(db.query(...)).rejects.toThrow()` the next query fails with: ``` current transaction is aborted, commands ignored until end of transaction block ``` -This breaks naive exception testing: +## Usage ```typescript -// THIS WILL FAIL it('users cannot insert pets for other users', async () => { - db.setContext({ - role: 'authenticated', - 'jwt.claims.user_id': bob - }); + db.setContext({ role: 'authenticated', 'jwt.claims.user_id': bob }); - await expect( - db.query(`INSERT INTO pets (name, owner_id) VALUES ('Fake', $1)`, [alice]) - ).rejects.toThrow(/violates row-level security/); + await db.expectError( + () => db.query(`INSERT INTO pets (name, owner_id) VALUES ('Fake', $1)`, [alice]), + /violates row-level security/ + ); - // ERROR: transaction is aborted, this query fails! - const count = await db.query(`SELECT COUNT(*) FROM pets WHERE owner_id = $1`, [alice]); + // The transaction is still usable — verify the attack had no effect + const count = await db.one(`SELECT count(*) FROM pets WHERE owner_id = $1`, [alice]); + expect(Number(count.count)).toBe(2); }); ``` -## The Solution: Savepoints - -Create a savepoint before the failing operation, then roll back to it: +### Signature ```typescript -it('users cannot insert pets for other users', async () => { - db.setContext({ - role: 'authenticated', - 'jwt.claims.user_id': bob - }); - - // 1. Create savepoint before expected failure - await db.savepoint('insert_attempt'); - - // 2. Test the operation that should fail - await expect( - db.query(`INSERT INTO pets (name, owner_id) VALUES ('Fake', $1)`, [alice]) - ).rejects.toThrow(/violates row-level security/); - - // 3. Roll back to clear the error state - await db.rollback('insert_attempt'); - - // 4. Continue with verification queries - const count = await db.query(`SELECT COUNT(*) FROM pets WHERE owner_id = $1`, [alice]); - expect(parseInt(count.rows[0].count)).toBe(2); -}); +db.expectError( + run: () => Promise, + expected?: string | RegExp | { code?: string; message?: string | RegExp } +): Promise // resolves to the caught error ``` -## Pattern Template +| `expected` | Matches when | +|---|---| +| omitted | `run` throws anything | +| `'duplicate key'` | the error message contains the string | +| `/permission denied/` | the regex matches the error message | +| `{ code: '42501' }` | the SQLSTATE (`err.code`) is exactly `42501` | +| `{ code: '23505', message: /email/ }` | both match | + +It **throws** (failing the test) when `run` succeeds or the error does not match, +with the expected matcher and the actual `code: message` in the failure. +It **resolves** to the caught error, so you can assert on more fields: ```typescript -// 1. Create savepoint -await db.savepoint('my_savepoint_name'); +const err = await db.expectError(() => db.query(`SELECT app.transfer($1)`, [-5])); +expect(err.code).toBe('P0001'); +expect(err.message).toMatch(/^INVALID_AMOUNT/); +``` -// 2. Execute operation that should fail -await expect( - db.query(`...`) -).rejects.toThrow(/expected error pattern/); +### What It Does -// 3. Roll back to savepoint -await db.rollback('my_savepoint_name'); - -// 4. Continue with additional queries -const result = await db.query(`...`); -``` +1. `SAVEPOINT` on this client +2. runs `run()` +3. `ROLLBACK TO SAVEPOINT` — **always**, in a `finally`, so the aborted state is + cleared even when the assertion fails, and an unexpected success leaves no rows +4. `RELEASE SAVEPOINT`, so the call nets out to nothing and can be repeated or + nested freely ## Common Scenarios ### RLS Policy Violations ```typescript -it('users cannot modify other users data', async () => { +it('users cannot hand their item to another user', async () => { db.setContext({ role: 'authenticated', 'jwt.claims.user_id': bob }); + await db.expectError( + () => db.query(`UPDATE items SET owner_id = $1 WHERE owner_id = $2`, [alice, bob]), + /violates row-level security/ // WITH CHECK rejects the new row + ); - await db.savepoint('update_attempt'); - await expect( - db.query(`UPDATE items SET name = 'stolen' WHERE owner_id = $1`, [alice]) - ).rejects.toThrow(/violates row-level security/); - await db.rollback('update_attempt'); - - // Verify data unchanged - db.setContext({ role: 'authenticated', 'jwt.claims.user_id': alice }); - const item = await db.query(`SELECT name FROM items WHERE owner_id = $1`, [alice]); - expect(item.rows[0].name).toBe('original'); + const item = await db.one(`SELECT owner_id FROM items WHERE name = 'bobs'`); + expect(item.owner_id).toBe(bob); }); ``` -### Permission Denied +> An `UPDATE`/`DELETE` that RLS filters to zero rows does **not** error — it +> affects 0 rows. Assert `rowCount === 0` for those; use `expectError` only when +> the policy raises (`WITH CHECK` on `INSERT`/`UPDATE`, or a permission error). + +### Permission Denied — Several In A Row ```typescript it('anonymous users cannot modify data', async () => { db.setContext({ role: 'anonymous' }); - await db.savepoint('anon_insert'); - await expect( - db.query(`INSERT INTO users (name) VALUES ('hacker')`) - ).rejects.toThrow(/permission denied/); - await db.rollback('anon_insert'); - - await db.savepoint('anon_update'); - await expect( - db.query(`UPDATE users SET name = 'hacked'`) - ).rejects.toThrow(/permission denied/); - await db.rollback('anon_update'); - - await db.savepoint('anon_delete'); - await expect( - db.query(`DELETE FROM users`) - ).rejects.toThrow(/permission denied/); - await db.rollback('anon_delete'); + await db.expectError(() => db.query(`INSERT INTO users (name) VALUES ('hacker')`), /permission denied/); + await db.expectError(() => db.query(`UPDATE users SET name = 'hacked'`), /permission denied/); + await db.expectError(() => db.query(`DELETE FROM users`), /permission denied/); }); ``` -### Constraint Violations +### Constraint Violations (match by SQLSTATE) ```typescript it('rejects duplicate emails', async () => { await db.query(`INSERT INTO users (email) VALUES ('test@example.com')`); - await db.savepoint('duplicate_email'); - await expect( - db.query(`INSERT INTO users (email) VALUES ('test@example.com')`) - ).rejects.toThrow(/duplicate key value violates unique constraint/); - await db.rollback('duplicate_email'); + await db.expectError( + () => db.query(`INSERT INTO users (email) VALUES ('test@example.com')`), + { code: '23505' } // unique_violation + ); - // Verify only one user exists - const count = await db.query(`SELECT COUNT(*) FROM users`); - expect(parseInt(count.rows[0].count)).toBe(1); + const count = await db.one(`SELECT count(*) FROM users`); + expect(Number(count.count)).toBe(1); }); ``` -### PLPGSQL Function Validation +### PL/pgSQL Raised Errors ```typescript -describe('plpgsql_expr', () => { - it('rejects NULL query', async () => { - await db.savepoint('null_query'); - await expect( - db.any(`SELECT my_function(NULL)`) - ).rejects.toThrow('query cannot be NULL'); - await db.rollback('null_query'); - }); - - it('rejects invalid input', async () => { - await db.savepoint('invalid_input'); - await expect( - db.any(`SELECT my_function('{"invalid": true}'::jsonb)`) - ).rejects.toThrow('invalid input format'); - await db.rollback('invalid_input'); - }); +it('rejects NULL query', async () => { + await db.expectError(() => db.any(`SELECT my_function(NULL)`), 'query cannot be NULL'); }); ``` -### Multiple Failure Tests in Sequence +### A Shared Call With Fixed Arguments + +Wrap the *call*, not the savepoint logic: ```typescript -it('validates all input constraints', async () => { - // Test 1: null name - await db.savepoint('null_name'); - await expect( - db.query(`INSERT INTO products (name, price) VALUES (NULL, 10)`) - ).rejects.toThrow(/null value in column "name"/); - await db.rollback('null_name'); - - // Test 2: negative price - await db.savepoint('negative_price'); - await expect( - db.query(`INSERT INTO products (name, price) VALUES ('item', -5)`) - ).rejects.toThrow(/violates check constraint/); - await db.rollback('negative_price'); - - // Test 3: valid insert works - await db.query(`INSERT INTO products (name, price) VALUES ('item', 10)`); - const result = await db.query(`SELECT * FROM products`); - expect(result.rows).toHaveLength(1); -}); +const record = (client: PgTestClient, rows: unknown[]) => + client.query(`SELECT app.record($1::jsonb)`, [JSON.stringify(rows)]); + +await db.expectError(() => record(db, [bad]), /FORBIDDEN/); +await pg.expectError(() => record(pg, [{ count: 0 }]), /count_chk/); ``` -## Key Rules +## Rules -1. **Always use unique savepoint names** - Avoid conflicts between tests -2. **Roll back immediately after the expected failure** - Before any other queries -3. **Use descriptive savepoint names** - Makes debugging easier -4. **Each failure needs its own savepoint** - Can't reuse savepoints after rollback +1. **Call `expectError` on the client that runs the statement.** `db.expectError(() => db.query(...))`, + `pg.expectError(() => pg.query(...))`. A savepoint on `db` cannot rescue an + aborted transaction on `pg` — they are separate connections. +2. **Never hand-roll it.** No local `expectRejects`/`expectRefused`/`expectRaise` + helpers, no `pg.query('SAVEPOINT ...')` / `ROLLBACK TO SAVEPOINT` strings, no + `db.savepoint()` → `expect().rejects` → `db.rollback()` triples. +3. **Match something specific.** Prefer a SQLSTATE `{ code }` or the error code + your function raises over a bare `expectError(fn)`, so the test cannot pass on + an unrelated failure (a typo, a missing grant). +4. **Verify state afterwards.** The point of keeping the transaction alive is to + prove the rejected operation had no effect. -## Why This Matters +## When You Still Need `savepoint()` / `rollback()` -Without savepoints, you cannot verify database state after a failed operation. That verification is often the most important part of a security test - confirming that the malicious operation had no effect. +Only for multi-statement scenarios that are *not* "this should fail" — e.g. +trying an operation and then undoing it on purpose. `db.savepoint(name)` and +`db.rollback(name)` remain available; a name can be reused, and +`ROLLBACK TO SAVEPOINT` keeps the savepoint, so you can roll back to it again. ## References +- Related: [pgsql-test-rls.md](./pgsql-test-rls.md) for RLS testing - Related skill: `pgpm` (`references/testing.md`) for general test setup -- Related skill: `pgpm` (`references/env.md`) for environment configuration diff --git a/.agents/skills/constructive-testing/references/pgsql-test-rls.md b/.agents/skills/constructive-testing/references/pgsql-test-rls.md index 9861d265c9..656795ed65 100644 --- a/.agents/skills/constructive-testing/references/pgsql-test-rls.md +++ b/.agents/skills/constructive-testing/references/pgsql-test-rls.md @@ -148,18 +148,14 @@ it('user cannot insert for another user', async () => { 'request.jwt.claim.sub': user1Id }); - // Use savepoint pattern for expected failures - const point = 'insert_other_user'; - await db.savepoint(point); - - await expect( - db.query(` + // expectError keeps the transaction usable after the expected failure + await db.expectError( + () => db.query(` INSERT INTO app.posts (title, owner_id) VALUES ('Hacked Post', $1) - `, [user2Id]) - ).rejects.toThrow(/permission denied|violates row-level security/); - - await db.rollback(point); + `, [user2Id]), + /permission denied|violates row-level security/ + ); }); ``` @@ -250,12 +246,10 @@ it('anonymous users have read-only access', async () => { expect(result.rows).toHaveLength(1); // Cannot modify - const point = 'anon_insert'; - await db.savepoint(point); - await expect( - db.query(`INSERT INTO app.public_posts (title) VALUES ('Hacked')`) - ).rejects.toThrow(/permission denied/); - await db.rollback(point); + await db.expectError( + () => db.query(`INSERT INTO app.public_posts (title) VALUES ('Hacked')`), + /permission denied/ + ); }); ``` @@ -304,20 +298,16 @@ describe('multi-user isolation', () => { ## Handling Expected Failures -When testing operations that should fail, use the savepoint pattern to avoid "current transaction is aborted" errors: +When testing operations that should fail, use `db.expectError()` to avoid "current transaction is aborted" errors (see [pgsql-test-exceptions.md](./pgsql-test-exceptions.md)): ```typescript it('rejects unauthorized access', async () => { db.setContext({ role: 'anonymous' }); - const point = 'unauthorized_access'; - await db.savepoint(point); - - await expect( - db.query('INSERT INTO app.private_data (secret) VALUES ($1)', ['hack']) - ).rejects.toThrow(/permission denied/); - - await db.rollback(point); + await db.expectError( + () => db.query('INSERT INTO app.private_data (secret) VALUES ($1)', ['hack']), + /permission denied/ + ); // Can continue using db connection const result = await db.query('SELECT 1 as ok'); diff --git a/.agents/skills/constructive-testing/references/pgsql-test.md b/.agents/skills/constructive-testing/references/pgsql-test.md index 0c4c62bc82..3670c7e8e1 100644 --- a/.agents/skills/constructive-testing/references/pgsql-test.md +++ b/.agents/skills/constructive-testing/references/pgsql-test.md @@ -86,19 +86,16 @@ expect(result.rows).toHaveLength(1); ## Handling Expected Failures (Savepoint Pattern) -When testing operations that should fail, use savepoints to avoid "current transaction is aborted" errors: +When testing operations that should fail, use `db.expectError()` to avoid "current transaction is aborted" errors (see [pgsql-test-exceptions.md](./pgsql-test-exceptions.md)): ```typescript it('rejects unauthorized access', async () => { db.setContext({ role: 'anonymous' }); - await db.savepoint('unauthorized_access'); - - await expect( - db.query('INSERT INTO app.private_data (secret) VALUES ($1)', ['hack']) - ).rejects.toThrow(/permission denied/); - - await db.rollback('unauthorized_access'); + await db.expectError( + () => db.query('INSERT INTO app.private_data (secret) VALUES ($1)', ['hack']), + /permission denied/ + ); // Connection still works const result = await db.query('SELECT 1 as ok'); diff --git a/.agents/skills/constructive-testing/references/supabase-test.md b/.agents/skills/constructive-testing/references/supabase-test.md index f1e2bb8c69..54412cf80c 100644 --- a/.agents/skills/constructive-testing/references/supabase-test.md +++ b/.agents/skills/constructive-testing/references/supabase-test.md @@ -197,20 +197,16 @@ it('user cannot see other users data', async () => { ### Testing Permission Denied -Use savepoint pattern for expected failures: +Use `db.expectError()` for expected failures: ```typescript it('anonymous cannot insert', async () => { db.setContext({ role: 'anon' }); - const point = 'anon_insert'; - await db.savepoint(point); - - await expect( - db.query(`INSERT INTO app.posts (title) VALUES ('Hacked')`) - ).rejects.toThrow(/permission denied/); - - await db.rollback(point); + await db.expectError( + () => db.query(`INSERT INTO app.posts (title) VALUES ('Hacked')`), + /permission denied/ + ); }); ``` diff --git a/.agents/skills/pgpm/SKILL.md b/.agents/skills/pgpm/SKILL.md index 7128012609..9fd1585080 100644 --- a/.agents/skills/pgpm/SKILL.md +++ b/.agents/skills/pgpm/SKILL.md @@ -338,7 +338,7 @@ In `noTty` mode, `inquirerer` uses defaults where available and throws with the | Can't connect to database | `pg_isready -h $PGHOST -p $PGPORT`; if nothing is listening, `pgpm docker start && eval "$(pgpm env)"` | | `PGHOST` not set | `eval "$(pgpm env)"` — must use `eval`, not run in subshell | | Developer already runs Postgres on 5432 | Use it: `export PGHOST=localhost PGPORT=5432 PGUSER= PGPASSWORD=...` then `pgpm admin-users bootstrap --yes`. No Docker needed | -| Transaction aborted in tests | Use `db.beforeEach()` / `db.afterEach()` savepoint pattern | +| `current transaction is aborted` after an expected failure | Wrap the failing call: `await db.expectError(() => db.query(...), /pattern/)` — never hand-roll `SAVEPOINT`/`ROLLBACK TO` | | Tests interfere with each other | Ensure every test file has `beforeEach`/`afterEach` hooks | | Module not found during deploy | Verify `.control` file exists and workspace structure is correct | | Dependency not found | Check `.control` `requires` uses control names, not npm names | diff --git a/.agents/skills/pgpm/references/testing.md b/.agents/skills/pgpm/references/testing.md index 047cbd0be1..21129d5132 100644 --- a/.agents/skills/pgpm/references/testing.md +++ b/.agents/skills/pgpm/references/testing.md @@ -76,6 +76,7 @@ Returns: | `db.query(sql, params?)` | Execute SQL query | | `db.beforeEach()` | Start savepoint (call in beforeEach) | | `db.afterEach()` | Rollback to savepoint (call in afterEach) | +| `db.expectError(fn, pattern?)` | Assert `fn` fails (message substring/RegExp or `{ code }`), keeping the transaction usable | | `db.setContext(context)` | Set session context variables | | `db.getPool()` | Get underlying pg Pool | @@ -285,6 +286,7 @@ beforeAll(async () => { | "Database does not exist" | Check PGDATABASE env var or use `pgpm env` | | Tests hang | Ensure `teardown()` is called in afterAll | | Data leaking between tests | Add `beforeEach/afterEach` savepoint calls | +| `current transaction is aborted` | Assert the failing statement with `db.expectError()` | | Permission denied | Check database user has CREATE DATABASE permission | | Slow tests | Use savepoints instead of recreating database per test | diff --git a/.agents/skills/pgpm/references/troubleshooting.md b/.agents/skills/pgpm/references/troubleshooting.md index 3eaf6381cd..76eff2eb79 100644 --- a/.agents/skills/pgpm/references/troubleshooting.md +++ b/.agents/skills/pgpm/references/troubleshooting.md @@ -134,46 +134,21 @@ current transaction is aborted, commands ignored until end of transaction block **Cause:** An error occurred in the transaction, and PostgreSQL marks the entire transaction as aborted. All subsequent queries fail until the transaction ends. -**Solution:** Use savepoints when testing operations that should fail: +**Solution:** Assert the failing statement with `db.expectError()` — it runs it inside a savepoint and rolls back to it, so the transaction stays usable: -```typescript -// Before the expected failure -const point = 'my_savepoint'; -await db.savepoint(point); - -// Operation that should fail -await expect( - db.query('INSERT INTO restricted_table ...') -).rejects.toThrow(/permission denied/); - -// After the failure - rollback to savepoint -await db.rollback(point); - -// Now you can continue using the connection -const result = await db.query('SELECT 1'); -``` - -**Pattern for multiple failures:** ```typescript it('tests multiple failure scenarios', async () => { - // First failure - const point1 = 'first_failure'; - await db.savepoint(point1); - await expect(db.query('...')).rejects.toThrow(); - await db.rollback(point1); - - // Second failure - const point2 = 'second_failure'; - await db.savepoint(point2); - await expect(db.query('...')).rejects.toThrow(); - await db.rollback(point2); - - // Continue with passing assertions + await db.expectError(() => db.query('INSERT INTO restricted_table ...'), /permission denied/); + await db.expectError(() => db.query('INSERT INTO users (email) VALUES ($1)', [dup]), { code: '23505' }); + + // The connection is still usable const result = await db.query('SELECT 1'); expect(result.rows[0]).toBeDefined(); }); ``` +Call it on the client that runs the statement (`pg.expectError(() => pg.query(...))` for `pg`). Don't hand-roll `SAVEPOINT` / `ROLLBACK TO SAVEPOINT` or write a local wrapper. See the `constructive-testing` skill's `references/pgsql-test-exceptions.md`. + ### Tests Interfering with Each Other **Symptom:** Tests pass individually but fail when run together @@ -312,7 +287,7 @@ pgpm docker start |-------|-----------| | Can't connect | `pgpm docker start && eval "$(pgpm env)"` | | PGHOST not set | `eval "$(pgpm env)"` | -| Transaction aborted | Use savepoint pattern | +| Transaction aborted | `db.expectError(() => db.query(...), /pattern/)` | | Tests interfere | Check beforeEach/afterEach hooks | | Module not found | Verify workspace structure | | Port in use | `pg_isready -h localhost -p 5432` — if it's Postgres, use it (export PG*); else `pgpm docker start --port 5433` + `PGPORT=5433` | diff --git a/AGENTS.md b/AGENTS.md index 85d2237659..8857e1771f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -158,6 +158,7 @@ afterEach(async () => { - **Never** use `getPgPool()` from `pg-cache` in tests — that's for production connection pooling - **Never** manually create/drop databases in tests — `pgsql-test` handles this automatically - **Never** skip `beforeEach`/`afterEach` hooks — tests will leak state to each other +- **Never** hand-roll `SAVEPOINT` / `ROLLBACK TO SAVEPOINT` (or a local `expectRejects` helper) around a statement that should fail — use `await db.expectError(() => db.query(...), /pattern/)`, which keeps the test transaction usable afterwards - **Never** construct connection strings manually — use the env configuration system ## Tooling Skills diff --git a/postgres/pgsql-test/AGENTS.md b/postgres/pgsql-test/AGENTS.md index c9d06d892c..7c4ce8b70c 100644 --- a/postgres/pgsql-test/AGENTS.md +++ b/postgres/pgsql-test/AGENTS.md @@ -29,6 +29,10 @@ beforeEach(() => db.beforeEach()); afterEach(() => db.afterEach()); ``` +## Expected Failures + +Assert a statement that should fail with `db.expectError(() => db.query(...), /pattern/ | 'substring' | { code })`. It wraps the call in a savepoint and always rolls back to it, so the test transaction stays usable. Never hand-roll `SAVEPOINT` / `ROLLBACK TO SAVEPOINT` or a local `expectRejects` helper. + ## Transaction-Local Context `setContext()` uses `set_config('key', 'value', true)` — the `true` makes settings **transaction-local**. This means: @@ -52,6 +56,6 @@ await db.commit(); ## Where To Look - `postgres/pgsql-test/src/connect.ts` – connection/bootstrap logic -- `postgres/pgsql-test/src/test-client.ts` – `PgTestClient` helpers (`beforeEach`, `afterEach`, `setContext`, etc.) +- `postgres/pgsql-test/src/test-client.ts` – `PgTestClient` helpers (`beforeEach`, `afterEach`, `expectError`, `setContext`, etc.) - `postgres/pgsql-test/src/seed/*` – seed adapters - `postgres/pgsql-test/__tests__/*` – usage examples diff --git a/postgres/pgsql-test/README.md b/postgres/pgsql-test/README.md index e95513a090..04215d3a70 100644 --- a/postgres/pgsql-test/README.md +++ b/postgres/pgsql-test/README.md @@ -131,6 +131,7 @@ The `PgTestClient` returned by `getConnections()` wraps a `pg.Client` and provid * `beforeEach()` – Begins a transaction and sets a savepoint (called at the start of each test) * `afterEach()` – Rolls back to the savepoint and commits the outer transaction (cleans up test state) * `setContext({ key: value })` – Sets PostgreSQL config variables (like `role`) to simulate RLS contexts +* `expectError(run, expected?)` – Asserts `run` fails (message substring/RegExp, or `{ code, message }`) inside a savepoint, so the transaction stays usable afterwards; resolves to the caught error (see [Expected failures](#expected-failures)) * `checkConstraints()` – Runs the commit-time checks for any pending deferred constraints now, without committing (see [Deferred constraints](#deferred-constraints-under-rollback-isolation)) * `any`, `one`, `oneOrNone`, `many`, `manyOrNone`, `none`, `result` – Typed query helpers for specific result expectations @@ -604,6 +605,26 @@ This table documents the available options for the `getConnections` function. Th | `db.prefix` | `string` | `'db-'` | Prefix used when generating test database names | | `db.deferredConstraints` | `'off' \| 'check' \| 'immediate'` | `'off'` | How `DEFERRABLE INITIALLY DEFERRED` constraints are handled under rollback isolation (env: `DB_DEFERRED_CONSTRAINTS`). See below. | +### Expected failures + +A failed statement aborts the surrounding transaction, so after a plain `expect(...).rejects` every later query in the test fails with `current transaction is aborted`. Use `expectError()` instead — it wraps the call in a savepoint, always rolls back to it, and throws if the call succeeds or fails differently: + +```ts +it('anonymous users cannot write', async () => { + db.setContext({ role: 'anonymous' }); + await db.expectError(() => db.query(`INSERT INTO app.posts (title) VALUES ('x')`), /permission denied/); + await db.expectError(() => db.query(`DELETE FROM app.posts`), { code: '42501' }); + + const err = await db.expectError(() => db.query(`SELECT app.transfer(-5)`)); + expect(err.message).toMatch(/^INVALID_AMOUNT/); + + // still usable + expect((await db.one(`SELECT count(*) FROM app.posts`)).count).toBe('1'); +}); +``` + +`expected` is a message substring, a `RegExp`, or `{ code?: string; message?: string | RegExp }` (`code` is the SQLSTATE). Run the statement on the same client you call `expectError` on. + ### Deferred constraints under rollback isolation `beforeEach()`/`afterEach()` isolate tests by rolling back instead of committing. Anything Postgres only does **at COMMIT** therefore never happens inside a test. The one that bites is `DEFERRABLE INITIALLY DEFERRED` constraints (FK / UNIQUE / EXCLUDE / `CONSTRAINT TRIGGER`): a test can leave a dangling deferred foreign key and still pass, because the check that would have failed the commit is discarded with the rollback. This is inherent to every rollback-based test harness, not specific to pgsql-test. diff --git a/postgres/pgsql-test/__tests__/postgres-test.expect-error.test.ts b/postgres/pgsql-test/__tests__/postgres-test.expect-error.test.ts new file mode 100644 index 0000000000..889e8725b8 --- /dev/null +++ b/postgres/pgsql-test/__tests__/postgres-test.expect-error.test.ts @@ -0,0 +1,101 @@ +process.env.LOG_SCOPE = 'pgsql-test'; + +import { getConnections } from '../src/connect'; +import { PgTestClient } from '../src/test-client'; + +let pg: PgTestClient; +let teardown: () => Promise; + +beforeAll(async () => { + ({ pg, teardown } = await getConnections()); +}); + +beforeEach(async () => { + await pg.beforeEach(); + await pg.query(`CREATE TABLE expect_error_items (id int PRIMARY KEY, name text NOT NULL)`); + await pg.query(`INSERT INTO expect_error_items VALUES (1, 'one')`); +}); + +afterEach(async () => { + await pg.afterEach(); +}); + +afterAll(async () => { + await teardown(); +}); + +const count = async () => + Number((await pg.one(`SELECT count(*) AS n FROM expect_error_items`)).n); + +describe('PgTestClient.expectError', () => { + it('keeps the transaction usable after the expected failure', async () => { + await pg.expectError( + () => pg.query(`INSERT INTO expect_error_items VALUES (1, 'dup')`), + /duplicate key value/ + ); + + expect(await count()).toBe(1); + await pg.query(`INSERT INTO expect_error_items VALUES (2, 'two')`); + expect(await count()).toBe(2); + }); + + it('can be called repeatedly in one test', async () => { + await pg.expectError(() => pg.query(`INSERT INTO expect_error_items VALUES (1, 'a')`), 'duplicate key'); + await pg.expectError(() => pg.query(`INSERT INTO expect_error_items VALUES (3, NULL)`), /not-null/); + await pg.expectError(() => pg.query(`SELECT 1/0`), { code: '22012' }); + + expect(await count()).toBe(1); + }); + + it('returns the caught error', async () => { + const err = await pg.expectError(() => pg.query(`SELECT 1/0`)); + expect(err.code).toBe('22012'); + }); + + it('matches code and message together', async () => { + await pg.expectError(() => pg.query(`SELECT 1/0`), { code: '22012', message: /division by zero/ }); + await expect( + pg.expectError(() => pg.query(`SELECT 1/0`), { code: '22012', message: /nope/ }) + ).rejects.toThrow(/expectError: expected .*got 22012: division by zero/); + }); + + it('fails when the statement succeeds, and rolls the success back', async () => { + await expect( + pg.expectError(() => pg.query(`INSERT INTO expect_error_items VALUES (2, 'two')`), /duplicate/) + ).rejects.toThrow(/expectError: expected \/duplicate\/, but the statement succeeded/); + + expect(await count()).toBe(1); + }); + + it('fails when the error does not match, and leaves the transaction usable', async () => { + const err = await pg + .expectError(() => pg.query(`SELECT 1/0`), /permission denied/) + .catch((e: Error) => e); + + expect((err as Error).message).toMatch(/expected \/permission denied\/, got 22012: division by zero/); + expect(((err as Error).cause as any).code).toBe('22012'); + expect(await count()).toBe(1); + }); + + it('keeps work done before the call and nests with a caller savepoint of the same name', async () => { + await pg.query(`INSERT INTO expect_error_items VALUES (2, 'two')`); + await pg.savepoint('pgsql_test_expect_error'); + await pg.query(`INSERT INTO expect_error_items VALUES (3, 'three')`); + + await pg.expectError(() => pg.query(`SELECT 1/0`), { code: '22012' }); + expect(await count()).toBe(3); + + await pg.rollback('pgsql_test_expect_error'); + expect(await count()).toBe(2); + }); + + it('reverts setContext-free session state set inside the failed block', async () => { + await pg.expectError(async () => { + await pg.query(`SELECT set_config('app.flag', 'on', true)`); + await pg.query(`SELECT 1/0`); + }, { code: '22012' }); + + const { flag } = await pg.one(`SELECT current_setting('app.flag', true) AS flag`); + expect(flag || null).toBeNull(); + }); +}); diff --git a/postgres/pgsql-test/src/test-client.ts b/postgres/pgsql-test/src/test-client.ts index 47d730c7b0..202849e224 100644 --- a/postgres/pgsql-test/src/test-client.ts +++ b/postgres/pgsql-test/src/test-client.ts @@ -24,6 +24,39 @@ export type PgTestClientOpts = PgClientOpts & { }; const IN_FAILED_SQL_TRANSACTION = '25P02'; +const EXPECT_ERROR_SAVEPOINT = 'pgsql_test_expect_error'; + +/** + * What an expected error must match: a substring or RegExp of the message, or + * an object checking the SQLSTATE `code` and/or the `message`. + */ +export type ExpectedPgError = + | string + | RegExp + | { code?: string; message?: string | RegExp }; + +const describeExpected = (expected?: ExpectedPgError): string => { + if (expected === undefined) return 'an error'; + if (typeof expected === 'string' || expected instanceof RegExp) return String(expected); + return JSON.stringify({ + code: expected.code, + message: expected.message === undefined ? undefined : String(expected.message) + }); +}; + +const matchesText = (text: string, pattern: string | RegExp): boolean => + typeof pattern === 'string' ? text.includes(pattern) : pattern.test(text); + +const matchesExpected = (err: any, expected?: ExpectedPgError): boolean => { + if (expected === undefined) return true; + const message = String(err?.message ?? ''); + if (typeof expected === 'string' || expected instanceof RegExp) { + return matchesText(message, expected); + } + if (expected.code !== undefined && err?.code !== expected.code) return false; + if (expected.message !== undefined && !matchesText(message, expected.message)) return false; + return true; +}; export class PgTestClient extends PgClient { protected testOpts: PgTestClientOpts; @@ -84,6 +117,45 @@ export class PgTestClient extends PgClient { if (violation) throw violation; } + /** + * Assert that `run` fails, and keep the test transaction usable afterwards. + * + * Wraps `run` in a savepoint on this client, rolls back to it whatever happens + * (so the aborted statement, or an unexpected success, leaves no trace), then + * releases it. Throws if `run` succeeds or its error does not match `expected`; + * otherwise returns the caught error for further assertions. + * + * `run` must execute on this same client — a savepoint cannot rescue another + * connection's aborted transaction. + * + * @example + * await db.expectError(() => db.query('INSERT INTO app.secrets ...'), /permission denied/); + * const err = await db.expectError(() => db.query('...'), { code: '23505' }); + */ + async expectError(run: () => Promise, expected?: ExpectedPgError): Promise { + await this.savepoint(EXPECT_ERROR_SAVEPOINT); + let caught: any; + let failed = false; + try { + await run(); + } catch (err) { + failed = true; + caught = err; + } finally { + await this.rollback(EXPECT_ERROR_SAVEPOINT); + await this.client.query(`RELEASE SAVEPOINT "${EXPECT_ERROR_SAVEPOINT}";`); + } + + if (!failed) { + throw new Error(`[pgsql-test] expectError: expected ${describeExpected(expected)}, but the statement succeeded`); + } + if (!matchesExpected(caught, expected)) { + const got = [caught?.code, String(caught?.message ?? caught).split('\n')[0]].filter(Boolean).join(': '); + throw new Error(`[pgsql-test] expectError: expected ${describeExpected(expected)}, got ${got}`, { cause: caught }); + } + return caught as E; + } + /** * Run the commit-time checks for every pending deferred constraint now, without committing. * Postgres checks all outstanding deferred constraint events when a constraint switches from