Skip to content

ci: bump test service containers to latest majors (postgres 18, mysql 9, redis 8) - #33097

Open
robobun wants to merge 6 commits into
mainfrom
farm/e3947f67/bump-ci-service-containers
Open

ci: bump test service containers to latest majors (postgres 18, mysql 9, redis 8)#33097
robobun wants to merge 6 commits into
mainfrom
farm/e3947f67/bump-ci-service-containers

Conversation

@robobun

@robobun robobun commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

For 1.4.0, update the Docker service containers CI tests run against to the latest stable major of each.

Service Before After
postgres_plain / postgres_auth postgres:15 postgres:18
postgres_tls postgres:15.13 postgres:18
mysql_plain / mysql_tls mysql:8.4 mysql:9 (9.7, the current LTS)
mysql_native_password mysql:8.0 mysql:8.4
redis_plain redis:7-alpine redis:8-alpine

redis_unified, the container the Bun.redis suite actually uses, was already on redis:8-alpine (the latest Redis major), and minio was already :latest.

Compatibility fixes the bumps required

PostgreSQL 18 declares VOLUME /var/lib/postgresql (not /var/lib/postgresql/data). Since #33822 the postgres Dockerfiles bake a pre-initialized cluster into a build-time PGDATA, which was /var/lib/postgresql/pgdata-init. That path is now inside postgres:18's VOLUME, so build-time writes there would be discarded and the baked-data optimization would silently disappear. PGDATA moves to /var/lib/pgdata-init (outside the volume) in all three postgres Dockerfiles.

MySQL 8.4 removed --default-authentication-plugin, and MySQL 9.0 removed the mysql_native_password plugin entirely. So:

  • mysql_plain and mysql_tls go to mysql:9; nothing in their tests is auth-plugin sensitive.

  • mysql_native_password cannot go to 9.x without losing the only real-server coverage of Bun's mysql_native_password scramble and the AuthSwitchRequest path (test/js/sql/sql-mysql.auth.test.ts, which relies on the handshake advertising a different plugin than the caching_sha2_password user it creates). It moves to the 8.4 LTS, the last line that ships the plugin, and starts with

    --mysql-native-password=ON --authentication-policy=mysql_native_password,,
    

    which enables the plugin (off by default in 8.4) and keeps it as the default first-factor plugin so the handshake still advertises it. Since test/docker: bake mysql+postgres data dirs so cold start is ~2-3s, stop healthcheck wedge #33822 moved init-time config into Dockerfile.mysql-native-password, the two flags are applied both in its build-time RUN init and its CMD. Staying on mysql:8.0 was not an option: it has been dropped from the official-images build list, so it no longer receives updates.

Incompatibilities CI found

CI on the bumped containers surfaced two real PostgreSQL 18 incompatibilities in test/js/sql/sql.test.ts, failing on every Linux x64 lane. Both were tests that encoded PostgreSQL-15-specific facts. Fixed in b7183fd:

pg_database[] - null values hardcoded a 16-field pg_database record literal. The pg_database catalog gained daticurules in PostgreSQL 16 and dathasloginevt in 17 (which also renamed daticulocale to datlocale), so on 18 the literal's eighth field (PG15's datconnlimit = -1) lands on a boolean column and the cast fails:

PostgresError: invalid input syntax for type boolean: "-1"
  errno: "22P02", code: "ERR_POSTGRES_SERVER_ERROR"

The record is now sourced from the live server ((SELECT d FROM pg_database d WHERE datname = 'postgres')), so the test no longer pins a catalog shape and exercises what it is named for: NULL positions preserved around a record element in a pg_database[] array.

text[] - special character escaping depended on E'\v' NOT being an escape. PostgreSQL did not recognize \v in escape strings until version 18, so the test was actually decoding a literal letter v (and its comment said so). PostgreSQL 18 added \v, so the element became a real U+000B and the toEqual broke:

- "v",
+ "\u000b",

The element now uses the version-independent hex escape E'\x0b' and the expectation is a real "\v", which is what the test name claims to cover and is a stronger assertion than a plain letter v.

Both fixes were verified through Bun's decoder against a live PostgreSQL.

Cleanup this forced

