Closed NoelDeMartin closed 5 months ago
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...
OT: @NoelDeMartin, I've force-pushed (without changes) your candidate branch to get the tests executed, it seems that some of them were stuck.
All modified and coverable lines are covered by tests :white_check_mark:
Comparison is base (
ce309ac
) 85.42% compared to head (0624fc0
) 87.89%.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
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.
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!
Side note, surely this closes #15
Thanks @NoelDeMartin,
it's 98% perfect for me! The remaining 2% is:
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.docs/Help.md
page (there is a Behat section there).Thoughts?
Thanks for the review @stronk7, I have applied your suggestions:
--selenium
flag.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.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 :-)
I will merge this as soon as we are green (tests still running). Thanks for your work and patience!
Sold!
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 withMOODLE_BEHAT_SELENIUM_CHROME_IMAGE
andMOODLE_BEHAT_SELENIUM_FIREFOX_IMAGE
.I also pondered whether to allow indicating only the browser version. For example, setting
MOODLE_BEHAT_CHROME_VERSION
or acceptingchrome: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).