moodlehq / moodle-docker

A docker environment for moodle developers
GNU General Public License v3.0
376 stars 245 forks source link

Selenium versions rebased #195

Closed aspark21 closed 2 years ago

aspark21 commented 2 years ago

Rebased #60 and made Readme docs more explicit that the tag is the selenium version not the browser version. (hoping this addresses https://github.com/moodlehq/moodle-docker/pull/60#issuecomment-331009133)

Been discussing in #154

I will go off and test it out against Windows/Mac/Linux but would be useful to know if any further tweaks are required.

aspark21 commented 2 years ago

Could rename MOODLE_DOCKER_BROWSER_TAG to MOODLE_DOCKER_SELENIUM_TAG but since the MOODLE_DOCKER_BROWSER_TAG is extracted from MOODLE_DOCKER_BROWSER there is a logic to that name.

stronk7 commented 2 years ago

Awesome sauce @aspark21 !

Only details that comes to my brain is that, recently, SeleniumHQ has started to stop building the 3.x images and they are only building the 4.x ones.

But... we aren't ready yet to 4.x ones (there are some ongoing PRs to get them working by default).

So great that you've pinned the defaults for both to be 3.x. In fact, unless I'm wrong, I think you can make both them to be "3" and done. And, maybe, those defaults should be explained in the README, clearly stating that the default is "firefox:3".

But other than that, if compose works ok for you for both unix and windows... yayayay!

stronk7 commented 2 years ago

And yeah, I think MOODLE_DOCKER_BROWSER_TAG is fair enough. Alternatives like MOODLE_DOCKER_SELENIUM_TAG (or MOODLE_DOCKER_BROWSER_SELENIUM[TAG]) are similarly imperfect. As far as it's clear (in the docs and in code) that it's a selenium docker image tag... all ok, I think.

Edited: They are internal to code, in any case, so users won't see them ever.

stronk7 commented 2 years ago

For reference, this will close: #154, #191, #194, #20 and #60

aspark21 commented 2 years ago

Linux full run has completed. Ran it with MOODLE_DOCKER_BROWSER: firefox:3 That seemed to have reduced our Behat failures by 4 but full build (master +120 plugins) run increased runtime from 9h57m to 11h23m. We probably have some night time system processes slowing things down so won't take note of that. Either way, this means we can indeed unpin the selenium version for Firefox.

Have changed that accordingly, including the README.

Moving onto Windows testing

aspark21 commented 2 years ago

Windows testing complete. behat2

behat-windows

Should be all clear to merge.

aspark21 commented 2 years ago

Happy New Year!

Giving this a nudge as this is ready to be integrated. We've done a good 100 runs with this while reworking our pipelines and this has been working well.

stronk7 commented 2 years ago

Happy new year!

And thanks for all the work here, merged!

I'm going to close related issues (#154, #191, #194, #20 and #60).

Ciao :-)