Stop the test suite from destroying a real database - #20
Merged
Merged
Conversation
Running `php artisan test` inside the deployed container DROPPED EVERY TABLE in the live lab database and reported 64 passing tests. Proven on the VM: 4 seeded rows, run the suite, 0 rows. Cause: most tests use RefreshDatabase, which drops and rebuilds every table on whatever connection it is handed. phpunit.xml declared DB_CONNECTION=sqlite, but Laravel resolves the REAL process environment first, and docker-compose exports DB_CONNECTION=mysql for the application. So the suite pointed at MySQL. Adding force="true" in phpunit.xml is not sufficient -- verified that an exported variable still wins. So the real fix is an interlock in Tests\TestCase that runs BEFORE parent::setUp(), and therefore before RefreshDatabase can touch anything. It fails closed: the run aborts with an explanation and a reseed command instead of trusting the environment. Also adds TestIsolationTest as a second line of defence, asserting the connection really is in-memory SQLite, and forces the DB_ variables in phpunit.xml anyway for the environments where that does work. The failure shape is what made this worth fixing properly rather than just remembering not to do it: destructive AND silent. Nothing in the output suggested the suite had done anything other than pass. Suite is now 66 tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Running
php artisan testinside the deployed container dropped every table in the live lab database and reported 64 passing tests.Proven on the VM:
Cause
Most tests use
RefreshDatabase, which drops and rebuilds every table on whatever connection it is handed.phpunit.xmldeclaredDB_CONNECTION=sqlite— but Laravel resolves the real process environment first, and docker-compose exportsDB_CONNECTION=mysqlfor the application. So the suite pointed at MySQL and did exactly what it was designed to do.Why
force="true"isn't the fixI tried it first. Verified that an exported variable still wins over PHPUnit's forced
<env>, because Laravel reads$_SERVER/getenvahead of it. It's included anyway for environments where it does work, but it can't be relied on.The actual fix
An interlock in
Tests\TestCase::setUp()that runs beforeparent::setUp(), and therefore beforeRefreshDatabasecan touch anything. It fails closed — the run aborts with an explanation and a reseed command rather than trusting the environment:Plus
TestIsolationTestas a second line of defence, asserting the connection really is in-memory SQLite.Why this was worth fixing properly
The failure shape, not the bug itself. It was destructive and silent — nothing in the output suggested the suite had done anything other than pass. A note-to-self not to run tests in the container would have held right up until the next person did it.
Third bug found by deploying, after #19's two. All three shared a cause: they needed a real container to surface, and a dev machine structurally cannot reproduce them.
Suite is now 66 tests.
🤖 Generated with Claude Code