test/harness.ts had a "mysql:8" / "mysql:9" alias layer that picked a compose service based on the caller's env field. It was already misleading ("mysql:9" resolved to the mysql:8.0 container), and after the bumps the labels would have inverted outright (the test labeled mysql:8 would run MySQL 9.7). The three call sites now name the compose service directly and the alias block is removed:

  • test/js/sql/sql-mysql.test.ts: the "MySQL 9" block becomes "MySQL with mysql_native_password" with image: "mysql_native_password". Its process.arch === "x64" guard is dropped; it dated from when that service was backed by mysql:8.0, and mysql:8.4 publishes amd64 and arm64 (sql-mysql.auth.test.ts already ran the same service ungated).
  • test/integration/mysql2/mysql2.test.ts: names the two services directly.

That deletion left describeWithContainer's env option with no reader, so it is removed from the signature along with args and archs, which were already unused. The five callers that passed no-op env: {} / args: [] objects are cleaned up too (sql-mysql.auth.test.ts, sql-mysql.helpers.test.ts, sql-mysql.transactions.test.ts, test/regression/issue/21311.test.ts, test/regression/issue/26030.test.ts).

Also deleted test/js/valkey/docker/ and test/js/valkey/docker-tls/: both FROM redis:7-alpine, referenced by nothing (not docker-compose.yml, not test/docker/index.ts, not any test), superseded by test/js/valkey/docker-unified/.

Two assertions in test/js/sql/sql-mysql.transactions.test.ts ("Transaction throws" and "Uncaught transaction request errors bubbles to transaction") were hardened. They pinned the server's exact error prose, which is the one field that is not stable: it changes across MySQL versions and differs on MariaDB, which the adapter also targets (mariadb:// URLs, and the BUN_TEST_SERVICE_* override lets the suite run against any MySQL-compatible server). They ignored errno, the MySQL wire-protocol error number, which is identical across MySQL 8/9 and MariaDB. They now assert code + errno (1366 ER_TRUNCATED_WRONG_VALUE_FOR_FIELD, 1054 ER_BAD_FIELD_ERROR) plus the identifying substring of the message, matching the existing expect(error.code) idiom in the same describe block. Strictly stronger than the prose-only assertion and robust to exactly the kind of change a container bump can cause.

Not done here

  • redis_plain is defined in compose and mapped by the harness but has zero test consumers today; bumped rather than deleted to keep this a version-bump PR.
  • None of the Bun.redis tests run against actual Valkey; they all use the official redis image. Valkey 9.1 is out. Covering it means either doubling the test/js/valkey/ suite or swapping the base image, so it is left for a separate decision.

Verification

Two full CI builds of the complete diff confirm the bumped containers work: #66936 and the ci: retrigger re-roll #66976. In both, postgres:18, mysql:9, mysql:8.4, and redis:8-alpine all came up and every database test suite passed on every Linux lane. The two sql.test.ts PostgreSQL 18 incompatibilities documented above went from red on all 7 Linux x64 lanes to green with the fixes.

#66976 has 283 of 286 jobs passing. The 3 failures are unrelated to this diff:

  • darwin 26 aarch64 - test-bun: buildkite-agent artifact download timed out after 120s for step 'darwin-aarch64-build-bun'. This infrastructure timeout has hit the same lane on every build of this PR.
  • alpine 3.23 x64 - test-bun and alpine 3.23 x64-baseline - test-bun: test/js/node/test/parallel/test-net-connect-memleak.js, a GC-observability assertion (assert.strictEqual(collected, true) after onGC) with no relationship to database containers. This is the known CI flake tracked in CI: test-net-connect-memleak.js fails on half of PR builds on linux-x64-musl since June 28 ~23:00 UTC #33044 and it is currently hitting most PR builds in the repo: the same annotation appears on 20 of the last 22 failed builds here, across unrelated branches.

I have stopped pushing so as not to keep re-rolling CI; the diff is ready.

Rebases

9fcc439e8b onto #33622, which reordered sql-mysql.test.ts's images array so mysql_tls runs last for startup ordering. Kept that order, dropped the now-dead .filter(Boolean), and updated the comment's mysql:9 to mysql_native_password to reflect the alias removal. Build #70079 afterwards (275/286 jobs passed) had no SQL, Redis, or docker-compose test in its failures; the red was repo-wide flakes that also hit other unrelated PR builds at the time (proxy-stress-concurrent.test.ts DNS-resolver LeakSanitizer on 7 builds, test-net-localport.js on 4, terminal.test.ts on 2) plus the darwin 26 aarch64 artifact-download timeout.

