Skip to content

SharedDatabases: Initiate only a single connection per database#105

Merged
nilmerg merged 1 commit into
mainfrom
fix/shared-db-test-trait
Jul 21, 2026
Merged

SharedDatabases: Initiate only a single connection per database#105
nilmerg merged 1 commit into
mainfrom
fix/shared-db-test-trait

Conversation

@nilmerg

@nilmerg nilmerg commented Jul 20, 2026

Copy link
Copy Markdown
Member

Since support for PHPUnit 10 was added, the data provider is being evaluated per class usage and only once per run, meaning that the setup and tear down routines for the schema are processed multiple times. This on its own isn't a problem, but test fixtures will add up during execution of multiple independent test cases and this wasn't intended when this trait was introduced.

This change introduces a specific singleton to maintain a global state across different test cases in order to have only a single connection per database. It also orchestrates schema handling better since it will be done per test case again now and is even adjustable to support shared state between different test cases. Each transaction can also be put into a database transaction which is rolled back automatically to ensure that no state, that is not part of the schema set-up, influences subsequent tests.

Also drops the following methods that were not marked internal:

  • getEnvironmentVariable
  • getConnectionConfig

@nilmerg nilmerg self-assigned this Jul 20, 2026
@cla-bot cla-bot Bot added the cla/signed label Jul 20, 2026
@nilmerg
nilmerg force-pushed the fix/shared-db-test-trait branch 2 times, most recently from 40bfb11 to ff71b37 Compare July 20, 2026 11:36
Since support for PHPUnit 10 was added, the data provider is being
evaluated per class usage and only once per run, meaning that the
setup and tear down routines for the schema are processed multiple
times. This on its own isn't a problem, but test fixtures will add
up during execution of multiple independent test cases and this
wasn't intended when this trait was introduced.

This change introduces a specific singleton to maintain a global
state across different test cases in order to have only a single
connection per database. It also orchestrates schema handling
better since it will be done per test case again now and is even
adjustable to support shared state between different test cases.
Each transaction can also be put into a database transaction
which is rolled back automatically to ensure that no state, that
is not part of the schema set-up, influences subsequent tests.

Also drops the following methods that were not marked internal:
- getEnvironmentVariable
- getConnectionConfig
@nilmerg
nilmerg force-pushed the fix/shared-db-test-trait branch from ff71b37 to 684152a Compare July 20, 2026 12:49

@nilmerg nilmerg left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude approves this:

I reviewed the diff between main and this branch (commit 684152a) against the stated goal: making SharedDatabases maintain a single connection per database across test classes, instead of a new one per class (which is what caused fixtures to accumulate under PHPUnit 10).

Design checks out. The key fix is moving $connections out of the trait (where PHP gives every consuming class its own copy of a trait's static properties) into SharedDatabases\State, a real singleton class shared process-wide. I traced the two shared-schema scenarios by hand:

  • Two unrelated test classes (no SchemaGroup): maintainSchema() computes a distinct group hash per class name, so each gets its schema dropped/recreated on first use — matches old behavior, but now against one shared connection.
  • Two classes with the same #[SchemaGroup('x')]: they hash to the same group id within a run, so the second class's setUpBeforeClass() sees __test_group.name already matching and skips the drop/recreate, correctly preserving shared fixtures.
  • State::uniqueGroup() salts the hash with uniqid() per process, so leftover __test_group rows from a previous run (stale DB) never accidentally match and skip schema setup.
  • TransactionIsolation: rollback-then-beginTransaction() runs in setUp(), so each test's changes get undone at the start of the next test (or at the start of the next class's maintainSchema(), which unconditionally drains any dangling transaction first). No test ever executes without a fresh transaction on
    top.

I also verified the methods being called that aren't defined directly on Connection (inTransaction(), exec()) are legitimate — Connection::__call() proxies unknown methods straight to the underlying PDO object, and both exist on PDO. ERRMODE_EXCEPTION is set in BaseAdapter, so the try/catch (Throwable)
around the __test_group existence check is well-founded (missing table reliably throws).

No functional bugs found. The dropped getEnvironmentVariable/getConnectionConfig methods are called out explicitly in the commit message as an intentional breaking removal of un-internal-marked API, which lines up with what the diff does.

@nilmerg
nilmerg merged commit 9d8763f into main Jul 21, 2026
13 checks passed
@nilmerg
nilmerg deleted the fix/shared-db-test-trait branch July 21, 2026 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant