Problem/Motivation

I've stumbled over the following inspection in the PHPStorm plugin "Php Inspections ​(EA Extended)​" called "promote ->willReturn*".

This points out PHPUnit MockObject ->expects()->will() which have $this->returnValue as a stub and replaces them with ->expects()->willReturn() (and friends).

I think at the very least it improves readability.

Since this is going to be a big patch touching _a lot_ of Test Classes, this is going to be something we want to (attempt to) commit in the 9.5 beta phase.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3295880

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

Spokje created an issue. See original summary.

Spokje opened merge request !2512

Spokje opened merge request !2513

Spokje closed merge request !2512

Spokje opened merge request !2514

spokje’s picture

spokje
Primary language Dutch
commented
Assigned: spokje » Unassigned
Status: Active » Needs review
spokje’s picture

spokje
Primary language Dutch
commented
Title: [w/c September 17th] ->willReturn(...) would make more sense here » ->willReturn(...) would make more sense here
Assigned: Unassigned » spokje
Status: Needs review » Needs work
spokje’s picture

spokje
Primary language Dutch
commented
Assigned: spokje » Unassigned
Status: Needs work » Needs review
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

Reviewed with git diff --color-words, everything is as expected. The only remaining uses of $this->returnValue* in core are:

core/modules/views_ui/tests/src/Unit/ViewListBuilderTest.php
94:        $this->returnValue('test_page'),
95:        $this->returnValue('<object>malformed_path</object>'),
96:        $this->returnValue('<script>alert("placeholder_page/%")</script>')));

core/tests/Drupal/Tests/Core/UrlTest.php
367:        ->will($this->returnValueMap($map, $index));

Should we just clean these up here as well and be done with it?

spokje’s picture

spokje
Primary language Dutch
commented

Thanks @longwave, changed the last two remaining $this->returnValue

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs review » Reviewed & tested by the community

Thank you @Spokje!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed fdaf5636e4 to 10.1.x and 7b462737ca to 10.0.x. Thanks!
Committed 6cb7c2a and pushed to 9.5.x. Thanks!

  • alexpott committed fdaf563 on 10.1.x 
    Issue #3295880 by Spokje, longwave: ->willReturn(...) would make more...

  • alexpott committed abe9891 on 10.0.x 
    Issue #3295880 by Spokje, longwave: ->willReturn(...) would make more...

  • alexpott committed cdcba2c on 9.5.x 
    Issue #3295880 by Spokje, longwave: ->willReturn(...) would make more...

Spokje closed merge request !2513

Spokje closed merge request !2514

Status: Fixed » Closed (fixed)

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