[Harri]

This PR adds headless Firefox and Chrome support for Open Browser keyword. It adds two browsers; Headless Firefox and Headless Google Chrome with couple aliases.

This allows us to write tests like this:

*** Settings ***
Library    SeleniumLibrary

*** Test Cases ***
Firefox
    Open Browser    http://robotframework.org/    headlessfirefox
    Capture Page Screenshot
    [Teardown]    Close All Browsers

Chrome
    Open Browser    http://robotframework.org/     headlesschrome
    Capture Page Screenshot
    [Teardown]    Close All Browsers

[Harri]

This change depends on changes made to Selenium 3.8.0.

[aaltat]

I can update the Travis configuration to have Selenium 3.8.0. When that is done, you can rebase with the master. Then some questions

1. Did you run the existing test with headless Chrome and Firefox. If you did not run, you should run. If you did run, did all the test pass?
2. Should we write separate test for the feature. This depends greatly on the answer on question number one.
3. If good set for the test work, we could configure Travis to run with headless browser too. But that is for me to do.

[pekkaklarck]

Awesome! Don't have time to go through or test the changes, but some general comments/questions:

1. This works also with remote, right?

2. Depending on latest Selenium versions ought to be fine as long as it's documented. Headless stuff anyway requires latest browser drivers as well. Related to this, the docs should contain a note which SL version supports headless.

3. Supporting all the aliases with headless prefix like headlessgc is a bit annoying. I'd rather use just Headless Chrome (case- and space-insensitively) or add separate headless=False argument. What do others think about this?

4. On a quick look the changes look really simple and this would be a nice addition. Should we consider adding it already in SL 3.0.1 (needed ASAP due to at lest one regression) or should we wait until SL 3.1 (will take longer as we want also other features in)?

[pekkaklarck]

I don't think this needs separate tests. We just need to run all tests with supported headless browsers on CI. If there are some tests that don't pass with headless, they can be excluded with tags like we already do with other browser incompatibilities.

[Harri]

With Chrome tests stats look like this:

Statistics by Tag     
Total                              Pass    Fail    Elapsed    Pass / Fail
knownissuechrome (non-critical)    2       0       2          00:00:00    
assertions                         42      42      0          00:00:12    
element count                      14      14      0          00:00:01    
Known Issue Firefox                4       4       0          00:00:01    
Known Issue Internet Explorer      135     135     0          00:01:39    
Known Issue Safari                 48      46      2          00:00:20    
Regression                         356     354     2          00:03:31  

Which I guess is considered that all tests passed. Firefox tests get stuck on Set Focus To Element.

Headless Chrome stats look like this:

knownissueheadlesschrome (non-critical)    0   0   0   00:00:00    
assertions                                 42  42  0   00:00:12    
element count                              14  14  0   00:00:01    
Known Issue Chrome                         2   2   0   00:00:00    
Known Issue Firefox                        4   4   0   00:00:01    
Known Issue Internet Explorer              135 134 1   00:01:27    
Known Issue Safari                         48  47  1   00:00:16    
Regression                                 356 353 3   00:02:57  

One webdriver test fails because headlesschrome is not expected browser name. Marionette seems to die every time with Firefox Headless at some point of the test run. Or at least that's what I think SessionNotCreatedException: Message: Tried to run command without establishing a connection means.

Yes, also remote works.

My thinking was that using different browser names for these makes it really simple to start using headless browsers in existing test setups just by issuing -variable BROWSER:headlessfirefox and things would work for most scenarios. Having this as a extra arg for Open Browser would in some cases be nicer, after all, those are not really different browsers.

[pekkaklarck]

Test run is considered OK if all critical tests pass. If there are critical failures, need to investigate why they occur. If they cannot be fixed, they can be ignored by using tags.

--variable BROWSER:HeadlessFirefox is a very good point. Obviously could also do --variable BROWSER:Firefox --variable HEADLESS:True but that's a bit more typing. One benefit of having headless as a separate argument is that if enabling the headless mode is in the future supported also by other browsers by using options.set_headless() like Chrome and Firefox apparently do, we didn't need to change the code to support e.g. headless Edge. This would require some changes to the current logic to create drivers, though, but shouldn't be too complicated.

