#2942900-13: Convert JavascriptTestBase Tests to use DrupalSelenium2Driver point 3 shows us that the Current implementation of the Drupal2Selenium driver isn't capable of handling file uploads.

I've researched this, and it appears to be a bug in the behat/mink-selenium2-driver library that we're relying on.

https://github.com/minkphp/MinkSelenium2Driver/issues/187
https://github.com/minkphp/MinkSelenium2Driver/pull/252

It appears as though pfrenssen ran into this independently, and fixed it, however there has not yet been a new release of the driver since it was fixed.

https://github.com/minkphp/MinkSelenium2Driver/issues/269

Upgrading to behat/mink-selenium2-driver:1.3.x-dev on my local dev fixes the file uploading issues for the following six tests:

Drupal\Tests\system\FunctionalJavascript\ThemeFormSettingsTest
Drupal\Tests\file\FunctionalJavascript\MaximumFileSizeExceededUploadTest
Drupal\Tests\image\FunctionalJavascript\QuickEditImageTest
Drupal\Tests\media\FunctionalJavascript\MediaSourceAudioVideoTest
Drupal\Tests\media\FunctionalJavascript\MediaSourceFileTest
Drupal\Tests\media\FunctionalJavascript\MediaSourceImageTest

Im not quite sure the process we go through to do composer upgrades of packages, and whether or not we're all that keen on having an upstream dev version (that isnt passing its own tests), or if we'd rather fork and apply pfrenssen's pull request to 1.3.1.

CommentFileSizeAuthor
#4 2944291-4.patch2.65 KBjibran
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Mixologic created an issue. See original summary.

Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
CreditAttribution: Mixologic at Drupal Association commented
Issue summary: View changes
jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran as a volunteer commented
Title: upgrade behat/mink-selenium2-driver to 1.3.x-dev » Upgrade behat/mink-selenium2-driver to 1.3.x-dev

We already have "behat/mink": "1.7.x-dev", in dev section so I don't see any reason to not update this too.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran as a volunteer commented
Category: Feature request » Task
Status: Active » Needs review
FileSize
2944291-4.patch2.65 KB

Here is the patch.

Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
CreditAttribution: Mixologic at Drupal Association commented

Funny thing here is that this is sortof chicken and egg. This patch upgrades the mink library that only one test currently relies on. Should we add this change back in on the parent issue to demonstrate that it works (it does, for me, in local testing, but works on my machine isnt really a high enough bar)?

It would mean that three of the four media tests that required manual override of the Phantomjs lib wouldn't be needed, but also means the patch becomes slightly more complex to review and understand...

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran as a volunteer commented

Should we add this change back in on the parent issue to demonstrate that it works

You should post a combined patch to the #2942900: Convert JavascriptTestBase Tests to use DrupalSelenium2Driver. Once we'll have those test passing we'll postpone #2942900 on this one and get it fixed.

Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
CreditAttribution: Mixologic at Drupal Association commented
Status: Needs review » Reviewed & tested by the community

Thanks bajillions @jibran.

Given that the combined patch shows that we've not exposed any other bugs with the upgrade(https://www.drupal.org/project/drupal/issues/2942900#comment-12479928) , this ought to go in first.

alexpott’s picture

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

Committed and pushed 3b0be18a1a to 8.6.x and 2a5cdaec84 to 8.5.x. Thanks!

Backported to 8.5.x as a test-only change.

  • alexpott committed 3b0be18 on 8.6.x 
    Issue #2944291 by jibran, Mixologic: Upgrade behat/mink-selenium2-driver...

  • alexpott committed 2a5cdae on 8.5.x 
    Issue #2944291 by jibran, Mixologic: Upgrade behat/mink-selenium2-driver...
Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
CreditAttribution: Mixologic at Drupal Association commented

👏👏👏

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Related issues: +#2947517: Selenium driver: API to get remote file paths

🌟Cool step! Let's take one more step in #2947517: Selenium driver: API to get remote file paths

Status: Fixed » Closed (fixed)

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