Skip to content

chore: make integration tests 60x faster - #22

Open
pascal-botpress wants to merge 3 commits into
pb/rds-replicationfrom
pb/faster-ci
Open

chore: make integration tests 60x faster#22
pascal-botpress wants to merge 3 commits into
pb/rds-replicationfrom
pb/faster-ci

Conversation

@pascal-botpress

Copy link
Copy Markdown
Member

I'm tired of waiting

Copilot AI lite review requested due to automatic review settings August 28, 2026 20:42

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR speeds up the integration test suite by avoiding per-test container startup: it reuses a single Postgres container across all integration tests in the file (resetting state between tests) and replaces LocalStack Secrets Manager with a lightweight in-process HTTP stub. It also updates the pnpm test script to build once and run all Vitest projects in a single invocation.

Changes:

  • Rework lambda.integration.test.ts to use beforeAll/afterAll shared fixtures and reset the DB between tests.
  • Add reusable test fixtures: startPostgresCluster() (testcontainers + fast PG config) and startFakeSecretsManager() (minimal Secrets Manager stub).
  • Update package.json test script to build once + vitest --run.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
cdk-postgresql/test/lambda.integration.test.ts Switch to shared Postgres + in-process Secrets Manager stub to reduce per-test startup cost
cdk-postgresql/test/fixtures/postgres-cluster.ts New Postgres fixture that starts once and provides a reset() routine between tests
cdk-postgresql/test/fixtures/fake-secrets-manager.ts New lightweight HTTP server implementing only the Secrets Manager operations used by tests
cdk-postgresql/package.json Run a single build + single Vitest run for all projects
Suppressed comments (1)

cdk-postgresql/test/lambda.integration.test.ts:68

  • cluster.reset() returns a Promise, but this beforeEach hook isn't async/awaiting it. That means the next test can start running while the database is still being wiped, causing cross-test contamination and flakiness (especially with replication slots / DB drops).
// The database container is shared by every test in this file, so each test
// starts from a blank cluster:
beforeEach(() => cluster.reset());

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +60 to +63
afterAll(async () => {
vi.unstubAllEnvs();
await pgContainer?.stop();
await localstackContainer?.stop();
secretsManagerServer?.close();
await cluster?.stop();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed by awaiting the callback-based close() in teardown so the fake Secrets Manager server fully stops before the next run. Commit 3078c36.

@greptile-apps

greptile-apps Bot commented Aug 28, 2026

Copy link
Copy Markdown

Greptile Summary

The PR speeds up integration tests by sharing one PostgreSQL container and replacing LocalStack with an in-process Secrets Manager stand-in.

  • Resets the shared PostgreSQL cluster before each test.
  • Starts and stops shared test infrastructure at suite boundaries.
  • Runs all Vitest projects through a single package test command.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
cdk-postgresql/package.json Consolidates the package test command into one build followed by a complete Vitest run.
cdk-postgresql/test/fixtures/fake-secrets-manager.ts Adds an in-process HTTP stand-in for the two Secrets Manager operations used by the handlers.
cdk-postgresql/test/fixtures/postgres-cluster.ts Adds a reusable PostgreSQL test container with explicit per-test state cleanup and suite-level shutdown.
cdk-postgresql/test/lambda.integration.test.ts Reuses shared PostgreSQL and Secrets Manager fixtures while resetting database state before each integration test.

Sequence Diagram

sequenceDiagram
  participant Suite as Integration suite
  participant Secrets as Fake Secrets Manager
  participant PG as Shared PostgreSQL
  participant Test as Each test
  Suite->>Secrets: Start once
  Suite->>PG: Start once
  Test->>PG: Reset cluster
  Test->>Secrets: Store/read test secrets
  Test->>PG: Exercise handlers and verify state
  Suite->>Secrets: Stop after suite
  Suite->>PG: Stop after suite
Loading

Reviews (3): Last reviewed commit: "fix: reject fake secrets manager startup..." | Re-trigger Greptile

Comment thread cdk-postgresql/test/fixtures/postgres-cluster.ts
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.

3 participants