Another approach is supporting Headless prefix with all browsers in addition to (or instead of) explicit headless argument:

if browser.upper().startswith('HEADLESS'):
    browser = browser[len('HEADLESS'):]
    headless = True

[aaltat]

It seems that update to Selenium 3.8.0 does make two of our acceptance tests fail. I need to take a closer look where the error is and fix it. See #1012 for details about Selenium 3.8.0 update.

[Harri]

Adding headless argument for Open Browser is bit tricky. For naturally headless browsers it does not make much sense and for rest it felt weird doing since the implementation is not really there. Anyway, here's one suggestion Harri@8222852

I agree that it would be really nice to build the support for future headless browsers right away, it just feels weird. For example how should PhantomJS react to headless=True/False? What should be built for IE, which most probably will not ever get headless support? For me it feels cleanest to only support existing features. Also it is convenient to handle headless browsers as different browsers, since most tooling already allow specifying the browsers, not adding artificial arguments. Yes, it probably means extra work if and when Egde, Android and Safari get headless support.

[aaltat]

Selenium 3.8.0 raises different type of exception with JavaScript and #1013 fixes this problem.

[pekkaklarck]

The referred implementation at Harri/SeleniumLibrary@8222852 is indeed somewhat complicated and there's lot of repetition. A lot simpler solution would be adding headless=False option to _generic_make_driver and in there do:

if is_truthy(headless):
    options.set_headless()

All functions calling this should obviously then pass options to it. If that doesn't make sense with all browsers, the above code could be enhanced to validated that options is not None.

[Harri]

https://github.com/SeleniumHQ/selenium/blob/master/py/selenium/webdriver/__init__.py has [browser]Options imported only for Firefox, Chrome and WebKitGTK. So in addition to checking whether options is None, there also should be something in the spirit of

try:
    options = webdriver.EdgeOptions()
except AttributeError:
    raise AttributeError('Seleniums Webdriver does not expose EdgeOptions')

And if/when in the future EdgeOptions() is a things, it will most probably be named EgdeOptions() but that's not sure.

[pekkaklarck]

Should also mention that they are new in SeleniumLibrary 3.x.x, depending on which version this is going to be added.

[aaltat]

Lets put this to the 3.1.0 release.

[pekkaklarck]

Calling _make_chrome here like _make_ff is called by _make_headless_ff above would probably be a good idea especially for consistency reasons.

[pekkaklarck]

Trying to make the code too future-proof isn't a good idea. Better to just concentrate on Chrome and Firefox that actually work. I took a bit more closer look at the changes and they are very simple. Added few line notes and have these general questions/comments as well:

1. Which version are we going to add this? Changes are so simple and headless testing so hot topic (there are countless questions about that on Slack and ugly code using Create WebDriver to enable it) that I'd like to add it already in SL 3.0.1. I know you shouldn't add new functionality in such a minor release, but I also know that practicality beats purity.

2. I don't think aliases Headless Ff, Headless GC and Headless Google Chrome make sense. If you need to write Headless, you can as well write Firefox and Chrome as well, and the last one is just overly long. (I'd be actually willing to drop Ff, GC and Google Chrome aliases altogether, but that's probably not worth the effort due to backwards compatibility reasons. Also, I think the official Firefox acronym is Fx, not Ff, and I doubt anyone really uses GC when referring to Chrome. The only browser commonly referred with an acronym is IE. I doubt we need to worry about it getting headless support so that we needed to decided should we support Headless IE or Headless Internet Exporer. =)

3. I haven't actually tested the changes. Do you @aaltat have time for that? If there are problems, we need to think can this be added already in 3.0.1 which is otherwise needed soon.

[aaltat]

