test(sql): use container.host instead of hardcoded localhost in 'Connect using uri'#31901
Conversation
…ect using uri' The compose helper returns the docker host (BUN_DOCKER_TEST_HOST aware) but this test built its URI with a hardcoded localhost, failing instantly against any remote docker daemon while every other test in the file passes.
|
Looking for one thing? Review this PR in Change Stack to search files, summaries, diffs, and code without losing your place. Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
Disabled knowledge base sources:
WalkthroughThe PostgreSQL connection test is updated to use a dynamically resolved Docker container host instead of a hardcoded localhost value when constructing the connection URI. ChangesDocker Container Host Integration
Possibly related PRs
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — straightforward test fix aligning with the existing container.host pattern.
Extended reasoning...
Overview
This PR changes a single line in test/js/sql/sql.test.ts, replacing a hardcoded @localhost: in the "Connect using uri" test's connection string with container.host. The file already uses container.host in 8 other places (and container.port is used on the very next line of this same test), so this just brings one straggler in line with the established pattern introduced in #31731.
Security risks
None. This is test-only code constructing a Postgres connection URI for an integration test against a Docker container. No production code, auth, crypto, or permission logic is touched.
Level of scrutiny
Low. It's a one-token mechanical substitution in a test file, matching the convention used everywhere else in the same file. The intent is clear (support remote Docker daemons via BUN_DOCKER_TEST_HOST), and on local setups container.host resolves to 127.0.0.1, so behavior is unchanged there.
Other factors
The PR description includes verification that the full suite now passes against a remote Docker daemon and remains green locally. No bugs were flagged by the bug-hunting pass, and there are no outstanding reviewer comments.
2 participants
What
One-line fix in
test/js/sql/sql.test.ts: the "Connect using uri" test built its connection string with a hardcoded@localhost:instead ofcontainer.host.Why
Every other test in the file uses
container.host(which respectsBUN_DOCKER_TEST_HOST, added in #31731), so they pass against any Docker setup — local or remote daemon. This one test only worked when the daemon is on the same machine. Against a remote daemon it fails instantly withPostgresError: Connection closed, and that single subtest marks the whole 845-test file as failed even though the other 844 pass.How verified
With this change, the full
sql.test.tspasses (843 pass / 2 todo / 0 fail) against a remote Docker daemon on macOS arm64, and behavior on a local daemon is unchanged (container.hostresolves to127.0.0.1there).