2b00a211eb onto #33822, a substantial test/docker/ rewrite that bakes pre-initialized data directories into the mysql/postgres images, moved the version pins from docker-compose.yml into new Dockerfile.mysql-plain and Dockerfile.mysql-native-password, removed the compose tmpfs/environment/command blocks those services used, and changed healthcheck intervals. Resolution:

  • docker-compose.yml: took main's build: structure everywhere; the per-service version bumps and the MySQL 8.4 flag rewrite move into the Dockerfiles instead.
  • The three postgres Dockerfiles' baked PGDATA=/var/lib/postgresql/pgdata-init is inside postgres:18's new VOLUME /var/lib/postgresql, so build-time writes there would be discarded and test/docker: bake mysql+postgres data dirs so cold start is ~2-3s, stop healthcheck wedge #33822's ~1-2s cold start would silently revert to a full initdb. PGDATA moves to /var/lib/pgdata-init in all three.
  • Dockerfile.mysql-plain: FROM mysql:8.4FROM mysql:9 (no other change; the baked-init approach is version-agnostic).
  • Dockerfile.mysql-native-password: FROM mysql:8.0FROM mysql:8.4, and --default-authentication-plugin=mysql_native_password--mysql-native-password=ON --authentication-policy=mysql_native_password,, in both the build-time RUN init and CMD.
  • test/js/sql/mysql-tls/Dockerfile: took main's restructured layout with ARG MYSQL_VERSION=9.

The full set of touched test files still passes locally after the rebase (50 pass, 0 fail). Build #71312 afterwards has 281/286 jobs passing with no SQL, Redis, or docker-compose test in its failures, so the PGDATA move and the Dockerfile bumps on top of #33822's baked-data-dir structure all work. Its 3 failures are test/js/node/test/sequential/test-net-localport.js on the two alpine 3.23 x64 lanes (same annotation on builds #71300, #71297, #71294, #71287, #71286, #71284, #71259; does not import harness) and test/js/sql/postgres-invalid-message-length.test.ts on windows 2019 x64 (a mock-server fault-injection test from #32769 that does not use docker; same annotation on builds #71313, #71300, #71297, #71294, #71291, #71290, #71287, #71286, #71284, #71259). This PR changes zero lines of src/.


[stamp-90s] gate passed · iteration 11 · 21 files touched

passes on PR (with fix)
Test-only change.

Debug/ASAN (expected pass):
$ bun bd test 'test/integration/mysql2/mysql2.test.ts' 'test/js/sql/sql-mysql.auth.test.ts' 'test/js/sql/sql-mysql.helpers.test.ts' 'test/js/sql/sql-mysql.test.ts' 'test/js/sql/sql-mysql.transactions.test.ts' 'test/js/sql/sql.test.ts' 'test/regression/issue/21311.test.ts' 'test/regression/issue/26030.test.ts'
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "test/integration/mysql2/mysql2.test.ts" test/js/sql/sql-mysql.auth.test.ts test/js/sql/sql-mysql.helpers.test.ts test/js/sql/sql-mysql.test.ts test/js/sql/sql-mysql.transactions.test.ts test/js/sql/sql.test.ts "test/regression/issue/21311.test.ts" "test/regression/issue/26030.test.ts"
info: syncing channel updates for nightly-2026-05-06-x86_64-unknown-linux-gnu
info: latest update on 2026-05-06 for version 1.97.0-nightly (e95e73209 2026-05-05)
info: component rust-src is up to date
info: checking for self-update (current version: 1.29.0)
bun test v1.4.0 (2b00a211e)

test/regression/issue/21311.test.ts:
Container ready via docker-compose: postgres_plain at 127.0.0.1:5432
(pass) postgres > should handle large batch inserts without crashing [780.18ms]
(pass) postgres > should handle empty result sets without crashing [629.35ms]
(pass) postgres > should handle mixed date formats in batch operations [1775.22ms]

test/regression/issue/26030.test.ts:
Container ready via docker-compose: mysql_plain at 127.0.0.1:3306
(pass) mysql > Sequential transactions with INSERT and returned SELECT should not hang [262.02ms]
(pass) mysql > Sequential transactions with returned array of multiple queries [187.57ms]
(pass) mysql > Many sequential transactions with awaited INSERT and returned SELECT [252.20ms]

