Upgrade PHPUnit 4.8 to PHPUnit 9 - #563
Open
gregcorbett wants to merge 11 commits into
Open
gregcorbett wants to merge 11 commits into
gregcorbett wants to merge 11 commits into
Conversation
The suite was pinned to phpunit 4.8.* alongside phpunit/dbunit >=1.2.
dbunit is abandoned and caps phpunit at ^7.0, and every phpunit release
in that range is covered by PKSA-z3gr-8qht-p93v, so composer could no
longer resolve the dev requirements at all and composer.lock could not
be regenerated. Nothing below phpunit 9 gets us out of that, so dbunit
has to go.
- phpunit/phpunit 4.8.* -> ^9.6.33, the lowest 9.x with no
outstanding advisory. 9 is also the ceiling: phpunit 10 needs
PHP 8.1 and CI runs 7.4.
- phpunit/dbunit dropped
- doctrine/data-fixtures ^1.5 added, for ORMPurger. Resolves to
1.5.3; 1.6 and later conflict with doctrine/orm <2.12, and we pin
2.9.*.
- phpunit/phpunit-dom-assertions ^2.6 added, for the assertSelect*
assertions that phpunit itself dropped in 5.0
The test suite is ported to the new API in the commit that follows.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Replaces everything the suite used from phpunit/dbunit with either a
native PHPUnit 9 equivalent or a plain PDO call, and brings the rest of
the tests up to the PHPUnit 9 API.
Database tests:
- DBUnit TestCase -> PHPUnit\Framework\TestCase; getDataSet(),
getSetUpOperation() and getTearDownOperation() removed
- getConnection() now returns the plain PDO handle from
bootstrap_pdo.php rather than wrapping it in a DBUnit connection
- per-test table clearing now goes through data-fixtures' ORMPurger,
which derives deletion order (including m2m join tables) from the
Doctrine mapping metadata, so it no longer has to be maintained by
hand
- createQueryTable()/getRowCount()/getRow() replaced with
PDO::query()->fetchAll(). rowCount() is not reliable for SELECT on
sqlite, and most call sites asked for the count twice anyway.
- truncateDataTables.xml is kept, but now only as the table-name list
that assertPreConditions() checks for emptiness. ORMPurger does the
clearing, so the ordering in it no longer matters; the header
comment says so.
- dead PHPUnit\DbUnit\Operation\Factory constructors removed. These
also broke TestCase::__construct($name), which PHPUnit calls with
the test method name.
API removals elsewhere:
- fixture methods given the : void return types PHPUnit 8 requires,
and onNotSuccessfulTest() the \Throwable parameter type
- @ExpectedException -> expectException()
- assertInternalType -> assertIsArray
- assertTag -> assertSelectCount/assertSelectEquals from
phpunit/phpunit-dom-assertions, via DOMTestTrait
phpunit.xml moves to the 9.6 schema: filter/whitelist becomes
coverage/include and cacheTokens is gone.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The PHPUnit 9 port rewrote 59 class and method definitions with the opening brace on the same line as the declaration. PSR-12, which this repo enforces via phpcs.xml and CodeClimate, requires the brace on its own line for both class and method definitions. Scoped to definitions introduced by 5b68d57 and 0fb7249 only; the pre-existing same-line braces elsewhere under tests/ are left alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Applies only to lines touched by 5b68d57, 0fb7249 and 2149488, so the pre-existing long lines in these files are left alone. - 33 $conn->query("SQL")->fetchAll() calls are broken on the query() brackets, with the SQL split once at " WHERE " where it still would not fit. The concatenation reassembles the original string exactly. - 28 ORMPurger calls were long only because of the fully qualified name, so each file gains a "use Doctrine\Common\DataFixtures\Purger\ORMPurger;" import and the call shortens to (new ORMPurger($this->em))->purge(). - The remaining 9 are hand wrapped: an expectException() argument, two trailing comments moved above their assertTrue(), two assertSelect calls split on their brackets, and four comment paragraphs re-flowed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Puts query( and )->fetchAll(); on their own lines and gives each line of SQL its own quoted string, joined with a trailing "." operator. The two joins in DoctrineCleanInsert1Test relied on newlines embedded in a single string literal, which left the continuation lines hanging at the margin. The six WHERE clauses in ExtensionsTest were split in the previous commit with a leading "." instead; both now read the same way. Each statement produces the same SQL as before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Moving these braces onto their own line in 2149488 left the blank that used to separate the class declaration from its first member sitting directly under the brace, which phpcs rejects. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The minimum configured variable name length is 3, so $e was rejected. Renaming the parameter of an override is safe here: the method is only ever called by PHPUnit positionally. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The port replaced the global PHPUnit_Framework_TestSuite with the namespaced class but kept referring to it by its fully qualified name, which phpcs flags as a missing import. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
rowan04
reviewed
Sep 17, 2026
rowan04
left a comment
rowan04
left a comment
Contributor
There was a problem hiding this comment.
I only reviewed: resourcestests, unit/lib/Gocdb_Services, writeAPI, DoctrineTestSuite1.php, phpunit.xml, README.md, and composer.json. looks good overall though, just a few comments to possibly be removed
Co-authored-by: rowan04 <rowanmoss04@gmail.com>
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
2 participants
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.
The suite was pinned to phpunit 4.8.* alongside phpunit/dbunit >=1.2. dbunit is abandoned and caps phpunit at ^7.0, and every phpunit release in that range is covered by PKSA-z3gr-8qht-p93v, so composer could no longer resolve the dev requirements at all and composer.lock could not be regenerated. Nothing below phpunit 9 gets us out of that, so dbunit has to go.
Replaces everything the suite used from phpunit/dbunit with either a native PHPUnit 9 equivalent or a plain PDO call, and brings the rest of the tests up to the PHPUnit 9 API.