PHPUnit 12.5.0+ is changing the way that mock objects work:
No expectations were configured for the mock object for \Drupal\comment\CommentManagerInterface. You should refactor your test code and use a test stub instead.$this->any() is deprecated. It was equivalent to having no expectation, in which case the object should be a stub (see the previous bullet) or a more exact number of expectations should be added.with() is deprecated for stubs because it was deemed to be equally pointless.The UnitTestCase base class is responsible for a huge number of notices throughout Core's tests, 556 notices! The mock objects created by getStringTranslationStub() are probably the main offender. But there are other mock objects that never have expectations set in there.
Because of the far-reaching implications of changing one of the base test classes I've chosen to only focus on UnitTestCase and any modifications to child classes that have to be made to support conversions. Luckly, that seems to be a very small number of classes!
--display-phpunit-notices --display-phpunit-deprecations.If you do a comparison of before and after test runs, for example using the command phpunit --display-phpunit-notices --display-phpunit-deprecations core/modules/*/tests/src/Unit core/tests/Drupal/Tests/, then you should ONLY see the number of PHPUnit notices go down. My results were:
Before:
Tests: 17671, Assertions: 89409, Errors: 22, Failures: 14, PHPUnit Deprecations: 13, PHPUnit Notices: 4099, Skipped: 5.
After:
Tests: 17671, Assertions: 89416, Errors: 22, Failures: 14, PHPUnit Deprecations: 13, PHPUnit Notices: 3543, Skipped: 5.
Fix notices caused by the UnitTestCase base class.
Information on how to review these issues and the types of changes you're likely to find in them is included in the parent issue's comments.
Show commandsStart 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:
changes, plain diff MR !15080
Comments
Comment #3
dcam commentedI'm pretty sure that test failure is random.
Anyway, I can't believe that changing all of those config and string translation mocks doesn't cause dozens of Unit tests to blow up, but apparently it's safe to do.
Comment #5
smustgrave commentedLanded here from #3569420: Convert expectation-less test mocks to stubs - Field modules with the MR used from that MR I still got a few notices and @dcam pointed out they would be fixed by this issue. Applying this addressed all the notices and the change here focusing on core/tests/Drupal/Tests/UnitTestCase.php makes sense. Talked to dcam on slack briefly too and he explained why the views change was needed (some reason the comment wasn't appearing on first page load).
Comment #6
catchDo we want a change record like 'Unit tests may need to call ::getContainerWithCacheTagsInvalidator() in test methods instead of the constructor' or something? Although I would imagine at most one or two contrib unit tests will fail given the small number of changes in core. Otherwise looks good to me.
Comment #7
catchComment #8
dcam commentedI added the CR, https://www.drupal.org/node/3580268.
Comment #9
smustgrave commentedCR is well detailed.
Comment #11
catchYes CR is great. Committed/pushed to main, thanks!