Problem/Motivation

Without updating jcalderonzumba/mink-phantomjs-driver we can't pass the PHP 7.2 tests because it is not conforming interface expectations.

Proposed resolution

Update the dependency in composer.lock.

Run composer update jcalderonzumba/mink-phantomjs-driver --with-dependencies

Note that #2832672: [PP-1] Upgrade jcalderonzumba/* for better test performance is trying to update more but this is doing the minimal to get PHP 7.2 test coverage passing.

Remaining tasks

User interface changes

None

API changes

None

Data model changes

None

CommentFileSizeAuthor
#5 2929477-2-5.patch2.58 KBalexpott
#2 2929477-2.patch2.58 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 Thunder commented
Issue summary: View changes
Status: Active » Needs review
FileSize
2929477-2.patch2.58 KB
droplet’s picture

droplet CreditAttribution: droplet commented

the extra in another issue prevents the tests errors after updates:
https://github.com/jcalderonzumba/MinkPhantomJSDriver/pull/68

Let's see what the tesbots say first

Status: Needs review » Needs work

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

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Thunder commented
Status: Needs work » Needs review
FileSize
2929477-2-5.patch2.58 KB

Ho hum let's see if we have any more luck on 0.3.2 since that has the fix we need for PHP 7.2

alexpott’s picture

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

The fix we need for PHP 7.2 is https://github.com/jcalderonzumba/MinkPhantomJSDriver/commit/9458e357b22... - the reason it fixes the PHP 7.2 errors is that behat/mink uses the return value and when the driver doesn't find anything it incorrectly returns NULL instead of an empty array on version 0.3.1. Locally the tests that fail on 0.3.3 pass so fingers-crossed we can do this minimal change to get 7.2 fixed and then get back to #2832672: [PP-1] Upgrade jcalderonzumba/* for better test performance - however I think that some people are trying to move us away from gastonjs and phantomjs so maybe the further work won't be necessary.

droplet’s picture

droplet CreditAttribution: droplet commented

Yes, I agree we can get a quick fix first.

Locally the tests that fail on 0.3.3 pass so fingers-crossed

It's not surprising, we have quite a lot JSTests random fails on d.org testbots only.

however I think that some people are trying to move us away from gastonjs and phantomjs so maybe the further work won't be necessary.

We still need that for 2 reasons:

1. the ChromeDrive / Webdriver runs even faster than PhantomJS. Tests fail because we shorten the waiting period. We can take the https://www.drupal.org/project/drupal/issues/2832672#comment-11931929 and then follow-ups to polish the functions.

2. It brings real benefits to the tests even without fails.

alexpott’s picture

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

So yep this allows us to get a passing test suite on PHP 7.2 see #2927806-61: Use PHPUnit 6 for testing when PHP version >= 7.2

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner at Chapter Three commented
Status: Needs review » Reviewed & tested by the community

I agree with @alexpott and @droplet we should update mink-phantomjs as long we still use it.

catch’s picture

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

Committed/pushed to 8.5.x, thanks!

  • catch committed 2970927 on 8.5.x 
    Issue #2929477 by alexpott, droplet: Update jcalderonzumba/mink-...

Status: Fixed » Closed (fixed)

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