Problem/Motivation
phpunit/phpunit released 10.5.30. (https://github.com/sebastianbergmann/phpunit/releases/tag/10.5.30)
In this release, there's a new check if willReturnOnConsecutiveCalls() has the minimum required amount of return values defined, if a not the new \PHPUnit\Framework\MockObject\NoMoreReturnValuesConfiguredException is thrown. (https://github.com/sebastianbergmann/phpunit/commit/490879817a1417fd5fa1...)
In core this happens on one on occasion:
Drupal\Tests\Core\Asset\AssetResolverTest::testGetJsAssets with data set "different libraries, same timestamps"
PHPUnit\Framework\MockObject\NoMoreReturnValuesConfiguredException: Only 6 return values have been configured for Drupal\Core\Asset\LibraryDiscovery::getLibraryByName()
core/lib/Drupal/Core/Asset/AssetResolver.php:264
core/tests/Drupal/Tests/Core/Asset/AssetResolverTest.php:230
Steps to reproduce
See first test failure on https://git.drupalcode.org/issue/drupal-3468781/-/pipelines/257385/test_...
(Other failures are being handled in #3468502: sebastianbergmann/comparator:5.0.2 Introduces (for Drupal) breaking changes)
Proposed resolution
- Fix the sole occurrence of NoMoreReturnValuesConfiguredException in AssetResolverTest
- Bump version of phpunit/phpunit to latest in composer.* in a new issue.
Remaining tasks
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
Issue fork drupal-3468781
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3468781-phpunitphpunit10.5.30-introduces-for
changes, plain diff MR !9242
Comments
Comment #2
spokjeUnsure if this is a bug or a task. Bug because now
is passing without enough return-values in it's Mock, so the test is untrustworthy. Task because that's what we usually do for breaking changes in a dependency update.
Went with task for now.
Comment #4
spokjeComment #5
spokjeBy the power of
almost Sherlock Holmes-like deduction, blunt trail-and-error it seems that$this->libraryDiscovery->expects($this->any())->method('getLibraryByName')->willReturnOnConsecutiveCalls()inDrupal\Tests\Core\Asset\AssetResolverTest::testGetJsAssetsneeds to return twelve instead of the current six values.I'll leave it to bigger brains what those values should be.
Comment #6
spokjeComment #8
catchComment #9
spokjeThanks @catch.
The test-failure I've opened this issue for is now indeed gone.
However, the version bump in
phpunit/phpunitalso causes a version bump insebastianbergmann/comparator, which causes test-failures.Those are being handled in[#3468502].
So I think the right thing to do now is postponing this issue on that one landing.
Comment #10
mondrakeCan’t the AssetResolverTest fix be done now, independently from the dependency bump? That sounds like a bug that needs fixing anyway
Comment #11
spokjeAbsolutely, makes sense to me.
However I would like to bump the version later on (not necessarily in this issue), because, at least to me, this issue shows the new version adds test coverage of our tests that is/was at least needed in one case.
Technically we don't need to bump the version, since the test in this issue in its new incarnation will pass in the current PHPUnit version as well.
But I think we want this extra test coverage for our tests, so we need a bump.
Comment #12
mondrakeSure. In any case IMHO it’s good practice to keep these tools updated. We will need this for working on PHPUnit bump to 11 anyway.
Comment #13
spokjeThanks @mondrake, per usual we agree :)
Comment #14
spokjeUpdated title and IS to show we want to bump the version of PHPUnit in another issue.
Needs work for removing the version bump in this issue
Comment #15
catchRebased so the MR only includes the fix to the tests and also re-titling for the specific bug fix here.
Comment #16
spokjeThanks @catch, code changes without the version bump, so RTBC.
Tagging for followup about version bumping of PHPUnit.
Comment #21
longwaveCommitted and pushed 239e6f638d to 11.x and 8d9b67396d to 11.0.x and 2f5393f842 to 10.4.x and fd1c79fa51 to 10.3.x. Thanks!
Looks like PHPUnit will be bumped in #3468502: sebastianbergmann/comparator:5.0.2 Introduces (for Drupal) breaking changes, removing tag.