Allow selenium image configuration

[NoelDeMartin]

I've decided to remove the private properties because they were only used once, and now everything is encapsulated in the getSeleniumImage method.

Other than that, there isn't much more to the PR. It allows to configure the selenium image by setting MOODLE_BEHAT_SELENIUM_IMAGE, or specify it for each profile with MOODLE_BEHAT_SELENIUM_CHROME_IMAGE and MOODLE_BEHAT_SELENIUM_FIREFOX_IMAGE.

I also pondered whether to allow indicating only the browser version. For example, setting MOODLE_BEHAT_CHROME_VERSION or accepting chrome:117 for the profile. But I wasn't convinced by that approach, so I ended up using this one that also allows customizing the image completely (for example, in case you'd need to use seleniarm or something else).

[stronk7]

I like to be able to specify the images... just guessing when we should follow the env route here or just add one option (--selenium) to the command that, of course, will take precedence over the profile one.

Right now it's really a mix of env and options, grrr. Need to think a little bit more about it...

[stronk7]

OT: @NoelDeMartin, I've force-pushed (without changes) your candidate branch to get the tests executed, it seems that some of them were stuck.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (ce309ac) 85.42% compared to head (0624fc0) 87.89%.

Additional details and impacted files

@@             Coverage Diff              @@
##               main     #268      +/-   ##
============================================
+ Coverage     85.42%   87.89%   +2.46%     
- Complexity      722      725       +3     
============================================
  Files            75       75              
  Lines          2216     2222       +6     
============================================
+ Hits           1893     1953      +60     
+ Misses          323      269      -54     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[NoelDeMartin]

Thanks @stronk7, I've implemented the --selenium option as well. I still think we should keep the env variables in case someone wants to specify different images for chrome/firefox; and I think it'd be overkill to have --selenium-firefox options and such.

It seems like some tests are failing in Travis but I cannot see the details.

[stronk7]

It seems like some tests are failing in Travis but I cannot see the details.

Yeah, Travis is being a nightmare since 2 weeks ago. Sometimes it installs PostgreSQL on port 5432, others in 5432... I've been fighting with that in #270 , just to, at the end, leave the port like it's now (5432).

Will look to this soon... thanks!

[stronk7]

Side note, surely this closes #15

[stronk7]

Thanks @NoelDeMartin,

it's 98% perfect for me! The remaining 2% is:

1. Do we really need the 3 env variables? Why isn't the "general" (MOODLE_BEHAT_SELENIUM_IMAGE) one enough for everything? Whoever wants to set a custom selenium image just does it using it (or the new option) and done. I've tried to imagine use cases and I'm not sure to get the point about the browser specific ones.
2. No matter the previous point outcome, I think we need to document a little bit more the env variables. Maybe in the changelog, or maybe in the docs/Help.md page (there is a Behat section there).

Thoughts?

[NoelDeMartin]

Thanks for the review @stronk7, I have applied your suggestions:

1. I just thought it'd made sense to override them individually given that we have 2 different profiles and the same image couldn't work for both. But I don't mind removing them. Most people is likely to use only one profile, and if they don't they can now pass a different --selenium flag.
2. There are other env variables such as MOODLE_START_BEHAT_SERVERS that are not documented, so I wasn't sure if that was necessary (specially now that we have the documented --selenium flag). But I added it in the CHANGELOG.

[stronk7]

There are other env variables such as MOODLE_START_BEHAT_SERVERS that are not documented,

Yeah, I was looking for it in the Docs and only the "quick start" section of https://moodlehq.github.io/moodle-plugin-ci/Help.html was showing some options.

I've created #271 about that, surely we'll need to make a few new pages (similar to the app one) and document many of those environmental variables. In any case that falls out of this PR scope.

Ciao :-)

[stronk7]

I will merge this as soon as we are green (tests still running). Thanks for your work and patience!

[stronk7]

Sold!