Fixes #36978 - Add possibility to use remote webdriver #9952

dosas · 2023-12-13T07:48:09Z

In order to easily execute the selenium tests it would be good to have the possibility to use selenium remote webdriver images

docker run -d -p 4444:4444 --shm-size="2g" selenium/standalone-chrome:119.0

To access the driver via vagrant or a CI docker image the IP address needs to be set using an evironment variable:

SELENIUM_REMOTE_HOST=<ip_of_docker_host>
JS_TEST_DRIVER=selenium_chrome_remote

There are certain environments that require different chrome options to work so it is possible to add them via semicolon separated string as environment variable:

ADDITIONAL_CHROME_OPTIONS="--disable-dev-shm-usage"

theforeman-bot · 2023-12-13T07:48:12Z

Can one of the admins verify this patch?

theforeman-bot · 2023-12-13T07:48:12Z

Can one of the admins verify this patch?

theforeman-bot · 2023-12-13T07:48:12Z

Can one of the admins verify this patch?

evgeni · 2023-12-14T06:31:25Z

ok to test

evgeni · 2023-12-15T10:51:34Z

@MariaAga would you mind looking at this? Looks totally reasonable to me, but my knowledge around the integration tests is rather low.

test/integration_test_helper.rb

ekohl

I've opened #9962 which should provide a cleaner base for this PR.

test/integration_test_helper.rb

test/test_helper.rb

test/integration_test_helper.rb

dosas · 2023-12-21T12:22:51Z

I have no idea why the labeler fails maybe someone can help?

evgeni · 2023-12-21T12:24:51Z

I have no idea why the labeler fails maybe someone can help?

that's just broken, ignore it for now :)

ekohl

Some small nitpicks.

A bigger question here is how we're testing in GitHub Actions today and if this would actually be better. I don't know the current chromedriver & chrome are installed.

test/integration_test_helper.rb

ekohl

This is just me reading the code. I hope to find some time to actually play around with this myself.

test/integration_test_helper.rb

dosas · 2024-01-31T08:16:03Z

Any news on this?

dosas · 2024-03-12T08:18:56Z

@ekohl what do we need to do to get this forward?

dosas · 2024-03-21T09:36:13Z

ping

dosas · 2024-04-15T14:45:04Z

@ekohl Do you need any additional information on the answers from above?

test/integration/compute_profile_js_test.rb

ekohl · 2024-06-27T15:37:22Z

test/integration_test_helper.rb

+  def work_around_selenium_file_detector_bug
+    # see: https://stackoverflow.com/questions/70441796/selenium-webdriver-for-aws-device-farm-error-when-sending-period-keystroke-t


