diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a6e38be..078eaae 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,6 +2,9 @@ name: CI on: push: + branches: + - 2.next-cake4 + - 3.next-cake5 pull_request: branches: @@ -52,8 +55,11 @@ jobs: - name: Run PHPUnit run: vendor/bin/phpunit - cs-stan: - name: Coding Standard & Static Analysis + - name: Run integration tests + run: composer test-integration + + cs: + name: Coding Standard runs-on: ubuntu-24.04 steps: diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..8ccaf3b --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,17 @@ +Changelog +========= + +Releases for CakePHP 5.0 +------------------------ +* 3.1.0 + * **Breaking (behaviour)** — the per-test transaction now opens before `setUp()` runs, so data written in `setUp()` (fixtures, factories, `save()`) is rolled back with the test instead of being committed. Tests that relied on `setUp()` data carrying over to later tests in the same class (for example, seeding once behind a static flag) must seed per test. + * **Breaking (API)** — `PHPUnitBeforeTestMethodCalledSubscriber` was removed; register `PHPUnitPreparationStartedSubscriber` instead. Applications that registered the old subscriber by hand will get a fatal error after upgrading. + * Note — do not combine this extension with tests that declare `$fixtures`: CakePHP's `TransactionStrategy` requires no active transaction, and the per-test transaction is already open when `setUp()` runs. Use one strategy or the other. + * Fix: the transaction is opened before `setUp()` (`PreparationStarted`), so `setUp()` data no longer leaks into the next test + * Fix: the per-test transaction is rolled back with `rollback(true)` consistently. Prepared tests are closed on `Test\Finished`; tests that never reach it (unmet requirements, or a skip/error during `setUp()`) are closed by the next test's `PreparationStarted` or by `TestRunner\Finished` at the end of the run. In that case a `tearDownAfterClass()` that runs in between is rolled back with the still-open transaction + * New: SQLite integration suite (`composer test-integration`) proving `setUp()` data does not survive between tests + * New public API: `PHPUnitPreparationStartedSubscriber`, `PHPUnitTestFinishedSubscriber`, `PHPUnitTestRunnerFinishedSubscriber` and `TransactionGuard`, plus the `TestConnectionTrait` helper; `PHPUnitAfterTestMethodFinishedSubscriber` is kept for backwards compatibility + * Chore: `TransactionGuard` centralises begin/end, and the unit tests no longer build PHPUnit internals + * CI: PHP 8.1–8.5 matrix, coding standard job, integration suite, and no duplicate runs on PR branches + +Previous releases (3.0.x and earlier) predate this changelog. See the Git history and tags. diff --git a/Docs/Documentation/Overview.md b/Docs/Documentation/Overview.md index 8912cf9..eb68ed3 100644 --- a/Docs/Documentation/Overview.md +++ b/Docs/Documentation/Overview.md @@ -12,11 +12,12 @@ Generic cycle of test execution under DbTest -------------------------------------------- * When test suite starts with DbTest, It loads test database from template database snapshot. -* Before test method starts, DbTest initializes the transaction. -* After test method finishes, DbTest rollbacks transaction. +* Before `setUp()` runs, DbTest opens the transaction. +* After the test finishes, DbTest rolls back the transaction. This way database modifications are quickly restored, but requires that the table uses a transaction engine -like InnoDb. +like InnoDb. Because the transaction is already open when `setUp()` runs, tests using DbTest must not declare +`$fixtures` (CakePHP's fixture transaction strategy cannot run inside an open transaction). Support ------- diff --git a/composer.json b/composer.json index d7bfd5c..1498c20 100644 --- a/composer.json +++ b/composer.json @@ -40,7 +40,8 @@ "phpstan-baseline": "tools/phpstan --generate-baseline", "psalm-baseline": "tools/psalm --set-baseline=psalm-baseline.xml", "stan-setup": "phive install", - "test": "phpunit" + "test": "phpunit", + "test-integration": "phpunit -c phpunit.integration.xml" }, "config": { "allow-plugins": { diff --git a/phpunit.integration.xml b/phpunit.integration.xml new file mode 100644 index 0000000..fb38578 --- /dev/null +++ b/phpunit.integration.xml @@ -0,0 +1,38 @@ + + + + + + + + + + + tests/Integration/ + + + + + + + + + + src/ + + + diff --git a/src/TestSuite/Fixture/Extension/PHPUnitAfterTestMethodFinishedSubscriber.php b/src/TestSuite/Fixture/Extension/PHPUnitAfterTestMethodFinishedSubscriber.php index ac8e2d2..3b4c37d 100644 --- a/src/TestSuite/Fixture/Extension/PHPUnitAfterTestMethodFinishedSubscriber.php +++ b/src/TestSuite/Fixture/Extension/PHPUnitAfterTestMethodFinishedSubscriber.php @@ -12,8 +12,6 @@ */ namespace CakeDC\DbTest\TestSuite\Fixture\Extension; -use Cake\Database\Connection; -use Cake\Datasource\ConnectionManager; use PHPUnit\Event\Test\AfterTestMethodFinished; use PHPUnit\Event\Test\AfterTestMethodFinishedSubscriber; @@ -22,14 +20,21 @@ */ class PHPUnitAfterTestMethodFinishedSubscriber implements AfterTestMethodFinishedSubscriber { + use TestConnectionTrait; + + /** + * @param \CakeDC\DbTest\TestSuite\Fixture\Extension\TransactionGuard $guard Transaction guard. + */ + public function __construct(private readonly TransactionGuard $guard = new TransactionGuard()) + { + } + /** * @param \PHPUnit\Event\Test\AfterTestMethodFinished $event Event * @return void */ public function notify(AfterTestMethodFinished $event): void { - $connection = ConnectionManager::get('test'); - assert($connection instanceof Connection); - $connection->rollback(); + $this->guard->end($this->testConnection()); } } diff --git a/src/TestSuite/Fixture/Extension/PHPUnitExtension.php b/src/TestSuite/Fixture/Extension/PHPUnitExtension.php index 81203ca..bc03bc1 100644 --- a/src/TestSuite/Fixture/Extension/PHPUnitExtension.php +++ b/src/TestSuite/Fixture/Extension/PHPUnitExtension.php @@ -38,7 +38,9 @@ public function bootstrap(Configuration $configuration, Facade $facade, Paramete $facade->registerSubscribers( new PHPUnitStartedSubscriber($fixtureManager), new PHPUnitPreparationStartedSubscriber(), - new PHPUnitAfterTestMethodFinishedSubscriber() + new PHPUnitAfterTestMethodFinishedSubscriber(), + new PHPUnitTestFinishedSubscriber(), + new PHPUnitTestRunnerFinishedSubscriber() ); } } diff --git a/src/TestSuite/Fixture/Extension/PHPUnitPreparationStartedSubscriber.php b/src/TestSuite/Fixture/Extension/PHPUnitPreparationStartedSubscriber.php index fc4a38f..483217b 100644 --- a/src/TestSuite/Fixture/Extension/PHPUnitPreparationStartedSubscriber.php +++ b/src/TestSuite/Fixture/Extension/PHPUnitPreparationStartedSubscriber.php @@ -12,8 +12,6 @@ */ namespace CakeDC\DbTest\TestSuite\Fixture\Extension; -use Cake\Database\Connection; -use Cake\Datasource\ConnectionManager; use PHPUnit\Event\Test\PreparationStarted; use PHPUnit\Event\Test\PreparationStartedSubscriber; @@ -28,18 +26,21 @@ */ class PHPUnitPreparationStartedSubscriber implements PreparationStartedSubscriber { + use TestConnectionTrait; + + /** + * @param \CakeDC\DbTest\TestSuite\Fixture\Extension\TransactionGuard $guard Transaction guard. + */ + public function __construct(private readonly TransactionGuard $guard = new TransactionGuard()) + { + } + /** * @param \PHPUnit\Event\Test\PreparationStarted $event Event * @return void */ public function notify(PreparationStarted $event): void { - $connection = ConnectionManager::get('test'); - assert($connection instanceof Connection); - // a previous test may have left its transaction open (e.g. unmet requirements) - if ($connection->inTransaction()) { - $connection->rollback(true); - } - $connection->begin(); + $this->guard->begin($this->testConnection()); } } diff --git a/src/TestSuite/Fixture/Extension/PHPUnitStartedSubscriber.php b/src/TestSuite/Fixture/Extension/PHPUnitStartedSubscriber.php index a4f2abb..534580f 100644 --- a/src/TestSuite/Fixture/Extension/PHPUnitStartedSubscriber.php +++ b/src/TestSuite/Fixture/Extension/PHPUnitStartedSubscriber.php @@ -24,6 +24,8 @@ */ class PHPUnitStartedSubscriber implements StartedSubscriber { + use TestConnectionTrait; + /** * @var true */ @@ -35,9 +37,11 @@ class PHPUnitStartedSubscriber implements StartedSubscriber /** * @param \CakeDC\DbTest\TestSuite\Fixture\FixtureManager $fixtureManager Fixture Manager instance + * @param \CakeDC\DbTest\TestSuite\Fixture\Extension\TransactionGuard $guard Transaction guard. */ public function __construct( - private readonly FixtureManager $fixtureManager + private readonly FixtureManager $fixtureManager, + private readonly TransactionGuard $guard = new TransactionGuard() ) { } @@ -47,8 +51,7 @@ public function __construct( */ public function notify(Started $event): void { - $connection = ConnectionManager::get('test'); - assert($connection instanceof Connection); + $connection = $this->testConnection(); $database = $connection->config(); try { @@ -58,10 +61,10 @@ public function notify(Started $event): void $this->fixtureManager->setupDatabase($database, true, false); $ds = ConnectionManager::get('test'); } - assert($ds instanceof Connection); + $ds = $this->asConnection($ds); // rollback transaction. gives us a 'fresh' copy of the database for the next // test suite to run - $ds->rollback(); + $this->guard->end($ds); if (!$this->databaseLoaded) { $this->loadDatabase($ds, $database); diff --git a/src/TestSuite/Fixture/Extension/PHPUnitTestFinishedSubscriber.php b/src/TestSuite/Fixture/Extension/PHPUnitTestFinishedSubscriber.php new file mode 100644 index 0000000..a006078 --- /dev/null +++ b/src/TestSuite/Fixture/Extension/PHPUnitTestFinishedSubscriber.php @@ -0,0 +1,43 @@ +guard->end($this->testConnection()); + } +} diff --git a/src/TestSuite/Fixture/Extension/PHPUnitTestRunnerFinishedSubscriber.php b/src/TestSuite/Fixture/Extension/PHPUnitTestRunnerFinishedSubscriber.php new file mode 100644 index 0000000..b1af485 --- /dev/null +++ b/src/TestSuite/Fixture/Extension/PHPUnitTestRunnerFinishedSubscriber.php @@ -0,0 +1,44 @@ +guard->end($this->testConnection()); + } +} diff --git a/src/TestSuite/Fixture/Extension/TestConnectionTrait.php b/src/TestSuite/Fixture/Extension/TestConnectionTrait.php new file mode 100644 index 0000000..4617be5 --- /dev/null +++ b/src/TestSuite/Fixture/Extension/TestConnectionTrait.php @@ -0,0 +1,46 @@ +asConnection(ConnectionManager::get('test')); + } + + /** + * @param mixed $connection Connection to validate. + * @return \Cake\Database\Connection + * @throws \RuntimeException When the connection is not a database connection. + */ + protected function asConnection(mixed $connection): Connection + { + if (!$connection instanceof Connection) { + throw new RuntimeException('The "test" connection must be a Cake\Database\Connection instance.'); + } + + return $connection; + } +} diff --git a/src/TestSuite/Fixture/Extension/TransactionGuard.php b/src/TestSuite/Fixture/Extension/TransactionGuard.php new file mode 100644 index 0000000..38be733 --- /dev/null +++ b/src/TestSuite/Fixture/Extension/TransactionGuard.php @@ -0,0 +1,49 @@ +inTransaction()) { + $connection->rollback(true); + } + + $connection->begin(); + } + + /** + * Rolls back the per-test transaction if it is still open. + * + * @param \Cake\Database\Connection $connection Test connection. + * @return void + */ + public function end(Connection $connection): void + { + if ($connection->inTransaction()) { + $connection->rollback(true); + } + } +} diff --git a/tests/Integration/TransactionIsolationTest.php b/tests/Integration/TransactionIsolationTest.php new file mode 100644 index 0000000..80f9d85 --- /dev/null +++ b/tests/Integration/TransactionIsolationTest.php @@ -0,0 +1,60 @@ +testConnection()->execute('INSERT INTO txn_probe (id) VALUES (NULL)'); + } + + /** + * @return void + */ + public function testFirstTestSeesItsOwnRow(): void + { + $this->assertSame(1, $this->countRows(), 'setUp() data must be visible inside the test'); + } + + /** + * @return void + */ + public function testSecondTestDoesNotSeeThePreviousRow(): void + { + $this->assertSame(1, $this->countRows(), 'setUp() data must not survive the previous test'); + } + + /** + * @return int + */ + protected function countRows(): int + { + return (int)$this->testConnection() + ->execute('SELECT COUNT(*) FROM txn_probe') + ->fetchColumn(0); + } +} diff --git a/tests/TestCase/TestSuite/Fixture/Extension/PHPUnitPreparationStartedSubscriberTest.php b/tests/TestCase/TestSuite/Fixture/Extension/PHPUnitPreparationStartedSubscriberTest.php deleted file mode 100644 index 082c9a5..0000000 --- a/tests/TestCase/TestSuite/Fixture/Extension/PHPUnitPreparationStartedSubscriberTest.php +++ /dev/null @@ -1,167 +0,0 @@ -connection = new class extends Connection { - /** - * @var array - */ - public array $calls = []; - - /** - * @var bool - */ - public bool $inTransaction = false; - - /** - * Skip the parent constructor, the test never touches the driver. - */ - public function __construct() - { - } - - /** - * @return bool - */ - public function inTransaction(): bool - { - return $this->inTransaction; - } - - /** - * @return void - */ - public function begin(): void - { - $this->calls[] = 'begin'; - $this->inTransaction = true; - } - - /** - * @param bool|null $toBeginning Whether the transaction should be rolled back to the beginning. - * @return bool - */ - public function rollback(?bool $toBeginning = null): bool - { - $this->calls[] = 'rollback'; - $this->inTransaction = false; - - return true; - } - }; - - ConnectionManager::drop('test'); - ConnectionManager::setConfig('test', $this->connection); - } - - /** - * The subscriber opens the per-test transaction on the PreparationStarted event. - * - * @return void - */ - public function testOpensTransactionOnPreparationStarted(): void - { - $subscriber = new PHPUnitPreparationStartedSubscriber(); - $this->assertInstanceOf(PreparationStartedSubscriber::class, $subscriber); - - $subscriber->notify($this->preparationStartedEvent()); - - $this->assertSame(['begin'], $this->connection->calls); - } - - /** - * A transaction left open by a previous test is rolled back before beginning. - * - * @return void - */ - public function testRollsBackAbandonedTransactionBeforeBeginning(): void - { - $this->connection->inTransaction = true; - - (new PHPUnitPreparationStartedSubscriber())->notify($this->preparationStartedEvent()); - - $this->assertSame(['rollback', 'begin'], $this->connection->calls); - } - - /** - * Builds a PreparationStarted event; the subscriber ignores the payload. - * - * @return \PHPUnit\Event\Test\PreparationStarted - */ - protected function preparationStartedEvent(): PreparationStarted - { - $snapshot = new Snapshot( - HRTime::fromSecondsAndNanoseconds(0, 0), - MemoryUsage::fromBytes(0), - MemoryUsage::fromBytes(0), - new GarbageCollectorStatus(0, 0, 0, 0, null, null, null, null, null, null, null, null), - ); - $telemetry = new Info( - $snapshot, - Duration::fromSecondsAndNanoseconds(0, 0), - MemoryUsage::fromBytes(0), - Duration::fromSecondsAndNanoseconds(0, 0), - MemoryUsage::fromBytes(0), - ); - $test = new class ('PreparationStarted') extends EventTest { - /** - * @return string - */ - public function id(): string - { - return 'CakeDC\DbTest\Test\TestCase\PreparationStarted::test'; - } - - /** - * @return string - */ - public function name(): string - { - return 'test'; - } - }; - - return new PreparationStarted($telemetry, $test); - } -} diff --git a/tests/TestCase/TestSuite/Fixture/Extension/TransactionGuardTest.php b/tests/TestCase/TestSuite/Fixture/Extension/TransactionGuardTest.php new file mode 100644 index 0000000..3937bef --- /dev/null +++ b/tests/TestCase/TestSuite/Fixture/Extension/TransactionGuardTest.php @@ -0,0 +1,139 @@ +connection = $this->fakeConnection(); + } + + /** + * @return void + */ + public function testBeginOpensTransaction(): void + { + (new TransactionGuard())->begin($this->connection); + + $this->assertSame(['begin'], $this->connection->calls); + $this->assertTrue($this->connection->inTransaction); + } + + /** + * @return void + */ + public function testBeginRollsBackAbandonedTransactionFirst(): void + { + $this->connection->inTransaction = true; + + (new TransactionGuard())->begin($this->connection); + + $this->assertSame(['rollback', 'begin'], $this->connection->calls); + $this->assertTrue($this->connection->inTransaction); + } + + /** + * @return void + */ + public function testEndRollsBackOpenTransaction(): void + { + $this->connection->inTransaction = true; + + (new TransactionGuard())->end($this->connection); + + $this->assertSame(['rollback'], $this->connection->calls); + $this->assertFalse($this->connection->inTransaction); + } + + /** + * @return void + */ + public function testEndDoesNothingWithoutTransaction(): void + { + (new TransactionGuard())->end($this->connection); + + $this->assertSame([], $this->connection->calls); + } + + /** + * @return \Cake\Database\Connection + */ + protected function fakeConnection(): Connection + { + return new class extends Connection { + /** + * @var array + */ + public array $calls = []; + + /** + * @var bool + */ + public bool $inTransaction = false; + + /** + * Skip the parent constructor, the test never touches the driver. + */ + public function __construct() + { + } + + /** + * @return bool + */ + public function inTransaction(): bool + { + return $this->inTransaction; + } + + /** + * @return void + */ + public function begin(): void + { + $this->calls[] = 'begin'; + $this->inTransaction = true; + } + + /** + * @param bool|null $toBeginning Whether the transaction should be rolled back to the beginning. + * @return bool + */ + public function rollback(?bool $toBeginning = null): bool + { + $this->calls[] = 'rollback'; + $this->inTransaction = false; + + return true; + } + }; + } +} diff --git a/tests/TestSuite/Fixture/SqliteExtension.php b/tests/TestSuite/Fixture/SqliteExtension.php new file mode 100644 index 0000000..25d2bd6 --- /dev/null +++ b/tests/TestSuite/Fixture/SqliteExtension.php @@ -0,0 +1,52 @@ +testConnection(); + $connection->execute('DROP TABLE IF EXISTS txn_probe'); + $connection->execute('CREATE TABLE txn_probe (id INTEGER PRIMARY KEY AUTOINCREMENT)'); + + $facade->registerSubscribers( + new PHPUnitPreparationStartedSubscriber(), + new PHPUnitTestFinishedSubscriber(), + new PHPUnitTestRunnerFinishedSubscriber() + ); + } +}