Skip to content

fix(test): close leaked database pools in server/tests - #796

Open
adamsiwiec1 wants to merge 1 commit into
CopilotKit:mainfrom
adamsiwiec1:fix/close-test-db-pools-785
Open

adamsiwiec1 wants to merge 1 commit into
CopilotKit:mainfrom
adamsiwiec1:fix/close-test-db-pools-785

Conversation

@adamsiwiec1

Copy link
Copy Markdown

Summary

Fixes #785 — server/tests files that open a createDatabase pool must give it back.

On main, ~98 files open a pool and only 43 called $client.close(). The other pools stay open for the whole bun test process. With TEST_POOL (max: 2), that sits against PostgreSQL's default 100-connection ceiling, so the next integration tests added fail with sorry, too many clients already for reasons unrelated to their change (as seen on #770).

What changed

  1. Close the remaining leaks. For each file that called createDatabase and never closed, added a trailing:

    afterAll(async () => {
      await database.$client.close();
    });

    Registered last so Bun's FIFO afterAll order runs that file's cleanup deletes before the close. Matches the idiom already used in identity-store.integration.test.ts. Files that already used $client.end() were left alone (they already release the pool).

  2. Guard test server/tests/database-pool-pairing.test.ts: any file that contains createDatabase( must also contain $client.close( or $client.end(. Pairing rule rather than a lint rule, as proposed in the issue.

  3. Sharded runner prose. scripts/test-server-sharded.ts was not on main (it lives on arcade-provider). Restored it with test:server:sharded and corrected the two misleading claims: slots are not exhausted "during the import phase", and the ceiling is not "never a reason to change source". Construction opens 0 connections; the first query takes the pool's max; closing pools is the right fix. Sharding remains useful for wall-clock.

Counts

  • 42 files gained a close (had neither close nor end)
  • 1 new guard test
  • 1 restored/corrected sharded runner + package.json script
  • 13 files already using $client.end() unchanged

Test plan

  • bun test server/tests/database-pool-pairing.test.ts (pass)
  • bun test server/tests/database.test.ts (pass)
  • Full bun test server/tests / test:server:sharded with TEST_DATABASE_URL (needs Postgres; not run in this environment)
  • CI tests job on this PR

55 files on main opened a createDatabase pool and never gave it back; with
TEST_POOL max: 2 that sits against PostgreSQL's 100-connection ceiling and
makes the next integration tests fail with "too many clients already".

- Close each formerly leaking pool with await database.$client.close() in a
  trailing afterAll so Bun's FIFO hooks run cleanup before the close.
- Add database-pool-pairing.test.ts so a file that opens a database must close it.
- Restore scripts/test-server-sharded.ts (was on arcade-provider, missing from
  main) with prose corrected: exhaustion tracks files that have run, and
  closing pools is the right fix; sharding stays for wall-clock.

Fixes CopilotKit#785

This branch has not been deployed

No deployments
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.

server/tests leaks database connections: 55 files open a pool and never close it

1 participant