If I'm really pedantic I'd suggest yardoc tags (which we don't use enough in our codebase).

Suggested change

def work_around_selenium_file_detector_bug

# see: https://stackoverflow.com/questions/70441796/selenium-webdriver-for-aws-device-farm-error-when-sending-period-keystroke-t

# @see https://stackoverflow.com/questions/70441796/selenium-webdriver-for-aws-device-farm-error-when-sending-period-keystroke-t

def work_around_selenium_file_detector_bug

I will change that but shouldn't we add a rubocop for it then and if the community decides that this is a valid cop we can do it like that?

ekohl · 2024-06-27T15:52:16Z

test/integration_test_helper.rb

+elsif javascript_driver == :selenium_chrome_headless
+  options = chrome_options
+  options.args << '--headless'
+  Capybara.register_driver :selenium_chrome_headless do |app|
+    Capybara::Selenium::Driver.new(
+      app,
+      browser: :chrome,
+      options: options)
+  end


Why is this needed? This should be available by default:
https://github.com/teamcapybara/capybara/blob/0480f90168a40780d1398c75031a255c1819dce8/lib/capybara/registrations/drivers.rb#L31-L42

I had hoped to utilize this if possible. Given we don't set the window size elsewhere, I'd be tempted to drop chrome_options. I'm not sure anyone actually uses ADDITIONAL_CHROME_OPTIONS anyway.

As far as I remember (I touched this code 5 months ago) but the window size was required. Although the discussion if that is needed is IMO out of scope for this PR (can be done in follow up PR)

I'm requiring ADDITIONAL_CHROME_OPTIONS downstream. This will make it more easily configurable.

I think its a good idea to be able to set additonal options which can be used. They are optional. So, if you don't need them, don't set the option and it doesn't hurt.

ekohl · 2024-06-27T15:53:01Z

test/integration_test_helper.rb

+  end
+elsif javascript_driver == :selenium_chrome_headless
+  options = chrome_options
+  options.args << '--headless'


If we keep this, there's also a dedicated API:

Suggested change

options.args << '--headless'

options.headless!

I don't understand, can you please explain in more detail?

pr-processor bot added the Not yet reviewed label Dec 13, 2023

evgeni requested a review from MariaAga December 15, 2023 10:51

stejskalleos reviewed Dec 19, 2023

View reviewed changes

test/integration_test_helper.rb Outdated Show resolved Hide resolved

ekohl mentioned this pull request Dec 19, 2023

Fixes #36997 - Use selenium_chrome_headless for integration tests #9962

Open

ekohl requested changes Dec 19, 2023

View reviewed changes

pr-processor bot added Waiting on contributor and removed Not yet reviewed labels Dec 19, 2023

dosas force-pushed the feature/remote-selenium-webdriver branch from 9529ef0 to f89c5ac Compare December 21, 2023 12:11

pr-processor bot added Needs re-review and removed Waiting on contributor labels Dec 21, 2023

dosas force-pushed the feature/remote-selenium-webdriver branch 2 times, most recently from 826ba53 to 40b0f2a Compare December 21, 2023 14:43

ekohl mentioned this pull request Jan 11, 2024

Rewrite GHA workflow using reusable actions theforeman/foreman_puppet#381

Merged

dosas force-pushed the feature/remote-selenium-webdriver branch 4 times, most recently from c369a67 to ac3b724 Compare January 15, 2024 13:56

dosas requested review from evgeni, stejskalleos and ekohl January 15, 2024 14:28

ekohl reviewed Jan 16, 2024

View reviewed changes

test/integration_test_helper.rb Outdated Show resolved Hide resolved

test/integration_test_helper.rb Outdated Show resolved Hide resolved

dosas force-pushed the feature/remote-selenium-webdriver branch from ac3b724 to d44b14f Compare January 18, 2024 07:57

dosas requested a review from ekohl January 18, 2024 08:28

dosas force-pushed the feature/remote-selenium-webdriver branch from d44b14f to 59b757a Compare January 18, 2024 12:14

ekohl reviewed Jan 18, 2024

View reviewed changes

dosas requested a review from ekohl January 18, 2024 14:23

dosas force-pushed the feature/remote-selenium-webdriver branch from 59b757a to f871fff Compare January 18, 2024 14:54

dosas mentioned this pull request Jan 25, 2024

Adapt foreman_puppet tests to run with remote_webdriver theforeman/foreman_puppet#382

Closed

stejskalleos self-assigned this Jan 30, 2024

dosas force-pushed the feature/remote-selenium-webdriver branch 2 times, most recently from bb361dc to 72a9c95 Compare January 30, 2024 15:34

stejskalleos removed their assignment Feb 13, 2024

dosas force-pushed the feature/remote-selenium-webdriver branch from 72a9c95 to faf5a31 Compare May 23, 2024 07:05

dosas force-pushed the feature/remote-selenium-webdriver branch from faf5a31 to 8577c35 Compare June 17, 2024 06:43

ekohl reviewed Jun 27, 2024

View reviewed changes

test/integration/compute_profile_js_test.rb Outdated Show resolved Hide resolved

Fixes #36978 - Add possibility to use remote webdriver

76f1da7

dosas force-pushed the feature/remote-selenium-webdriver branch from 8577c35 to 76f1da7 Compare June 27, 2024 14:28

ekohl approved these changes Jun 27, 2024

View reviewed changes

pr-processor bot removed the Needs re-review label Jun 27, 2024

stejskalleos removed their request for review July 1, 2024 06:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #36978 - Add possibility to use remote webdriver #9952

Fixes #36978 - Add possibility to use remote webdriver #9952

dosas commented Dec 13, 2023 •

edited

Loading

theforeman-bot commented Dec 13, 2023

theforeman-bot commented Dec 13, 2023

theforeman-bot commented Dec 13, 2023

evgeni commented Dec 14, 2023

evgeni commented Dec 15, 2023

ekohl left a comment

dosas commented Dec 21, 2023

evgeni commented Dec 21, 2023

ekohl left a comment

ekohl left a comment

dosas commented Jan 31, 2024

dosas commented Mar 12, 2024

dosas commented Mar 21, 2024

dosas commented Apr 15, 2024

ekohl Jun 27, 2024

dosas Jul 1, 2024 •

edited

Loading

ekohl Jun 27, 2024

dosas Jul 1, 2024

sbernhard Jul 8, 2024

ekohl Jun 27, 2024

dosas Jul 1, 2024

		def work_around_selenium_file_detector_bug
		# see: https://stackoverflow.com/questions/70441796/selenium-webdriver-for-aws-device-farm-error-when-sending-period-keystroke-t

Fixes #36978 - Add possibility to use remote webdriver #9952

Are you sure you want to change the base?

Fixes #36978 - Add possibility to use remote webdriver #9952

Conversation

dosas commented Dec 13, 2023 • edited Loading

theforeman-bot commented Dec 13, 2023

theforeman-bot commented Dec 13, 2023

theforeman-bot commented Dec 13, 2023

evgeni commented Dec 14, 2023

evgeni commented Dec 15, 2023

ekohl left a comment

Choose a reason for hiding this comment

dosas commented Dec 21, 2023

evgeni commented Dec 21, 2023

ekohl left a comment

Choose a reason for hiding this comment

ekohl left a comment

Choose a reason for hiding this comment

dosas commented Jan 31, 2024

dosas commented Mar 12, 2024

dosas commented Mar 21, 2024

dosas commented Apr 15, 2024

ekohl Jun 27, 2024

Choose a reason for hiding this comment

dosas Jul 1, 2024 • edited Loading

Choose a reason for hiding this comment

ekohl Jun 27, 2024

Choose a reason for hiding this comment

dosas Jul 1, 2024

Choose a reason for hiding this comment

sbernhard Jul 8, 2024

Choose a reason for hiding this comment

ekohl Jun 27, 2024

Choose a reason for hiding this comment

dosas Jul 1, 2024

Choose a reason for hiding this comment

dosas commented Dec 13, 2023 •

edited

Loading

dosas Jul 1, 2024 •

edited

Loading