test(node): Fix flaky postgresjs integration test#20351
test(node): Fix flaky postgresjs integration test#20351
Conversation
JPeer264
changed the title
JPeer264
force-pushed
the
jp/node-integration-flake
branch
from
f9fbd1d to
96ef6a3
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
size-limit report 📦
|
The test was flaky because it used `readyMatches: ['port 5432']` to detect
when PostgreSQL was ready. However, PostgreSQL logs "port 5432" when it starts
listening, but before it's fully ready to accept connections.
On CI runners under load, the gap between binding to the port and being ready
to serve queries was long enough that tests sometimes received PostgreSQL error
57P03 ("cannot_connect_now") because the database was still initializing.
Changed to wait for "database system is ready to accept connections" which
PostgreSQL logs when it's actually ready to serve queries.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
JPeer264
force-pushed
the
jp/node-integration-flake
branch
from
fd55ebc to
9bbdd3c
Compare
|
closed in favor of #20429 |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| .withDockerCompose({ | ||
| workingDirectory: [__dirname], | ||
| readyMatches: ['postgresjs-ready'], | ||
| }) |
There was a problem hiding this comment.
Bug: The readyMatches parameter is passed to a test runner that does not support it. The parameter is silently ignored, making the code misleading and ineffective.
Severity: LOW
Suggested Fix
Remove the readyMatches: ['postgresjs-ready'] parameter from the startDocker call in the test file. The container readiness is already correctly handled by the docker-compose.yml healthcheck and the --wait flag used by the runner.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
dev-packages/node-integration-tests/suites/tracing/postgresjs/test.ts#L221-L224
Potential issue: The test configuration passes a `readyMatches` parameter to the
`node-integration-tests` runner. However, the runner's `DockerOptions` interface does
not define or use this property, causing the parameter to be silently ignored at
runtime. This makes the code misleading as it appears to configure a readiness check
that is not actually performed. The test's stability relies solely on the `docker
compose up --wait` command and the service's healthcheck, not this parameter.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 9bbdd3c. Configure here.
| .withDockerCompose({ | ||
| workingDirectory: [__dirname], | ||
| readyMatches: ['postgresjs-ready'], | ||
| }) |
There was a problem hiding this comment.
readyMatches not supported by this test runner
High Severity
The readyMatches option is passed to withDockerCompose but the DockerOptions interface in the node-integration-tests runner (dev-packages/node-integration-tests/utils/runner.ts) does not include a readyMatches property — it only supports workingDirectory and setupCommand. This feature only exists in the node-core-integration-tests runner. The runner uses docker compose up -d --wait and never reads container logs for match strings, so the entire ready-gate + readyMatches pattern has no effect here and the PR's stated fix (waiting for postgresjs-ready) is not actually achieved.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 9bbdd3c. Configure here.
| depends_on: | ||
| db: | ||
| condition: service_healthy | ||
| command: ['echo', 'postgresjs-ready'] |
There was a problem hiding this comment.
ready-gate service missing required Docker healthcheck
High Severity
The new ready-gate service does not define a healthcheck:. The node-integration-tests runner uses docker compose up -d --wait, which relies on healthchecks to know when services are ready. Since ready-gate runs echo and exits immediately, it will be in an "exited" state when --wait checks it, potentially causing the command to fail and throwing an error in the runner.
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 9bbdd3c. Configure here.
3 participants
While releasing a new SDK version I've seen a flaky integration test: https://github.com/getsentry/sentry-javascript/actions/runs/24508519594/job/71634188547?pr=20348
The error actually showed
"db.response.status_code": "57P03",which means that the DB is not yet ready: https://www.postgresql.org/docs/current/errcodes-appendix.htmlHope this fixes it. Addition from Claude:
The test was flaky because it used
readyMatches: ['port 5432']to detectwhen PostgreSQL was ready. However, PostgreSQL logs "port 5432" when it starts
listening, but before it's fully ready to accept connections.
On CI runners under load, the gap between binding to the port and being ready
to serve queries was long enough that tests sometimes received PostgreSQL error
57P03 ("cannot_connect_now") because the database was still initializing.
Changed to wait for "database system is ready to accept connections" which
PostgreSQL logs when it's actually ready to serve queries.