test/js/sql/sql-mysql.auth.test.ts:
failed to connect to the docker API at unix:///var/run/docker.sock; check if the path is correct and if the daemon is running: dial unix /var/run/docker.sock: connect: no such file or directory
(pass) caching_sha2_password fast-auth success: the trailing OK belongs to auth, not the first query (split framing) [389.05ms]
(pass) caching_sha2_password fast-auth success: the trailing OK belongs to auth, not the first query (coalesced framing) [144.23ms]
(pass) caching_sha2_password scramble hashes the double-SHA256 before the nonce [141.59ms]

test/js/sql/sql-my
... (truncated)
Exit: 0
diff hotspot
test/docker/Dockerfile.mysql-native-password | 13 ++++--
 test/docker/Dockerfile.mysql-plain           |  2 +-
 test/docker/Dockerfile.postgres-auth         |  4 +-
 test/docker/Dockerfile.postgres-plain        | 12 +++---
 test/docker/README.md                        |  2 +-
 test/docker/docker-compose.yml               |  6 ++-
 test/harness.ts                              | 32 ++-------------
 test/integration/mysql2/mysql2.test.ts       | 15 ++-----
 test/js/sql/docker-tls/Dockerfile            |  4 +-
 test/js/sql/mysql-tls/Dockerfile             |  2 +-
 test/js/sql/sql-mysql.auth.test.ts           |  2 -
 test/js/sql/sql-mysql.helpers.test.ts        |  2 -
 test/js/sql/sql-mysql.test.ts                | 17 +++-----
 test/js/sql/sql-mysql.transactions.test.ts   | 27 +++++++------
 test/js/sql/sql.test.ts                      | 13 +++---
 test/js/valkey/docker-tls/Dockerfile         | 59 ----------------------------
 test/js/valkey/docker-tls/server.crt         | 33 ----------------
 test/js/valkey/docker-tls/server.key         | 52 ------------------------
 test/js/valkey/docker/Dockerfile             | 44 ---------------------
 test/regression/issue/21311.test.ts          |  2 -
 test/regression/issue/26030.test.ts          |  2 -
 21 files changed, 59 insertions(+), 286 deletions(-)

gate history · 1 passed · 0 rejected · iteration 11

evidence per changed file
file                                          reads  edits  tests
test/docker/Dockerfile.mysql-native-password      1      1      0
test/docker/Dockerfile.mysql-plain                1      1      0
test/docker/Dockerfile.postgres-auth              2      3      0
test/docker/Dockerfile.postgres-plain             2      3      0
test/docker/README.md                             1      1      0
test/docker/docker-compose.yml                    3     11      0
test/harness.ts                                   2      5      0
test/integration/mysql2/mysql2.test.ts            1      2      0
test/js/sql/docker-tls/Dockerfile                 2      3      0
test/js/sql/mysql-tls/Dockerfile                  2      3      0
test/js/sql/sql-mysql.auth.test.ts                1      1      0
test/js/sql/sql-mysql.helpers.test.ts             1      1      0
test/js/sql/sql-mysql.test.ts                     2      2      0
test/js/sql/sql-mysql.transactions.test.ts        2      4      0
test/js/sql/sql.test.ts                           1      2      0
test/js/valkey/docker-tls/Dockerfile              0      0      0
(+ 5 more files)

@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Updates Docker and test harness container configurations, adjusts SQL test assertions, removes Valkey TLS Docker assets, and refreshes documentation formatting.

Changes

Test infrastructure and docs

Layer / File(s) Summary
Docker image and compose updates
test/docker/Dockerfile.postgres-auth, test/docker/Dockerfile.postgres-plain, test/js/sql/docker-tls/Dockerfile, test/docker/docker-compose.yml, test/js/sql/mysql-tls/Dockerfile
Updates PostgreSQL base images to 18, changes PostgreSQL tmpfs mounts, bumps MySQL and Redis compose images, and updates MySQL TLS build defaults.
Container harness and MySQL test updates
test/harness.ts, test/integration/mysql2/mysql2.test.ts, test/js/sql/sql-mysql.test.ts, test/js/sql/sql-mysql.auth.test.ts, test/js/sql/sql-mysql.helpers.test.ts, test/regression/issue/21311.test.ts, test/regression/issue/26030.test.ts
describeWithContainer now uses the image name directly, removes image alias translation and per-container env/args plumbing, and the MySQL integration and SQL tests switch to the updated container image names.
SQL test expectation updates
test/js/sql/sql-mysql.transactions.test.ts, test/js/sql/sql.test.ts
The MySQL transaction tests assert stable error codes and errno values, and the PostgreSQL array tests adjust escape and record-shape expectations.
Remove Valkey TLS docker artifacts
test/js/valkey/docker-tls/Dockerfile, test/js/valkey/docker-tls/server.crt, test/js/valkey/docker-tls/server.key, test/js/valkey/docker/Dockerfile
Removes the Valkey TLS Dockerfile and deletes the TLS certificate and key files, along with the non-TLS Valkey Dockerfile.
Docs formatting updates
docs/guides/util/base64.mdx, docs/runtime/web-apis.mdx, test/docker/README.md
Reformats the base64 warning example, reflows the Web APIs table, and renames the docker-compose override environment variable in the Docker README.