1. Having this in the 3.0.1 is really tempting l. Although I also agree that we should not do it... I need to also take a closer look at the code and decide what to do. I would really like to get the 3.0.1 out, because of the Change between 1.8.0 and 3.0.0 when using Wait Until Element Is Not Visible. In 3.0.0 element is consider visible if the element does not exist in the DOM. #1008 Other plan would be release 3.0.1 really soon and let this be released in 3.1.0 as a small but important increment. And release the 3.1.0 quite soon after 3.0.1 and with minimal content.

2. Having separate headless or similar argument is better way to also for me.

3. I haven't have time to look at the changes with real thought. In any case I would like to change the CI to run all test with headless mode too. And if there are failing tests, we need to label them accordingly. Which reminds me that when we go with the new argument direction and if there are failing tests, we need to improve the runner to correctly mark them as noncritical.

[pekkaklarck]

I've started to thing Headless Chrome/Firefox as a browser name is better than special headless=False argument. This avoids problem handling cases like Open Browser ${URL} browser=IE headless=True and thus keeps the code simple, and it is also directly compatible with start-up scripts accepting the browser name as a configuration value. For example, our own test runner needed to get a separate headless argument too.

[aaltat]

The 3.0.1 is out. But this feature is requested often within the users of the SeleniumLibrary. We could make next a 3.1.0 which contains this feature and possible some other enhancement if they are ready when this PR is merged to master.

Created issue #1014 to cover this PR.

[aaltat]

I did test the PR with local browser in Ubuntu.

1. All test do work with headless chrome.
2. Headless Firefox is more problematic. In the middle of the test run, connection to the Firefox is lost and this is seen in the logs: WebDriverException: Message: Failed to decode response from marionette. Because our acceptance test do not open browser that often, more than half of the test do fail. But this could be also a problem with the Firefox (57.0.1 is available in Ubuntu) or in the geckodriver.

Todo:

1. I need test this in my windows machine, which has Firefox 57.0.2 available
2. I also need to try out the remote browser with SauceLabs.

But in any case, I am loving the feature because now I can run acceptance test locally and same time do something else too.

[aaltat]

It is true that doing this from command line: robot --variable BROWSER:headless_chrome ... is easier than doing this: robot --variable BROWSER:chrome --variable HEADLESS: True .... Although the difference is not big at the command line, there is a significant difference if one has a custom runner, like the library test runner or what I have in day job. Perhaps this justify that having a single argument is better. Let's go with single argument.

[jimpriest]

I'm just getting started using Robot but I'd vote for one argument as well. Then I can simply change one line in my test / automation code vs having to add things for a new 'headless' argument.

${BROWSER} chrome
to
${BROWSER} chrome_headless

[Harri]

Now only "headless chrome" and "headless firefox" are valid headless browser names, so ff, gc and google chrome were dropped. If those need to be added back, let me know. Sorry for the delay. It was weekend and surly a good time to catch a flu.

[aaltat]

No problems, the 3.0.1 is out to fix the immediate problem in the 3.0.0 release and this is targeted to the 3.1.0 release. The 3.1.0 is planned to be out before Christmas.

I did read the code from a mobile screen, but I need to take a closer look from a bigger screen, because I miss stuff from the smaller screen.

[aaltat]

I think we need to support only for headlessfirefox and headlesschrome.

[aaltat]

I only had one commit and then this should be ready to merged to master.

[Harri]

I don't know why the tests failed. The error message is "No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself.". On my local machine things are ok and the latest diff could not cause that kind of issue where no output was generated.

[aaltat]

I think the Firefox got stuck at something and then Travis will kill the job after 10mins of the inactivity. I did restart the FF job and let's see how it goes. Other reason might be that Travis has updated their environments, yesterday, and it might have broken something.

[aaltat]

Looks like this has something to do with the Travis update or it might be that Firefox is behaving badly. I will try to figure out what is making the Travis to fail the tests Firefox.

[aaltat]

I changed the Travis configuration to run with previous version of the Trusty image and then tests started to pass. Because failure in Travis this is not related to this PR, I am merging the code the master.