Make the test suite pass on MySQL - #307
Conversation
social_accounts.workspace_id carries a foreign key, and the composite unique index is the only one covering it, as its leftmost prefix. MySQL refuses to drop the sole index backing a foreign key (SQLSTATE[HY000] 1553), so both rehearsal suites failed in beforeEach and never ran a single assertion on MySQL. Add a plain index on workspace_id first; PostgreSQL has no such requirement and simply carries it. This unmasks one assertion underneath that had never executed: the automation graph comparison at DuplicateIdentityMigrationTest.php:419 depended on JSON object key order, which MySQL normalises on storage. (cherry picked from commit 98a494bd2205e873321a18232f63b358ae259fdf)
MySQL normalises JSON object keys (length, then lexicographic) on storage, so an identity comparison against a literal asserts how the driver chose to lay the object out rather than what it contains. PostgreSQL preserves insertion order, which is why these passed there. toEqual compares associative arrays recursively without regard to key order. Applied to every assertion in this class, including the few that pass today only because their keys already happen to match MySQL's ordering. (cherry picked from commit 3124023c548d6c2b8b52126afc6fc5f38d461ea6)
Four DB::listen predicates matched 'select * from "post_platforms"'. PostgreSQL quotes identifiers with double quotes and MySQL with backticks, so on MySQL the predicates never matched, the simulated mid-run pause never fired, and the race these tests exist to cover went unexercised while the tests still reported failures elsewhere. Compare against the unquoted form via a small helper. (cherry picked from commit 67a81df5de155e80227df748b34cd8b3cfd744f9)
Three assertions read oauth_refresh_tokens.revoked through the query builder rather than Eloquent, so no cast applies and the driver's native representation leaks into the test: a real boolean on PostgreSQL, 1 on MySQL. Cast explicitly at the call site. (cherry picked from commit 2911c5c48cf65d24a34a41e667335c40005839a7)
MySQL TIMESTAMP columns end at 2038-01-19, so the 2099 sentinel these tests used is rejected outright with SQLSTATE[22007]. 2037-12-31 still reads as a far-future schedule and works on both engines. (cherry picked from commit bde33eb239cdbd3a5567d4c21e1d85302913cdd7)
The suite rebuilt a pre-migration schema by dropping the unique index in beforeEach and re-running the migration by hand, exercising a database state the application never runs in.
The migration's down() dropped a unique whose leftmost prefix is an FK column, which MySQL refuses when nothing else backs the constraint (SQLSTATE 1553). It now creates a standalone index first, so migrate:rollback works on MySQL and stays a no-op change for PostgreSQL. up() is untouched: every database already migrated keeps its schema. The rehearsal test calls that down() instead of hand-rolling the drop, so it exercises the real rollback rather than an imitation of it. Matches logged SQL through the connection's query grammar rather than stripping quote characters, and adds a MySQL leg to the backend CI job.
mysql:8.4 installs mysql-community-server-minimal, which ships neither mysqladmin nor the mysql client, so a mysqladmin health command never succeeds and the service never reports healthy. Both images run their init phase without networking, so an open port is the point either engine starts accepting connections - one check covers both, and the per-engine matrix key goes away.
pg_isready and mysqladmin ping are what the respective images ship for this, and the mysql image's entrypoint invokes mysqladmin itself, so it is present. Keeps 20 retries, which MySQL needs to finish initialising.
The 2038 TIMESTAMP limit binds anything written to the column, not just the sentinel dates in fixtures, and the same reasoning generalises: what the app supports is the intersection of both engines.
The published image installed only pdo_pgsql, so DB_CONNECTION=mysql failed with "could not find driver" before any query ran - the app supports MySQL but the image people actually deploy could not reach it. mysql-client mirrors the postgresql-client already present, for artisan db and dumps.
|
Thanks for this — really solid work, and the writeup made it easy to follow. The part that mattered most was catching that both I pushed a few things on top. Summarising so nothing is a surprise: The 1553 was not a test problem. The migration's own
Identifier quoting. Went with asking the connection instead of stripping quote characters: str_starts_with($sql, 'select * from '.DB::getQueryGrammar()->wrapTable($table))Identical behaviour to the original on PostgreSQL, correct on any driver, and it keeps the closing quote as an anchor — without it the prefix would also match a table merely starting with that name. Removed CI now runs both engines. The backend job is a matrix over PostgreSQL 16 and MySQL 8.4, One thing your PR surfaced indirectly. The published Docker image installs only Also documented the engine differences you found in Full suite is green on both, same test and assertion counts: 3927 passed, 14652 assertions. On your sequencing question — landing the fixes complete was the right call, agreed. And thanks for checking #283 for conflicts while you were in there. |
Matrixing the job split its check in two, so the "backend" context the branch protection requires was never reported and every PR sat waiting on it. The matrix is now "tests" and a small "backend" job gates on it, which keeps the required check stable however many engines the matrix grows to - and leaves the open PRs mergeable without a rebase.
2 participants
Follow-up to #302, which made search work on MySQL but left the suite unable to prove it. This makes the test suite pass on MySQL, so the assertions added there become load-bearing rather than decorative.
32 failures → 0. No application changes — every edit is in
tests/.The five classes
Commits are ordered largest first and are independently reviewable.
18 —
SQLSTATE[HY000] 1553, index backing a foreign key.social_accounts.workspace_idhas a foreign key, and the composite unique index is the only thing covering it, as its leftmost prefix. MySQL refuses to drop the sole index backing a foreign key, so bothDuplicateIdentity*Testfiles failed inbeforeEach— meaning no assertion in either suite had ever executed on MySQL. Giving the constraint another index to rest on first fixes it; PostgreSQL has no such requirement and simply carries the extra index.That also unmasked one assertion underneath which had never run: the automation graph comparison now at
DuplicateIdentityMigrationTest.php:419depended on JSON object key order. It is fixed in the same commit, which is why that commit shows 19 rather than 18.5 — JSON object key order.
MySQL normalises JSON object keys on storage (length, then lexicographic); PostgreSQL preserves insertion order.
toBeisassertSame, so it was asserting how the driver chose to lay the object out rather than what it contains.toEqualcompares associative arrays recursively without regard to key order.Applied to every assertion in this class, including two that pass today only because their keys already happen to match MySQL's ordering — one added key and they would have broken.
4 — identifier quoting in
DB::listenpredicates.VerifyUpcomingPostConnectionsTestmatchedselect * from "post_platforms". MySQL emits backticks, so those predicates never matched, the simulated mid-run pause never fired, and the race those tests exist to cover went unexercised. They now compare against the unquoted form.3 — raw boolean reads.
Not a casting gap:
AccessTokencastsrevokedcorrectly. These three assertions readoauth_refresh_tokensthrough the query builder, where no cast applies, so the driver's native representation leaks through — a real boolean onPostgreSQL,
1on MySQL. Cast explicitly at the call site.2 —
TIMESTAMPrange.A
2099-12-31sentinel against a column that tops out at 2038-01-19.2037-12-31still reads as a far-future schedule and works on both engines.Verification
Full suite on both engines, on this branch:
Same test count and same assertion count on both, so this fixes MySQL without altering PostgreSQL behaviour anywhere.
On sequencing
I offered
continue-on-errorfirst and the fixes after, on the assumption they would take a while. They didn't, so this lands the fixes complete and a MySQL CI leg can follow as a small second PR that is green and required from day one —no red job on
mainat any point, and no follow-up flip. Happy to switch back if you'd rather have the leg first.One measurement for that decision: the MySQL suite takes roughly 600s against 280s for PostgreSQL. The 18 rehearsal tests account for about 365s of it, versus 4.5s on PostgreSQL — not caused by these fixes, that is simply what they cost once they actually run. Likely the DDL in
beforeEachforcing implicit commits and defeatingRefreshDatabase's rollback, though I haven't confirmed it and it felt out of scope here.One note on #283
While checking for conflicts I noticed #283 adds a number of
toBe([...])assertions, seven of which read back through the database. I checked them against MySQL and they are fine — they assert on JSON arrays, and MySQL preserves arrayelement order; only object keys get reordered. No action needed, just flagging that I looked, since the two PRs touch the same file.