Run tests in webdriver for CI, both firefox and chrome #14
Conversation
| import com.google.gwt.core.ext.UnableToCompleteException; | ||
| import com.google.gwt.junit.JUnitShell; | ||
| import com.google.gwt.junit.RunStyle; | ||
| import java.net.*; |
There was a problem hiding this comment.
| import org.openqa.selenium.remote.DesiredCapabilities; | ||
| import org.openqa.selenium.remote.RemoteWebDriver; | ||
|
|
||
| public class RunStyleWebDriver extends RunStyle { |
There was a problem hiding this comment.
That class should probably be named RunStyleRemoteWebDriver, as it doesn't support running "local" WebDriver.
| String[] parts = args.split("\\?"); | ||
| URL remoteAddress = null; | ||
| try { | ||
| remoteAddress = new URL(parts[0] + "/wd/hub"); |
There was a problem hiding this comment.
How about letting the user put /wd/hub in the argument? (for forward compatibility)
There was a problem hiding this comment.
Unless I'm mistaken, /wd/hub is the default, and at least I personally haven't seen a different path used since webdriver first shipped (some 10 years ago).
Perhaps default to /wd/hub if no path is provided in the url before the ??
| String[] browserNames = parts[1].split(","); | ||
|
|
||
| for (String browserName : browserNames) { | ||
| DesiredCapabilities capabilities = new DesiredCapabilities(browserName, "", Platform.ANY); |
There was a problem hiding this comment.
Should this possibly read from a JSON file to further configure capabilities? (e.g. allow running against Sauce Labs, etc.)
There was a problem hiding this comment.
Do you have an example of what such a JSON would look like? I see that DesiredCapabilities can take a Map<String, ?>, but the values can be pretty arbitrary.
Proxy looks like the only property in the default impls that actually has any structure, and it accepts a Map as well, so perhaps it is safe to just deserialize JSON to a recursive map.
My main concern then is parameterizing this, passing a path when you just need a browser seems excessive, and passing a path as a string seems messy too (since runStyle can only take a single string, which you then parse yourself, etc).
What do you think of supporting multiple arg patterns, one which roughly matches the current setup (just specify URL and browsers), and another for a config file? Perhaps two concrete types, to allow specifying which format will be used by the runstyle name, but shared implementation.
There was a problem hiding this comment.
It might be possible to have a single run style that supports everything with, say something like
-runStyle "org.gwtproject.junit.RunStyleRemoteWebDriver:http://localhost:4444;firefox,chrome;/path/to/config.json"
(I used ; as a separator, but that could be | or whatever, or ? and # to make it look like a URL (not sure it's a good idea))
If the config file path is absent, then the browser names correspond to "standard" WebDriver browser names, with their default capabilities; otherwise they should match keys in the JSON object (there could be a fallback mechanism to use default capabilities for the browser name, not sure it's a good idea to mix things here). And in case there's a config file path, the browsers' list could be empty to test on all configured browsers (i.e. the browsers' list works as a filter on the JSON keys); that way you can configure many browsers but only actually run tests on a few of them.
But maybe it's still better to have separate run styles 🤷♂️
Disclosure: I don't know Selenium/WebDriver much and never actually used them; only saw several FLOSS projects using WebDriver (from JS) with such configurations as JSON (or JS objects).
|
Updated with my understanding of your suggestions, happy to iterate here more. |
This is a fork of GWT's own SeleniumRunStyle, updated to use WebDriver so that recent selenium updates work with it. The github actions wiring is also updated to start a hub and two nodes, one chrome and one firefox, and the gwt2 tests run once in each browwser.
Still needs docs in the readme to show how to use this locally (either by launching the selenium server directly or through docker.
If we think this is generally a good idea, we should move the class to its own reusable module in case of later changes and to keep documentation all in one place.