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 library with --display-phpunit-notices --display-phpunit-deprecations.
  4. Observe the PHPUnit notices that occur.

Proposed resolution

Fix notices in core/tests/Drupal/Tests/Core/Access/.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3579172

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’s picture

dcam commented
Parent issue: » #3561671: [meta] Refactor tests to use stubs instead of mocks where mocks do not configure expectations

dcam opened merge request !15090

dcam’s picture

dcam commented
Status: Active » Needs review

I've changed tactics a bit with these library tests, which you'll probably see more of later. I'm doing more of moving property instantiations out of setUp() functions into the individual tests. There are three or four test classes where I did that in this batch.

dcam’s picture

dcam commented

FYI: the recently-committed UnitTestCase changes have been merged into this MR, so they do not need to be applied separately.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Running this one I got about 15 errors

TypeError: Drupal\Tests\Core\Access\AccessManagerTest::setupAccessArgumentsResolverFactory(): Return value must be of type PHPUnit\Framework\MockObject\Builder\InvocationMocker, PHPUnit\Framework\MockObject\InvocationStubberImplementation returned
/var/www/html/core/tests/Drupal/Tests/Core/Access/AccessManagerTest.php:606
/var/www/html/core/tests/Drupal/Tests/Core/Access/AccessManagerTest.php:193
dcam’s picture

dcam commented
Status: Needs work » Needs review

Running this one I got about 15 errors

I'm sorry that I didn't note this earlier! It looks like one of the PHPUnit return types got changed in 12.x. I can't update it in this MR because the new class doesn't exist in the version Drupal Core depends on. So that change has to wait until we actually update the dependency, otherwise it would break our tests. That means the important thing right now is for the tests to continue passing when not run on PHPUnit 12, which they just did.

smustgrave’s picture

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

Surprised they are passing but clearly I can see they do. That was the only issue I found for this one.

catch closed merge request !15090

catch’s picture

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

Committed/pushed to main, 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 35a71468 on main 
    task: #3579172 Convert expectation-less test mocks to stubs - Access...

Status: Fixed » Closed (fixed)

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