Problem/Motivation

In #3306554: InvocationMocker::withConsecutive() is deprecated in PHPUnit 9.6 and removed from PHPUnit 10 Drupal created a workaround that mimics the behavior or InvocationMocker::withConsecutive() to facilitate the upgrade to PHPUnit 10 but the method was removed because it was complicated and was code smell for tests that where generally over complicated and mocking too much.

https://thephp.cc/articles/do-not-mock-what-you-do-not-own

This replacement is just a stop gap though and each use should be reviewed and refactored to use a different mechanism of mocking or stubbing to improve the test.

Proposed resolution

  • Replace uses of MockTrait::consecutiveCalls
  • Deprecate MockTrait::consecutiveCalls

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Comments

neclimdul created an issue. See original summary.

catch’s picture

catch
he/him
Primary language English
commented

Is this done?

spokje’s picture

spokje
Primary language Dutch
commented

Nope, still four suppressed warnings Call to deprecated method withConsecutive() of class PHPUnitFrameworkMockObjectBuilderInvocationMocker. in the PHPStan baseline.

catch’s picture

catch
he/him
Primary language English
commented

Yeah I should have grepped instead of asking:

core/tests/Drupal/Tests/Core/UrlTest.php
core/tests/Drupal/Tests/Core/TempStore/PrivateTempStoreTest.php
core/tests/Drupal/Tests/Core/TempStore/SharedTempStoreTest.php
core/tests/Drupal/Tests/Core/Routing/RouteBuilderTest.php
andypost’s picture

andypost
he/him
Primary language Russian
commented

With applied patch from #3419288: Remove withConsecutive() in RouteBuilderTest

core$ git grep withConsecutive

core/modules/migrate/tests/src/Unit/MigrateExecutableMemoryExceededTest.php:85:        ->withConsecutive(
core/phpstan-baseline.neon:3407:                        message: "#^Call to deprecated method withConsecutive\\(\\) of class PHPUnit\\\\Framework\\\\MockObject\\\\Builder\\\\InvocationMocker\\.$#"
core/tests/Drupal/Tests/Core/UrlTest.php:123:      ->withConsecutive(
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

PHPStan hasn't flagged MigrateExecutableMemoryExceededTest for some reason.

longwave credited mondrake.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Active » Needs review

All child issues are in. Also crediting @mondrake here for discussions and the techniques learned in #3306554: InvocationMocker::withConsecutive() is deprecated in PHPUnit 9.6 and removed from PHPUnit 10

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

Saving credits.

neclimdul’s picture

neclimdul
He/Him
commented
Parent issue: #3306554: InvocationMocker::withConsecutive() is deprecated in PHPUnit 9.6 and removed from PHPUnit 10 » #3217904: [meta] Support PHPUnit 10 in Drupal 11

Moving parent since we dropped the deprecation path.

Thought this had stalled but looks like a lot of great work was happening I just wasn't watching the right issues! Thanks!

smustgrave’s picture

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

With the 2 RTBC tickets applied and searching for withConsecutive seems all have been replaced.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs change record updates

Committed the last two RTBC. But https://www.drupal.org/node/3365413 needs an update. I think it might be a good idea to link to core issues with examples and show a couple of diffs on the MR for different approaches like using callbacks.

joachim’s picture

joachim commented

> Instead, prophecies should be instantiated from the prophet factory directly

Does this mean using it like how it's explained here?

https://github.com/phpspec/prophecy?tab=readme-ov-file#how-to-use-it

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs work » Fixed
Issue tags: -Needs change record updates

I updated https://www.drupal.org/node/3365413 to point to more examples in core and remove the reference to the shim that we ended up not needing, I think that's enough to close this issue but the CR could use more help in general - however it's going to be very hard to provide example before/after since it all depends on what people are doing.

quietone’s picture

quietone commented

@catch, thanks for updating the change records

Status: Fixed » Closed (fixed)

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