From db15597f42cb74fe9f4a9fedbd11578f67bf6fb5 Mon Sep 17 00:00:00 2001 From: Hideyuki MORI Date: Sat, 23 May 2026 01:09:57 +0900 Subject: [PATCH] test: TransactionManager nested cases + DataModelBase schema-driven behavior (#407) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 評価レポート (#401 § 3 Testing) 指摘の test coverage gap を補強。 ### TransactionManager 追加 3 cases 既存テスト (commit / rollback on throw / outer transaction preserved) に対して、nested run 系の missing scenarios を追加: 1. testNestedRunCommitsOuterTransactionOnlyOnce - 外 run() + 内 run() の両方 commit、内側は no-op に - boundary が 1 つに集約されることを assert 2. testNestedRunInnerExceptionRollsBackOuter - 内 run() 内 throw が outer まで bubble、外で rollback 1 回 + re-throw 3. testRollbackSkipsWhenTransactionAlreadyEnded - callback が pdo->commit() 自己発火 → throw した場合、manager の catch が double-rollback しないこと ### DataModelBase 新規 12 cases eval report が Phan baseline 5 件残存 + テストカバレッジ薄を指摘した DataModelBase の schema-driven magic を pin。fixture model (DataModelBaseTestFixtureModel) で constructor の Log/ErrorCode/RouteContext 依存を bypass し、schema-only 振る舞いに集中: - BOOLEAN / INTEGER / DOUBLE / STRING 各型の cast (4 cases) - 同型値の no-cast (1 case) - unknown property __set throw (1 case) - unknown property __get throw (1 case) - known but unset → null get (1 case) - __isset semantics (1 case) - toArray (1 case) - fromArray (1 case) - getSchema (1 case) ### DataMapperBase scope 外。PDO 依存が大きく、TestDouble の組み立てが今回の trial 範疇を 超えるため future PR で別途。 ### Verification - composer test 128/128 (113 → 128、+15 新) - composer test:http 24/24 (1 expected skip) - composer analyze (Phan) exit 0 - composer format:check exit 0 Closes #407. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/Unit/Xion/DataModelBaseTest.php | 153 +++++++++++++++++++++ tests/Unit/Xion/TransactionManagerTest.php | 69 ++++++++++ 2 files changed, 222 insertions(+) create mode 100644 tests/Unit/Xion/DataModelBaseTest.php diff --git a/tests/Unit/Xion/DataModelBaseTest.php b/tests/Unit/Xion/DataModelBaseTest.php new file mode 100644 index 0000000..71c310d --- /dev/null +++ b/tests/Unit/Xion/DataModelBaseTest.php @@ -0,0 +1,153 @@ +is_completed = '1'; + self::assertTrue($model->is_completed); + $model->is_completed = ''; + self::assertFalse($model->is_completed); + } + + public function testSetCastsIntegerValues(): void + { + $model = new DataModelBaseTestFixtureModel(); + $model->id = '42'; + self::assertSame(42, $model->id); + $model->id = 7.9; + self::assertSame(7, $model->id); + } + + public function testSetCastsDoubleValues(): void + { + $model = new DataModelBaseTestFixtureModel(); + $model->ratio = '0.75'; + self::assertSame(0.75, $model->ratio); + } + + public function testSetCastsStringValues(): void + { + $model = new DataModelBaseTestFixtureModel(); + $model->title = 123; + self::assertSame('123', $model->title); + } + + public function testSetSkipsCastWhenIncomingTypeMatches(): void + { + // Native type already matches the schema; no cast applied. + $model = new DataModelBaseTestFixtureModel(); + $model->title = 'native'; + self::assertSame('native', $model->title); + } + + public function testSetThrowsForUnknownProperty(): void + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('SET unknown IS DISABLE'); + $model = new DataModelBaseTestFixtureModel(); + $model->unknown = 'value'; + } + + public function testGetReturnsNullWhenPropertyDeclaredButUnset(): void + { + $model = new DataModelBaseTestFixtureModel(); + self::assertNull($model->title); + } + + public function testGetThrowsForUnknownProperty(): void + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('GET unknown IS DISABLE'); + $model = new DataModelBaseTestFixtureModel(); + $unused = $model->unknown; + } + + public function testIssetMatchesSetState(): void + { + $model = new DataModelBaseTestFixtureModel(); + self::assertFalse(isset($model->title)); + $model->title = 'set'; + self::assertTrue(isset($model->title)); + } + + public function testToArrayReturnsCurrentData(): void + { + $model = new DataModelBaseTestFixtureModel(); + $model->id = '7'; + $model->title = 'hello'; + $model->is_completed = '1'; + self::assertSame( + ['id' => 7, 'title' => 'hello', 'is_completed' => true], + $model->toArray() + ); + } + + public function testFromArrayPopulatesData(): void + { + $model = new DataModelBaseTestFixtureModel(); + $model->fromArray([ + 'id' => '5', + 'title' => 'imported', + 'is_completed' => 1, + ]); + self::assertSame(5, $model->id); + self::assertSame('imported', $model->title); + self::assertTrue($model->is_completed); + } + + public function testGetSchemaReturnsConfiguredSchema(): void + { + $model = new DataModelBaseTestFixtureModel(); + self::assertSame(DataModelBaseTestFixtureModel::expectedSchema(), $model->getSchema()); + } +} + +/** + * Schema-only fixture model. Bypasses `DataModelBase::__construct` so + * the test does not need a working `Log` / `ErrorCode` / `RouteContext` + * — those are wired by `Initialize::init()` in the real request boot. + */ +final class DataModelBaseTestFixtureModel extends DataModelBase +{ + /** @var array */ + protected static array $schema = [ + 'id' => 'integer', + 'title' => 'string', + 'is_completed' => 'boolean', + 'ratio' => 'double', + ]; + + public function __construct() + { + // Skip the parent boot intentionally — the fixture only exercises + // schema-driven get/set behavior, not the framework's logging + // or error-code wiring. + } + + /** + * @return array + */ + public static function expectedSchema(): array + { + return self::$schema; + } +} diff --git a/tests/Unit/Xion/TransactionManagerTest.php b/tests/Unit/Xion/TransactionManagerTest.php index 870774e..bb608b7 100644 --- a/tests/Unit/Xion/TransactionManagerTest.php +++ b/tests/Unit/Xion/TransactionManagerTest.php @@ -71,6 +71,75 @@ public function testRunDoesNotCommitOuterTransaction(): void self::assertSame([], $this->pdo->committedWrites()); self::assertFalse($this->pdo->inTransaction()); } + + public function testNestedRunCommitsOuterTransactionOnlyOnce(): void + { + // Outer run() opens a transaction; inner run() detects the + // existing transaction and only forwards the callback. Outer's + // commit is the single boundary. + $manager = new TransactionManager($this->pdo); + + $result = $manager->run(function () use ($manager): string { + $this->pdo->recordWrite('outer-step'); + $inner = $manager->run(function (): string { + $this->pdo->recordWrite('inner-step'); + return 'inner-result'; + }); + return $inner . '+committed'; + }); + + self::assertSame('inner-result+committed', $result); + self::assertSame(['outer-step', 'inner-step'], $this->pdo->committedWrites()); + self::assertSame(1, $this->pdo->beginCount); + self::assertSame(1, $this->pdo->commitCount); + self::assertSame(0, $this->pdo->rollbackCount); + } + + public function testNestedRunInnerExceptionRollsBackOuter(): void + { + // When the inner run() throws, the exception propagates up to + // the outer run() which is the one with the actual transaction + // boundary. The outer catch triggers rollback exactly once and + // re-throws. Inner does not double-handle the boundary. + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('inner failure'); + + $manager = new TransactionManager($this->pdo); + try { + $manager->run(function () use ($manager): void { + $this->pdo->recordWrite('outer-step'); + $manager->run(function (): void { + $this->pdo->recordWrite('inner-step'); + throw new RuntimeException('inner failure'); + }); + }); + } finally { + self::assertSame([], $this->pdo->committedWrites()); + self::assertSame(1, $this->pdo->beginCount); + self::assertSame(0, $this->pdo->commitCount); + self::assertSame(1, $this->pdo->rollbackCount); + self::assertFalse($this->pdo->inTransaction()); + } + } + + public function testRollbackSkipsWhenTransactionAlreadyEnded(): void + { + // If the callback itself commits or rolls back the PDO + // transaction (e.g. an inner library does so), the manager's + // catch must not double-rollback when the callback then throws. + $this->expectException(RuntimeException::class); + + try { + (new TransactionManager($this->pdo))->run(function (): void { + $this->pdo->commit(); // ends transaction prematurely + throw new RuntimeException('post-commit failure'); + }); + } finally { + self::assertSame(1, $this->pdo->beginCount); + self::assertSame(1, $this->pdo->commitCount); + self::assertSame(0, $this->pdo->rollbackCount); // no double rollback + } + } } final class TransactionRecordingPdo extends PDO