Possibly related PRs

  • oven-sh/bun#31732: Updates the same PostgreSQL test Dockerfiles that are bumped to postgres:18 here.
  • oven-sh/bun#32139: Modifies the same describeWithContainer Docker harness flow and service mapping logic.
  • oven-sh/bun#33040: Changes the same base64 guide example formatting around legacy btoa()/atob() usage.

Suggested reviewers

  • cirospaciari
  • alii
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main change: bumping CI test service containers to newer major versions.
Description check ✅ Passed The description covers both what changed and how it was verified, with detailed compatibility notes and CI validation.

Comment @coderabbitai help to get the list of available commands.

@robobun

robobun commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator Author
Updated 11:06 PM PT - Jul 9th, 2026

@robobun, your commit 2b00a21 has 2 failures in Build #71312 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 33097

That installs a local version of the PR into your bun-33097 executable, so you can run:

bun-33097 --bun

@mintlify

mintlify Bot commented Jun 29, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
bun 🟢 Ready View Preview Jun 29, 2026, 8:08 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

Comment thread test/harness.ts
Comment thread test/docker/Dockerfile.postgres-plain

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM — test-infra-only container bumps with both prior review points addressed and two green CI runs on the final diff.

Extended reasoning...

Overview

This PR bumps the Docker service containers that CI database tests run against to their latest stable majors: PostgreSQL 15→18, MySQL 8.4→9 (and the mysql_native_password service 8.0→8.4), and Redis 7→8. All 19 changed files are under test/ — no runtime or production code is touched. The bumps forced a handful of well-scoped follow-ups: adjusting the postgres tmpfs mount for the new PGDATA layout, replacing MySQL 8.4's removed --default-authentication-plugin with --mysql-native-password=ON + --authentication-policy, fixing two sql.test.ts assertions that encoded PG15-specific facts, hardening two MySQL error assertions to use the stable errno instead of server prose, removing the now-dead env/args/archs options from describeWithContainer and their no-op call sites, deleting the unreferenced test/js/valkey/docker*/ directories, and fixing a README env-var name.

Security risks

None. This touches only test infrastructure — Docker image tags, compose config, test harness plumbing, and test assertions. No auth, crypto, permissions, or user-facing code paths are involved. The deleted TLS cert/key were unused test fixtures.

Level of scrutiny

Low-to-moderate. The blast radius is confined to CI test reliability for the SQL suites, which is highly visible and easily reverted. No src/ code changes. The PR description documents two full CI builds (#66936, #66976) with 283/286 passing and the 3 failures traced to known unrelated flakes. Each non-obvious change (tmpfs path, MySQL 8.4 auth flags, PG18 \v escape, pg_database catalog shape, arch-guard removal) is explicitly justified.

Other factors

I raised two issues on earlier revisions of this PR — the newly-dead env parameter and the PG15-shaped pg_database record literal breaking under PG18 — and both were addressed (3ccb5b2 and b7183fd respectively). The bug-hunting pass on the current diff found nothing. I verified that the deleted valkey directories have no remaining references, that the README's BUN_DOCKER_COMPOSE_FILE fix matches what test/docker/index.ts actually reads, and that no CODEOWNERS entry covers these paths. The MySQL error-assertion changes are strictly stronger than before (code + errno + message substring vs. exact prose).

robobun added 6 commits July 10, 2026 03:41
… 9, redis 8)

PostgreSQL 18 moved PGDATA under /var/lib/postgresql/<major>/docker, so the
tmpfs mounts now target /var/lib/postgresql.

MySQL 8.4 removed --default-authentication-plugin and MySQL 9 removed the
mysql_native_password plugin. mysql_plain and mysql_tls move to mysql:9;
mysql_native_password stays on the 8.4 LTS with --mysql-native-password=ON and
--authentication-policy=mysql_native_password,, so the handshake keeps
advertising native_password (sql-mysql.auth.test.ts depends on that for its
AuthSwitchRequest coverage). mysql:8.0 is no longer maintained upstream.

Remove the mysql:8/mysql:9 aliases in test/harness.ts (they routed on the
caller's env field and were already mislabeled); tests now name the compose
service directly. Delete the unreferenced redis:7 Dockerfiles under
test/js/valkey/docker and test/js/valkey/docker-tls.
…reads

The README said BUN_DOCKER_COMPOSE_PATH, but index.ts reads
BUN_DOCKER_COMPOSE_FILE.
…ontainer

Removing the mysql:8/mysql:9 alias block deleted the only reader of env.
args and archs were never read. Remove all three from the signature and
the five callers that passed no-op env: {} / args: [] objects.
The two transaction error tests pinned the server's exact error message,
the one field that is not stable: it varies across MySQL versions and
differs on MariaDB, which Bun's MySQL adapter also targets (mariadb://
URLs and the BUN_TEST_SERVICE_* override let the suite run against any
MySQL-compatible server). They ignored errno, the MySQL wire-protocol
error number, which is identical across MySQL 8/9 and MariaDB (1366 =
ER_TRUNCATED_WRONG_VALUE_FOR_FIELD, 1054 = ER_BAD_FIELD_ERROR).

Assert code + errno plus the identifying message substring instead.
Strictly stronger than the prose-only assertion, and matches the
existing expect(error.code) idiom in the same describe block.
Two PostgreSQL tests encoded PostgreSQL-15-specific facts and fail on
postgres:18 on every Linux x64 lane:

"pg_database[] - null values" hardcoded a 16-field pg_database record
literal. pg_database gained daticurules in PostgreSQL 16 and
dathasloginevt in 17 (which also renamed daticulocale to datlocale), so
on 18 the eighth field of the literal lands on a boolean column and the
cast fails with 'invalid input syntax for type boolean: "-1"'. Source
the record from the live server instead of pinning the catalog shape.

"text[] - special character escaping" produced its vertical tab via
E'\v', which PostgreSQL did not recognize as an escape until version
18, so the test actually decoded a literal letter v. On 18 E'\v' is a
real U+000B and the assertion broke. Use the version-independent hex
escape E'\x0b' and expect a real vertical tab, which is what the test
name claims to cover.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Both earlier review comments are addressed and I found no further issues, but this is a coordinated multi-service version bump plus harness API cleanup and test-assertion rewrites across 21 files — worth a quick human glance (cirospaciari?) to sign off on the target server versions before merge.

Extended reasoning...

Overview

This PR bumps the CI test service containers to their latest stable majors (postgres 15→18, mysql 8.4→9, mysql_native_password 8.0→8.4, redis 7→8) and carries the fallout: PGDATA relocated outside the new VOLUME /var/lib/postgresql, MySQL 8.4's --mysql-native-password=ON / --authentication-policy replacing the removed --default-authentication-plugin, two PostgreSQL-18-incompatible assertions in sql.test.ts rewritten, two MySQL transaction assertions hardened from server prose to errno, the dead mysql:8/mysql:9 alias layer and its env/args/archs options removed from describeWithContainer, and the unreferenced test/js/valkey/docker*/ directories deleted. Zero lines of src/ change.

Security risks

None. All changes are to test infrastructure. The deleted server.key/server.crt were unreferenced self-signed test fixtures.

Level of scrutiny

Medium. Each individual change is mechanical and well-justified, and the PR description documents multiple CI builds where every database suite passes on the bumped containers. But this changes which server majors the entire SQL test surface runs against going forward, removes an option from a shared harness helper, and rewrites four test assertions — that's a policy/coverage decision a maintainer familiar with the SQL adapters should sign off on rather than a pure config tweak.

Other factors

I previously left two inline comments (dead env param; PG15-shaped pg_database literal breaking under PG18) — both were fully addressed and the threads are resolved. The bug-hunting system found nothing on the current diff. No CODEOWNERS cover the touched paths. Given the 21-file scope and the "which versions do we test against" decision embedded here, deferring to a human is the safer call.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant