-
Notifications
You must be signed in to change notification settings - Fork 413
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 NewSeleniumServiceV4
to support cli args for selenium version 4
#275
base: master
Are you sure you want to change the base?
Conversation
Would it be possible to merge this? @minusnine |
@@ -194,6 +194,37 @@ func NewSeleniumService(jarPath string, port int, opts ...ServiceOption) (*Servi | |||
return s, nil | |||
} | |||
|
|||
func NewSeleniumServiceV4(jarPath string, port int, opts ...ServiceOption) (*Service, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a documentation comment.
https://github.com/golang/go/wiki/CodeReviewComments#doc-comments
@@ -194,6 +194,37 @@ func NewSeleniumService(jarPath string, port int, opts ...ServiceOption) (*Servi | |||
return s, nil | |||
} | |||
|
|||
func NewSeleniumServiceV4(jarPath string, port int, opts ...ServiceOption) (*Service, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
selenium_test.go needs to be updated to test Selenium v4:
Line 22 in e9100b7
selenium3Path = flag.String("selenium3_path", "", "The path to the Selenium 3 server JAR. If empty or the file is not present, Firefox tests using Selenium 3 will not be run.") |
Line 131 in e9100b7
t.Run("Selenium3", func(t *testing.T) { |
Line 161 in e9100b7
} else { |
Line 196 in e9100b7
t.Run("Selenium3", func(t *testing.T) { |
Line 227 in e9100b7
SeleniumVersion: semver.MustParse("3.0.0"), |
Line 284 in e9100b7
s, err = selenium.NewSeleniumService(webDriverPath, port, c.ServiceOptions...) |
We should also add Selenium v4 to be automatically downloaded by vendor/init.go:
Line 63 in e9100b7
url: "https://selenium-release.storage.googleapis.com/3.141/selenium-server-standalone-3.141.59.jar", |
I'm also not sure that this is correct. The PR uses
(same for POST) @Prendo93 did this actually work for you? |
Can confirm this is working for me with https://github.com/SeleniumHQ/selenium/releases/download/selenium-4.1.0/selenium-server-4.1.1.jar I think the urlPrefix is named thus because there are other routes called, eg: |
Hm, ok. Do you specify a prefix? Or do you omit it? Would you mind posting a quick example where it's running for you? Would help tremendously! Edit: it actually does work! It just doesn't work with the selenium standalone docker images for some reason. |
Selenium V3 seems to now be considered legacy (https://www.selenium.dev/documentation/legacy/selenium_3/). This PR adds NewSeleniumServiceV4 which sets up the service using the new format for v4. During my (admittedly limited) testing it seems to work as a drop-in replacement for Selenium V3 implementation.