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

  1. Require PHPUnit 12 with Composer.
  2. Fix compatibility issues with PHPUnit 12 by applying this patch.
  3. Run the Unit tests for a module with --display-phpunit-notices --display-phpunit-deprecations.
  4. 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

Command icon 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:

About issue forks

Comments

dcam created an issue. See original summary.

dcam opened merge request !14483

dcam’s picture

dcam commented
Status: Active » Needs review
dcam’s picture

dcam commented
Issue summary: View changes
dcam’s picture

dcam commented
Status: Needs review » Needs work

Needs work due to PHPUnit 12.5.11 and a new deprecation.

dcam’s picture

dcam commented
Issue summary: View changes
Status: Needs work » Needs review

New development: stubs cannot set with(). The IS and MR have been updated.

The use of any() in MigrateSourceTest::testCount() and MigrateSourceTest::testCountCacheKey() are removed from the scope of this issue. Those instances represent bad tests that will be handled by #3572915: Invalid tests in MigrateSourceTest.

dcam’s picture

dcam commented
Status: Needs review » Postponed

Postponing on #3572915: Invalid tests in MigrateSourceTest.

dcam’s picture

dcam commented
Status: Postponed » Needs work

This is unblocked. NW to figure out what the next step is.

dcam’s picture

dcam commented

No 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.

dcam’s picture

dcam commented
Status: Needs work » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Tests: 417, Assertions: 1105, PHPUnit Notices: 316.

Ran with the MR and still got about 19 notices with messages to consider running as stubs.

dcam’s picture

dcam commented
Status: Needs work » Needs review

@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.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Applied the MR from the other issue and did address all.

  • catch committed 0b318c43 on 11.x 
    task: #3569422 Convert expectation-less test mocks to stubs - Migrate...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

core/.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!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • catch committed bc23f1dd on main 
    task: #3569422 Convert expectation-less test mocks to stubs - Migrate...

catch closed merge request !14483

catch’s picture

catch
he/him
Primary language English
commented
Version: main » 11.x-dev
Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • catch committed e0001c7f on 11.x 
    Revert "task: #3569422 Convert expectation-less test mocks to stubs -...
catch’s picture

catch
he/him
Primary language English
commented
Version: 11.x-dev » main

Reverted from 11.x after a similar backport caused failures running against phpunit 10. We can just commit these to main.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.