Problem/Motivation
PHPUnit 12.5.0+ is changing the way that mock objects work:
- Mock objects that do not configure expectations should be converted to stubs. Otherwise, they throw a notice:
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.
Steps to reproduce
- Require PHPUnit 12 with Composer.
- Fix compatibility issues with PHPUnit 12 by applying this patch.
- Run the Unit tests for a module with
--display-phpunit-notices --display-phpunit-deprecations. - Observe the PHPUnit notices that occur.
Proposed resolution
Fix notices in the migrate and migrate_drupal modules. The use of any() in MigrateSourceTest::testCount() and MigrateSourceTest::testCountCacheKey() are not in the scope of this issue. Those instances represent bad tests that will be handled by #3572915: Invalid tests in MigrateSourceTest.
Remaining tasks
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.
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
Issue fork drupal-3569422
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:
- 3569422-stub-conversion
changes, plain diff MR !14483
Comments
Comment #3
dcam commentedComment #4
dcam commentedComment #5
dcam commentedNeeds work due to PHPUnit 12.5.11 and a new deprecation.
Comment #6
dcam commentedNew development: stubs cannot set
with(). The IS and MR have been updated.The use of
any()inMigrateSourceTest::testCount()andMigrateSourceTest::testCountCacheKey()are removed from the scope of this issue. Those instances represent bad tests that will be handled by #3572915: Invalid tests in MigrateSourceTest.Comment #7
dcam commentedPostponing on #3572915: Invalid tests in MigrateSourceTest.
Comment #8
dcam commentedThis is unblocked. NW to figure out what the next step is.
Comment #9
dcam commentedNo further edits were needed beyond a rebase and updating the baseline, but I decided to merge in the changes from #3569415: Convert expectation-less test mocks to stubs - MigrateProcessTestCase since it became smaller in scope with the deletion of migrate_drupal. That issue has been closed as a duplicate.
Comment #10
dcam commentedComment #11
smustgrave commentedTests: 417, Assertions: 1105, PHPUnit Notices: 316.
Ran with the MR and still got about 19 notices with messages to consider running as stubs.
Comment #12
dcam commented@smustgrave These additional notices are a result of mocks created by
UnitTestCase::getStringTranslationStub(), which contrary to the name returns mock objects, not stubs. These notices will be fixed by #3578904: Convert expectation-less test mocks to stubs - UnitTestCase as they are among the over 500 notices caused by that class and warrant special attention.Comment #13
smustgrave commentedApplied the MR from the other issue and did address all.
Comment #15
catchcore/.phpstan-baseline.php had permissions changed in the MR but fixed that locally. I wonder if we can add a lint step to MR pipelines to catch that.
Committed/pushed to main and 11.x, thanks!
Comment #19
catchComment #22
catchReverted from 11.x after a similar backport caused failures running against phpunit 10. We can just commit these to main.