Conversation
Make `Cycle\ActiveRecord\TransactionMode` a runtime alias of `Cycle\Transaction\TransactionMode` (with an IDE/Psalm stub under stubs/) and reference the original enum throughout. Delegate the transaction mechanics to the cycle/transaction package: - `ActiveRecord::transact()` now runs through `Cycle\Transaction\Transaction` (FlushMode::OnWrite preserves the "execute right away" contract), keeping the ambient EM holder so save()/delete()/groupActions() still join it; - save()/delete()/saveOrFail()/deleteOrFail() persist a single entity via `transact()` (source resolved from the instance to handle ORM proxies); - groupActions() uses the standard `Cycle\ORM\EntityManager` with the runner passed to run(). This removes the bespoke `Internal\EntityManager` and `Internal\EmptyState`. Add multi-database test coverage: a secondary in-memory SQLite database, a `Post` entity bound to it, and MultiDatabaseTest exercising per-database persistence, independent transactions, rollback isolation and the cross-database guard. Also assert immediate ORM execution inside transact().
Replace the PHPUnit/Mockery/spiral-testing based test suite with Testo. - Split tests into a driver-agnostic Unit suite and a driver-matrix Acceptance suite; entity fixtures moved to tests/Stub. - Add a Testo DatabasePlugin: the ORM schema is compiled once and shared via the suite container, connections are pooled per driver, and each test runs inside a rolled-back transaction for isolation (opt out with the WithoutTransaction attribute). Unreachable drivers are skipped. - Replace container mocks with hand-rolled PSR-11 fakes; the Facade is wired through a scoped Testo container instead of a custom one. - Drop phpunit.xml.dist, the Spiral test app/bootloaders and the legacy Functional/Arch tests; update composer.json, CI workflows, Rector, Psalm and Infection config; add testo.php and tests/docker-compose.yml.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 1.x #47 +/- ##
=============================================
+ Coverage 97.23% 100.00% +2.76%
=============================================
Files 10 6 -4
Lines 181 96 -85
=============================================
- Hits 176 96 -80
+ Misses 5 0 -5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…ionException - Make leaf classes final (Facade, ConfigurationException, the Spiral/Laravel bridges) and mark ActiveQuery/ActiveRepository as @api since they are user extension points. - Add the #[\Override] attributes and purity/immutability annotations Psalm requires at errorLevel 1, and type the repository $orderBy argument. - Deprecate Cycle\ActiveRecord\Exception\Transaction\TransactionException, aliasing it to Cycle\Transaction\Exception\TransactionException (mirroring TransactionMode); the @throws now reference the cycle/transaction exception. - Consolidate the IDE/static-analysis stubs into resources/stubs.php.
There was a problem hiding this comment.
Pull request overview
This PR refactors ActiveRecord’s transaction and grouping implementation to delegate to the external cycle/transaction package, removing the custom internal EntityManager/transaction state and adding multi-database safety checks. It also modernizes the dev/test toolchain by migrating the test suite from PHPUnit to Testo, updating CI workflows accordingly, and bumping the minimum PHP version to 8.2.
Changes:
- Replace in-house transaction runner/EntityManager with
cycle/transaction, including aliases to preserve deprecatedCycle\ActiveRecord\TransactionMode/TransactionExceptionnames. - Rework and expand the test suite under Testo (Unit + multi-driver Acceptance), plus new DB provisioning plugin/interceptor.
- Update package/CI configuration: PHP >= 8.2, refreshed Cycle constraints, Testo-based coverage/mutation testing and new GitHub Actions workflows.
Reviewed changes
Copilot reviewed 73 out of 75 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Unit/Stub/Container/ServiceNotFoundException.php | Adds a concrete PSR-11 not-found exception for test fakes. |
| tests/Unit/Stub/Container/ConfigurableContainer.php | Adds a lightweight PSR-11 container fake used by unit tests. |
| tests/Unit/InheritanceTest.php | Migrates inheritance test from PHPUnit to Testo. |
| tests/Unit/FacadeTest.php | Adds Testo-based unit tests for Facade container/caching behavior. |
| tests/Unit/ActiveRecordBootloaderTest.php | Adds unit tests for Spiral bootloader integration using Testo. |
| tests/Stub/Repository/RepositoryWithActiveQuery.php | Moves test stubs to Cycle\\Tests\\Stub\\... namespaces. |
| tests/Stub/Query/UserQuery.php | Moves query stub to Cycle\\Tests\\Stub\\... namespace. |
| tests/Stub/Entity/User.php | Moves user entity stub to Cycle\\Tests\\Stub\\... namespace. |
| tests/Stub/Entity/Post.php | Adds a secondary-database entity for multi-db transaction tests. |
| tests/Stub/Entity/Identity.php | Moves identity entity stub to Cycle\\Tests\\Stub\\... namespace. |
| tests/src/Functional/Repository/ActiveRepositoryTest.php | Removes legacy PHPUnit functional repository tests. |
| tests/src/Functional/Query/ActiveQueryTest.php | Removes legacy PHPUnit functional query test. |
| tests/src/Functional/Internal/EmptyStateTest.php | Removes tests for deleted internal EmptyState. |
| tests/src/Functional/FacadeTest.php | Removes legacy PHPUnit functional Facade tests. |
| tests/src/Functional/DatabaseTestCase.php | Removes PHPUnit functional DB harness (replaced by Testo plugin). |
| tests/src/Functional/Bridge/Spiral/Bootloader/ActiveRecordBootloaderTest.php | Removes legacy Spiral bootloader functional test. |
| tests/src/Functional/ActiveRecordTest.php | Removes large legacy PHPUnit functional ActiveRecord test suite. |
| tests/src/Arch/DebugTest.php | Removes PHPUnit architecture/debug test. |
| tests/docker-compose.yml | Adds DB services (MySQL/Postgres/SQLServer) for acceptance testing. |
| tests/app/Testing/TestLogger.php | Removes legacy Spiral testing logger. |
| tests/app/Testing/TestCase.php | Removes legacy Spiral testing base TestCase. |
| tests/app/Testing/Loggable.php | Removes legacy logging trait used by old functional tests. |
| tests/app/config/monolog.php | Removes Spiral monolog test config. |
| tests/app/config/database.php | Removes Spiral DB test config. |
| tests/app/config/cycle.php | Removes Spiral Cycle ORM test config. |
| tests/app/Bootloader/SyncTablesBootloader.php | Removes Spiral bootloader previously used for schema sync. |
| tests/app/Bootloader/AppBootloader.php | Removes Spiral application bootloader used by legacy tests. |
| tests/Acceptance/Testo/WithoutTransaction.php | Adds attribute to opt out of automatic per-test transactions. |
| tests/Acceptance/Testo/OrmEnvironment.php | Adds ORM/schema compilation, seeding, and purge helpers for Testo. |
| tests/Acceptance/Testo/EntityClassLocator.php | Adds deterministic entity locator for annotated schema compilation. |
| tests/Acceptance/Testo/DatabasePlugin.php | Adds Testo plugin wiring schema + interceptors into suite container. |
| tests/Acceptance/Testo/DatabaseInterceptor.php | Adds per-case DB provisioning + per-test transaction isolation. |
| tests/Acceptance/Testo/DatabaseDriver.php | Adds driver enum + env-based driver config builder for acceptance tests. |
| tests/Acceptance/Testo/ConnectionPool.php | Adds per-suite DB manager pool and one-time preparation tracking. |
| tests/Acceptance/Driver/SQLServer/ActiveRecordTest.php | Adds SQLServer acceptance test entrypoint (grouped). |
| tests/Acceptance/Driver/SQLite/ActiveRecordTest.php | Adds SQLite acceptance test entrypoint (grouped). |
| tests/Acceptance/Driver/Postgres/ActiveRecordTest.php | Adds Postgres acceptance test entrypoint (grouped). |
| tests/Acceptance/Driver/MySQL/ActiveRecordTest.php | Adds MySQL acceptance test entrypoint (grouped). |
| tests/Acceptance/Common/BaseTestCase.php | Adds shared acceptance helpers that read ORM/DBAL from Facade. |
| tests/Acceptance/Common/ActiveRecordTestCase.php | Adds the main cross-driver acceptance scenario suite in Testo. |
| testo.php | Adds Testo application config defining Unit + Acceptance suites and coverage. |
| src/TransactionMode.php | Replaces enum with runtime alias to Cycle\\Transaction\\TransactionMode. |
| src/Repository/ActiveRepository.php | Marks repository as @api, refines docs, aligns Psalm mutation annotation. |
| src/Query/ActiveQuery.php | Marks ActiveQuery as @api. |
| src/Internal/TransactionFacade.php | Reimplements grouping + transact via cycle/transaction + ORM EntityManager. |
| src/Internal/EntityManager.php | Removes custom internal EntityManager implementation. |
| src/Internal/EmptyState.php | Removes internal EmptyState implementation. |
| src/Facade.php | Makes Facade final and adds Psalm external-mutation-free annotations. |
| src/Exception/Transaction/TransactionException.php | Replaces custom exception with alias to Cycle\\Transaction\\.... |
| src/Exception/ConfigurationException.php | Makes ConfigurationException final. |
| src/Exception/ActiveRecordException.php | Adds Psalm mutability annotation to exception marker interface. |
| src/Bridge/Spiral/Bootloader/ActiveRecordBootloader.php | Makes bootloader final, adds #[Override] and Psalm annotations. |
| src/Bridge/Laravel/Providers/ActiveRecordProvider.php | Makes Laravel provider final, adds #[Override] and Psalm annotations. |
| src/ActiveRecord.php | Routes save/delete + transact/groupActions behavior through TransactionFacade. |
| skills.json | Adds skills configuration (agent tooling metadata). |
| resources/stubs.php | Adds IDE/Psalm stubs for deprecated TransactionMode/TransactionException names. |
| rector.php | Simplifies Rector config and removes PHPUnit-related set/import. |
| README.md | Normalizes headings (removes emoji prefixes). |
| psalm.xml | Removes PHPUnit plugin config and registers stub file. |
| psalm-baseline.xml | Updates baseline after deleting internal EntityManager/throw site. |
| phpunit.xml.dist | Removes PHPUnit configuration (test runner migrated to Testo). |
| infection.json | Updates Infection config for Testo framework + schema and excludes. |
| composer.json | Bumps PHP to >=8.2, adds cycle/transaction, migrates testing deps/scripts to Testo. |
| CLAUDE.md | Adds repo guidance documenting Testo conventions and test architecture. |
| .gitignore | Removes tests/runtime ignore (legacy layout), keeps root runtime ignored. |
| .github/workflows/testing.yml | Updates CI to run Testo unit/no-driver tests and separate coverage job. |
| .github/workflows/testing-sqlserver.yml | Adds SQLServer acceptance workflow using docker compose + Testo. |
| .github/workflows/testing-pgsql.yml | Adds Postgres acceptance workflow using docker compose + Testo. |
| .github/workflows/testing-mysql.yml | Adds MySQL acceptance workflow using docker compose + Testo. |
| .github/workflows/static-analysis.yml | Adjusts composer validation strictness for updated toolchain. |
| .github/workflows/security-analysis.yml | Adjusts composer validation strictness for updated toolchain. |
| .github/workflows/refactoring.yml | Adjusts composer validation strictness for updated toolchain. |
| .github/workflows/infection.yml | Adds a dedicated Infection (mutation testing) workflow. |
| .gitattributes | Expands export-ignore patterns and explicitly keeps composer.json in exports. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+48
to
+51
| public function has(string $id): bool | ||
| { | ||
| return true; | ||
| } |
| @@ -21,6 +21,8 @@ | |||
| * | |||
| * @see self::forUpdate() as an example of immutabile method. | |||
Comment on lines
112
to
114
| * @return Select<TEntity> | ||
| * @mutation-free | ||
| * @psalm-mutation-free | ||
| */ |
Comment on lines
+6
to
+8
| environment: | ||
| SA_PASSWORD: "YourStrong!Passw0rd" | ||
| ACCEPT_EULA: "Y" |
Comment on lines
+19
to
+22
| environment: | ||
| MYSQL_DATABASE: "spiral" | ||
| MYSQL_ROOT_PASSWORD: "YourStrong!Passw0rd" | ||
| MYSQL_ROOT_HOST: "%" |
Comment on lines
+29
to
+32
| environment: | ||
| POSTGRES_DB: "spiral" | ||
| POSTGRES_USER: "postgres" | ||
| POSTGRES_PASSWORD: "YourStrong!Passw0rd" |
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
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.
🔍 What was changed
Transaction handling in ActiveRecord is now powered by the dedicated
cycle/transactionpackage instead of an in-house implementation.ActiveRecord::transact(),groupActions()and the implicit transactions behindsave()/saveOrFail()/delete()/deleteOrFail()all run through it. ORM operations insidetransact()still execute immediately, exactly as before, so existing user code keeps the same behaviour.Cycle\ActiveRecord\TransactionModehas moved tocycle/transactionasCycle\Transaction\TransactionMode. The old name is kept as a runtime alias (backed by an IDE/Psalm stub), so existing code and theTransactionMode::Ignore|Current|OpenNewcases keep working without changes.transact()gained cross-database safety: persisting an entity that belongs to a different database than the one the transaction was opened on now throws aTransactionExceptioninstead of silently writing to the wrong connection.Minimum PHP is now
>= 8.2(was8.1), and the package pulls incycle/transactionalongside refreshedcycle/orm/cycle/databaseconstraints.Note
Cycle\ActiveRecord\TransactionModeis deprecated — please switch toCycle\Transaction\TransactionMode. It still works today through an alias, so no immediate action is required.🤔 Why?
ActiveRecord shipped its own EntityManager, transaction runner and result-state, duplicating logic that
cycle/transactionnow provides in a shared, tested form. Delegating to it removes the duplication, gives multi-database guarding for free, and keeps transaction semantics consistent across the Cycle ecosystem. The customInternal\EntityManager/EmptyStateare removed; grouping reuses the standardCycle\ORM\EntityManager.📝 Checklist
transact())📃 Documentation
The deprecated
TransactionModealias keeps existing docs and examples valid, so no documentation changes are required in this PR.