Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add firefox driver #162

Merged
merged 10 commits into from
Feb 29, 2024
Merged

Add firefox driver #162

merged 10 commits into from
Feb 29, 2024

Conversation

wout
Copy link
Contributor

@wout wout commented Feb 24, 2024

This implements #161. I've had to jump through some hoops to make tests pass. If you could suggest better solutions, I'll happily change the code. Here's more info on changes not directly related to the Firefox driver.

Locking down at Crystal 1.5.1
Ameba wouldn't install on Crystal 1.4.1, so I had to upgrade. 1.5 is the lowest working version.

Added support for docker compose
With the move to Go in Docker v2, the Python script docker-compose is now deprecated. But since it's still used quite often, I've updated the setup and test scripts to work with both versions.

Using headless_firefox for five failing specs
The five specs tagged with headless_chrome that I changed consistently failed, and I couldn't make them work. Changing them to headless_firefox served two purposes: the errors went away, and it proved that the Firefox driver was working. Here's the full trace of the headless_chrome errors:

       session not created: session not created: Chrome failed to start: exited normally.
         (chrome not reachable)
         (The process started from chrome location /usr/bin/chromium-browser is no longer running, so ChromeDriver is assuming that Chrome has crashed.) (Selenium::Error)
         from lib/selenium/src/selenium/http_client.cr:13:7 in 'post'
         from lib/selenium/src/selenium/command_handler.cr:22:7 in 'execute'
         from lib/selenium/src/selenium/command_handler.cr:12:5 in 'execute:parameters'
         from lib/selenium/src/selenium/driver.cr:43:12 in 'create_session'
         from lib/selenium/src/selenium/driver.cr:53:7 in 'create_session'
         from lib/selenium/src/selenium/driver.cr:41:3 in 'create_session'
         from lib/selenium/src/selenium/chrome/driver.cr:3:5 in 'create_session'
         from src/lucky_flow/selenium/driver.cr:85:5 in 'start_session'
         from src/lucky_flow/selenium/driver.cr:6:50 in 'session'
         from src/lucky_flow/selenium/driver.cr:17:5 in 'visit'
         from src/lucky_flow.cr:49:5 in 'visit'
         from spec/spec_helper.cr:43:3 in 'visit_page_with'
         from spec/lucky_flow_spec.cr:344:12 in '->'
         from /usr/share/crystal/src/spec/example.cr:45:13 in 'internal_run'
         from /usr/share/crystal/src/spec/example.cr:32:73 in '->'
         from /usr/share/crystal/src/spec/example/procsy.cr:16:15 in 'run'
         from /usr/share/crystal/src/spec/context.cr:365:11 in '->'
         from /usr/share/crystal/src/spec/example/procsy.cr:16:15 in 'run'
         from spec/spec_helper.cr:17:1 in '->'
         from /usr/share/crystal/src/spec/context.cr:71:26 in 'run_around_each_hook'
         from /usr/share/crystal/src/spec/context.cr:66:7 in 'internal_run_around_each_hooks'
         from /usr/share/crystal/src/spec/context.cr:59:7 in 'run_around_each_hooks'
         from /usr/share/crystal/src/spec/context.cr:357:13 in 'run_around_each_hooks'
         from /usr/share/crystal/src/spec/example.cr:32:15 in 'run'
         from /usr/share/crystal/src/spec/context.cr:18:23 in 'internal_run'
         from /usr/share/crystal/src/spec/context.cr:339:7 in 'run'
         from /usr/share/crystal/src/spec/context.cr:18:23 in 'internal_run'
         from /usr/share/crystal/src/spec/context.cr:156:7 in 'run'
         from /usr/share/crystal/src/spec/dsl.cr:220:7 in '->'
         from /usr/share/crystal/src/crystal/at_exit_handlers.cr:14:19 in 'run'
         from /usr/share/crystal/src/crystal/main.cr:50:5 in 'exit'
         from /usr/share/crystal/src/crystal/main.cr:45:5 in 'main'
         from /usr/share/crystal/src/crystal/main.cr:127:3 in 'main'
         from /lib/x86_64-linux-gnu/libc.so.6 in '__libc_start_main'
         from /root/.cache/crystal/crystal-run-spec.tmp in '_start'
         from ???

Styling fixes
Ameba suggested a bunch of things, so I fixed the low-impact ones and created a to-do for the ones that need more insight.

it "can hover over an element", tags: "headless_chrome" do
it "can hover over an element", tags: "headless_firefox" do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this mean we no longer test headless chrome?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, good point. I thought there were more headless chrome specs, but that's not true. I'll have to reinstate some of them and determine why Chrome fails. It's not working for me locally (elementary OS), and also not in a docker container. So I guess we won't be the only ones with he problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems the Windows GH workflow also fails using Chrome with the same errors. So something is definitely not right.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added separate specs for headless chrome and headless firefox. Chrome still fails but I'm looking into it.

@wout
Copy link
Contributor Author

wout commented Feb 25, 2024

Okay, it seems all tests now pass except for windows:

session not created: session not created: This version of ChromeDriver only supports Chrome version 122
       Current browser version is 121.0.6167.185 with binary path C:\Program Files\Google\Chrome\Application\chrome.exe (Selenium::Error)

I've changed the chrome version in the workflow from stable to beta, which is at 123.x at the moment. Let's see if that makes it pass.

@wout
Copy link
Contributor Author

wout commented Feb 29, 2024

I've enabled workflows on my fork to test the specs without having to bother you, and now they pass: https://github.com/wout/lucky_flow/actions/runs/8092510779/job/22113359024

So this should be mergeable now.

@jwoertink jwoertink merged commit f89f7b8 into luckyframework:main Feb 29, 2024
7 checks passed
@jwoertink jwoertink mentioned this pull request Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants