Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[runner] Update to Chromedriver 96.0 custom #66

Merged
merged 2 commits into from
Dec 9, 2021
Merged

Conversation

andrewnicols
Copy link
Collaborator

@andrewnicols andrewnicols commented Nov 30, 2021

@andrewnicols
Copy link
Collaborator Author

WIP still... have to recompile chrome.

@andrewnicols
Copy link
Collaborator Author

Okay, I've worked out what's going on.

Selenium 4.X validates paramers, and does not like the marionette = null capability because that capability is a boolean.

I've raised moodlehq/moodle-behat-extension#50 moodlehq/moodle-behat-extension#51 moodlehq/moodle-behat-extension#52 moodlehq/moodle-behat-extension#53 to work around it in our driver.

I've also raised Behat/MinkExtension#377 against Behat/MinkExtension and FriendsOfBehat/MinkExtension#10 against the FOB version.

Tests above are now passing (except one which is not a symptom of this issue.

@stronk7
Copy link
Member

stronk7 commented Dec 1, 2021

Hi Andrew,

nice work! Just sharing that the complete run above, with the new sele-4 image has ended with a couple of the infame zero-size failures. (link.

My understanding is that, no matter of that... still this is a good change because will kill the on-the-edge failures. And also enable CI to use sele-4.

So I'm going to proceed. Once merged I'll look to the moodle-behat-extensions, tagging them and creating a MDL to update our branches.

Finally, about the Mink extension, just commenting about we already using our own fork of the FriendsOfBehat one, because they never applied some PHP8 fixes to it (from memory). So surely we need to apply the same patch to:

https://github.com/moodlehq/MinkExtension.git

Ciao :-)

@stronk7
Copy link
Member

stronk7 commented Dec 1, 2021

Just had a thought... if we fix the marionette problem in our MinkExtension fork... then surely we don't need to do anything in moodle-behat-extension?

@stronk7
Copy link
Member

stronk7 commented Dec 1, 2021

I've relaunched the Chrome job above, with the new rebuilt once again selenium 96 image. Let's see how it goes this time:

https://ci.moodle.org/view/Testing/job/DEV.01%20-%20Developer-requested%20Behat/18810/

@andrewnicols
Copy link
Collaborator Author

I've just built it against 3.141.59 and kicked off a job for comparison of speed:
https://ci.moodle.org/view/Testing/job/DEV.01%20-%20Developer-requested%20Behat/18817/console

There's some speed issues reported in SeleniumHQ/docker-selenium#1444 and it looks like S4 is 33% slower!

@andrewnicols
Copy link
Collaborator Author

@stronk7
Copy link
Member

stronk7 commented Dec 2, 2021

Yay, so it passed! In 3h22m!

Little detail, I saw that https://hub.docker.com/r/moodlehq/selenium-standalone-chrome is still saying sele-4 instead of sele-3 (in case we are going to stay with sele-3 finally).

For the records, I've launched one more run, this time using the STABLE runner (current) against the very same commit, to confirm that, without the new image, we were getting the failures:

https://ci.moodle.org/view/Testing/job/DEV.01%20-%20Developer-requested%20Behat/18850/

@stronk7
Copy link
Member

stronk7 commented Dec 9, 2021

I've created moodlehq/selenium-standalone-chrome#1 to remember about the non-matching README information.

@stronk7 stronk7 merged commit 191b387 into master Dec 9, 2021
@Yozhef
Copy link

Yozhef commented Dec 13, 2021

@stronk7 @andrewnicols I apologize for blocking you (
Now I have been added to the team and I can help you with that

FriendsOfBehat/MinkExtension#10

@stronk7
Copy link
Member

stronk7 commented Dec 13, 2021

Thanks @Yozhef !

I imagine this implies that we can now stop using our MinkExtension fork and switch back to the FriendsOfBehat one.

Ciao :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants