ARQ-2037 Added a support of additional browser capabilities for Chrome #65

MatousJobanek · 2016-09-02T10:56:10Z

With this commit, it should be possible to set all possible chrome options into
a chromeDriver through arquillian.xml file. All the options you can find
here: https://sites.google.com/a/chromium.org/chromedriver/capabilities
All the options are set into ChromeOptions class:
https://seleniumhq.github.io/selenium/docs/api/java/org/openqa/selenium/chrome/ChromeOptions.html
which means that the parameter names tightly depend on names of the set/add
methods.
It is expected that the name of a parameter consist of:
"chrome" + (name of set/add method of ChromeOption class without first three chars)
whole string in camelcase.
eg for option args there should be:
<property name="chromeArguments">--your-cool --arguments</property>
If the value can be an array or list of strings/files, then specify all of
them in one string separated by space. (This is also applied for
extensions as well as for encoded extensions.)
In the case of experimental options it is a little bit different. As there
is expected a set of dictionaries, the most suitable way is using JSON
format - eg:
<property name="chromeExperimentalOption">{"perfLoggingPrefs": { "traceCategories": ",blink.console,disabled-by-default-devtools.timeline,benchmark" }, "prefs": {"download.default_directory": "/usr/local/path/to/download/directory"} }</property>
(can be in multiline format )

If you are still struglling with passing required options through
arquillian.xml, you can use a parameter chromePrintOptions with a value
true:
<property name="chromePrintOptions">true</property>
This ensures that Drone prints whole content of ChromeOptions in a JSON
format on a standard output.

This change is

With this, it should be possible to set all possible chrome options into a chromeDriver through arquillian.xml file. All the options you can find here: https://sites.google.com/a/chromium.org/chromedriver/capabilities All the options are set into ChromeOptions class: https://seleniumhq.github.io/selenium/docs/api/java/org/openqa/selenium/chrome/ChromeOptions.html which means that the parameter names tightly depend on names of the set/add methods. It is expected that the name of a parameter consist of: "chrome" + (name of set/add method of ChromeOption class without first three chars) whole string in camelcase. eg for option args there should be: <property name="chromeArguments">--your-cool --arguments</property> If the value can be an array or list of strings/files, then specify all of them in one string separated by space. (This is also applied for extensions as well as for encoded extensions.) In the case of experimental options it is a little bit different. As there is expected a set of dictionaries, the most suitable way is using JSON format - eg: <property name="chromeExperimentalOption">{"perfLoggingPrefs": { "traceCategories": ",blink.console,disabled-by-default-devtools.timeline,benchmark" }, "prefs": {"download.default_directory": "/usr/local/path/to/download/directory"} }</property> (can be in multiline format ) If you are still struglling with passing required options through arquillian.xml, you can use a parameter chromePrintOptions with a value "true": <property name="chromePrintOptions">true</property> This ensures that Drone prints whole content of ChromeOptions in a JSON format on a standard output.

MatousJobanek · 2016-09-02T12:02:10Z

This is a fix for #64

TadeasKriz · 2016-09-04T12:32:32Z

Review status: 0 of 7 files reviewed at latest revision, 9 unresolved discussions.

drone-webdriver/src/main/java/org/jboss/arquillian/drone/webdriver/factory/CapabilitiesMapper.java, line 173 [r1] (raw file):

    private static <T> T booleanNumberStringOrFile(Class<T> clazz, String value) {
        return (T) (!String.class.equals(clazz) ?

This method is really cluttered and confusing. Wouldn't it be better to not use nesting/ternary operators? I.e.:

if (File.class.equals(clazz)) {
    return new File(value)
}
if (Integer.class.equals(clazz)) {
    return Integer.valueOf(value)
}

// the same for other types

return null

drone-webdriver/src/main/java/org/jboss/arquillian/drone/webdriver/factory/ChromeDriverFactory.java, line 116 [r1] (raw file):

        if (Validate.nonEmpty(printChromeOptions) && Boolean.valueOf(printChromeOptions.trim())){
            try {
                System.out.println("======== Chrome options =========");

Shouldn't some logger be used instead of writing directly to out?

drone-webdriver/src/main/java/org/jboss/arquillian/drone/webdriver/factory/ChromeDriverFactory.java, line 120 [r1] (raw file):

                System.out.println("===== End of Chrome options =====");
            } catch (IOException e) {
                e.printStackTrace();

Who is throwing the IOException here? Is it really possible to ignore it and is it wise to print the stack trace?

drone-webdriver/src/main/java/org/jboss/arquillian/drone/webdriver/utils/StringUtils.java, line 58 [r1] (raw file):

    }

    public static String trimMultiline(String toTrim) {

This whole method looks like it was taken from some disassembled Java code.

drone-webdriver/src/main/java/org/jboss/arquillian/drone/webdriver/utils/StringUtils.java, line 60 [r1] (raw file):

    public static String trimMultiline(String toTrim) {
        StringBuilder builder = new StringBuilder(toTrim.length());
        String[] var2 = toTrim.split("\\s+");

More descriptive variable names would help in this method.

drone-webdriver/src/main/java/org/jboss/arquillian/drone/webdriver/utils/StringUtils.java, line 60 [r1] (raw file):

    public static String trimMultiline(String toTrim) {
        StringBuilder builder = new StringBuilder(toTrim.length());
        String[] var2 = toTrim.split("\\s+");

Correct me if I'm wrong, but doesn't this split by every whitespace, removing the need to do .trim()? Also I though this method is trimming spaces on every line. So if it trims each space-separated part, I suggest renaming the method.

drone-webdriver/src/main/java/org/jboss/arquillian/drone/webdriver/utils/StringUtils.java, line 63 [r1] (raw file):

        int var3 = var2.length;

        for(int var4 = 0; var4 < var3; ++var4) {

Why not iterate using for (String token : var2) { .. }?

drone-webdriver/src/main/java/org/jboss/arquillian/drone/webdriver/utils/StringUtils.java, line 65 [r1] (raw file):

        for(int var4 = 0; var4 < var3; ++var4) {
            String token = var2[var4];
            if(token != null && !"".equals(token.trim())) {

I'm not sure about the performance of .trim method, but it might be better to extract it to a variable.

drone-webdriver/src/main/java/org/jboss/arquillian/drone/webdriver/utils/StringUtils.java, line 70 [r1] (raw file):

        }

        return builder.toString().trim();

Isn't this trim unnecessary?

Comments from Reviewable

MatousJobanek · 2016-09-05T05:31:33Z

Review status: 0 of 7 files reviewed at latest revision, 9 unresolved discussions.

drone-webdriver/src/main/java/org/jboss/arquillian/drone/webdriver/factory/ChromeDriverFactory.java, line 120 [r1] (raw file):