ViewExecutableTest uses a mocked argument callback wrongly (and fails in PHPUnit8) [#3102899]

Found in #3063887: Support PHPUnit 8 in Drupal 9, drop support for PHPUnit 7.

Problem/Motivation

ViewExecutableTest has mock expectation fails in PHPUnit8 due to callback return type should be bool and not string.
The callback return type is signed for PHPUnit 8, and it currently returns a string being the argument passed in - which is not necessary.

Proposed resolution

Use $this->anything() as expected argument in place of the callback.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Comment	File	Size	Author
#2	3102899-2.patch	1.17 KB	mondrake
#2	8.9.x: PHP 7.1 & MySQL 5.7 28,132 pass

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

24 December 2019 at 08:54

mondrake created an issue. See original summary.

Comment #2

mondrake

🇮🇹

CreditAttribution: mondrake as a volunteer commented 24 December 2019 at 08:59

Status:

Active

» Needs review

File	Size
3102899-2.patch	1.17 KB
8.9.x: PHP 7.1 & MySQL 5.7 28,132 pass

Patch.

Comment #3

Lendude

Dutch

Amsterdam

CreditAttribution: Lendude as a volunteer and at iO commented 28 December 2019 at 09:59

Status:

Needs review

» Reviewed & tested by the community

This is good.

Comment #4

mondrake

🇮🇹

CreditAttribution: mondrake as a volunteer commented 29 December 2019 at 17:05

I have some doubts on these tests... what's the purpose of looping through

     foreach (['field', 'filter', 'argument', 'sort'] as $handler_type) {

to have

       $display->expects($this->atLeastOnce())
         ->method('setOption')
        ->with($this->anything(), [

? In the end, it will just pass if the method is only called once, but I suppose we expect at least to be called once per handler.

Note this is so already in HEAD, this is not related to the change here.

Comment #5

31 December 2019 at 18:32

catch committed ed88513 on 9.0.x

Issue #3102899 by mondrake: ViewExecutableTest uses a mocked argument...

Comment #6

31 December 2019 at 18:32

catch committed 9df9496 on 8.9.x

Issue #3102899 by mondrake: ViewExecutableTest uses a mocked argument...

Comment #7

31 December 2019 at 18:32

catch committed 9fe9316 on 8.8.x

Issue #3102899 by mondrake: ViewExecutableTest uses a mocked argument...

Comment #8

catch

he/him

English

CreditAttribution: catch at Third and Grove commented 31 December 2019 at 18:32

Version:	8.9.x-dev	» 8.8.x-dev
Status:	Reviewed & tested by the community	» Fixed

Committed/pushed to 9.0.x/8.9.x/8.8.x, thanks! @mondrake could you open a follow-up for the issue noticed in #4, i don't think we should fix it here but worth looking at it properly.