moodlehq / moodle-docker

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

Allow configuration of the selenium docker repo #250

Open andrewnicols opened 1 year ago

andrewnicols commented 1 year ago

For ARM chips, you may wish to use the seleniarm repo instead of selenium.

Please note that the 'seleniarm' repository contains officially sanctioned ARM images:

https://github.com/SeleniumHQ/docker-selenium#experimental-mult-arch-aarch64armhfamd64-images

stronk7 commented 1 year ago

I was going to suggest to use, under Windows, the %PROCESSOR_ARCHITECTURE% to, similarly, look under Windows for a better default.

But them I've realised that there isn't Windows Docker Desktop for Arm64 yet (or I've not been able to find it).

So, could we just add a tiny comment after https://github.com/moodlehq/moodle-docker/pull/250/files#diff-31cc1d735f2a6a04693553db3ac6be87f39766a315913557db6a0921bf1bb40bR114 in order to say that, once there is Windows Docker Desktop for Arm we'll need to do something similar there (with %PROCESSOR_ARCHITECTURE%).

With that tiny detail, I'm happy processing this, thanks!

stronk7 commented 1 year ago

Note this fixes #237

stronk7 commented 1 year ago

And... one real problem... is that we default to MOODLE_DOCKER_BROWSER_TAG=3 and there aren't 3.x selearm images... so the docker compose command ends with error.

Maybe we should bump to 4 as default? Or only do it for ARM ? I've seen that, for core... they are not 100% ready / passing ... not sure ... there is https://github.com/moodlehq/moodle-ci-runner/pull/87 related to that.

stronk7 commented 1 year ago

Hi @andrewnicols ,

  1. I've added a tiny comment to the CMD as TODO, to point to the need of adding a condition if somedays there is a Windows Docker Desktop Arm64 available. Feel free to squash it.
  2. More important (see my previous comment), what to do with our current default of Sele3, when there aren't Sele3 images in selearm? Should we bump to Sele4 as default? At some point we certainly will have to, so...

Ciao :-)