Problem/Motivation

#3044155: PHP7.1 vendor max Testing issue - DO NOT COMMIT shows

Drupal\BuildTests\Framework\Tests\DrupalMinkClientTest
fail: [Other] Line 0 of sites/default/files/simpletest/phpunit-2.xml:
PHPunit Test failed to complete; Error: Declaration of Drupal\BuildTests\Framework\DrupalMinkClient::followMetaRefresh($followMetaRefresh = true) should be compatible with Symfony\Component\BrowserKit\Client::followMetaRefresh(bool $followMetaRefresh = true)

Proposed resolution

Fix it

Remaining tasks

User interface changes

None

API changes

None

Data model changes

None

Release notes snippet

N/a

CommentFileSizeAuthor
#2 3086005-2.patch672 bytesalexpott
#2 3086005-2-test.patch1.22 KBalexpott
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce commented
Status: Active » Needs review
FileSize
3086005-2-test.patch1.22 KB
3086005-2.patch672 bytes

The last submitted patch, 2: 3086005-2-test.patch, failed testing. View results

mikelutz’s picture

mikelutz
He/Him
Primary language English
Location Michigan, USA
CreditAttribution: mikelutz as a volunteer and at ICON Interactive commented
Status: Needs review » Reviewed & tested by the community

Looks good. I know we are trying to avoid scaler type hints until 8.7 is eol, but extensions of vendor code will need a few exceptions.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented

Hey! Max tests to the rescue.

+1 on #4, and also: This is in code that's not in 8.7 and probably won't be.

mondrake’s picture

mondrake
Location 🇮🇹
CreditAttribution: mondrake as a volunteer commented

Does this have any implication on MIN testing, too? i.e. should we also constrain the package in composer.json so that it won't downgrade to a version that has not the method with scalar typehints?

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce commented

@mondrake our current version in composer.lock does not have scalar typehints... otherwise HEAD would be failing.

  • catch committed beaa069 on 8.8.x 
    Issue #3086005 by alexpott: Make DrupalMinkClientTest::followMetaRefresh...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Agreed with the reasoning in #4. Committed beaa069 and pushed to 8.8.x. Thanks!

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented

Another similar one: #3086557: Make DrupalMinkClient::request() compatible with max dependencies

Status: Fixed » Closed (fixed)

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