Issue 6391 - WebDriver: Fix runtime problen with Gecko and Firefox

Issue 6391 - WebDriver: Fix runtime problen with Gecko and Firefox

[hub, 2018-08-17 12:33:01]

Tristan, Dave, this fixes running the test with WebDriver on Chrome. I have
tested this on Debian 9. And TravisCI.

[tlucas, 2018-08-17 13:45:23]

On 2018/08/17 12:33:01, hub wrote:
> Tristan, Dave, this fixes running the test with WebDriver on Chrome. I have
> tested this on Debian 9. And TravisCI.

Confirmed working,
LGTM

[hub, 2018-08-21 14:01:28]

Updated patch. This did pass on TravisCI. We run test in Firefox by default
again.

[tlucas, 2018-08-21 14:06:12]

Still LGTM

(Since Dave is in his holidays, you might want to add someone else to also
review this?)

[Sebastian Noack, 2018-08-22 17:02:04]

https://codereview.adblockplus.org/29858555/diff/29860563/test/runners/chromi...
File test/runners/chromium_process.js (right):

https://codereview.adblockplus.org/29858555/diff/29860563/test/runners/chromi...
test/runners/chromium_process.js:38: // disabling sandboxing. It is needed on
some systems.
Nit: Proper capitalization and punctuation please:

  Disabling sandboxing, as needed on some systems.

You also might want to specify "some systems" a little more.

https://codereview.adblockplus.org/29858555/diff/29860563/test/runners/firefo...
File test/runners/firefox_process.js (right):

https://codereview.adblockplus.org/29858555/diff/29860563/test/runners/firefo...
test/runners/firefox_process.js:27: const FIREFOX_VERSION = "57.0";
From the issue description it isn't clear to me why this change bumps up the
Firefox version. Can you elaborate?

FWIW, at some point we'd would want to run the tests on the oldest and newest
supported version, out of scope for now, but is there a chance that this will
ever work with Firefox 51 (current oldest supported version)?

[hub, 2018-08-22 17:36:56]

updated patch

https://codereview.adblockplus.org/29858555/diff/29860563/test/runners/chromi...
File test/runners/chromium_process.js (right):

https://codereview.adblockplus.org/29858555/diff/29860563/test/runners/chromi...
test/runners/chromium_process.js:38: // disabling sandboxing. It is needed on
some systems.
On 2018/08/22 17:02:04, Sebastian Noack wrote:
> Nit: Proper capitalization and punctuation please:
> 
>   Disabling sandboxing, as needed on some systems.
> 
> You also might want to specify "some systems" a little more.

Done.

https://codereview.adblockplus.org/29858555/diff/29860563/test/runners/firefo...
File test/runners/firefox_process.js (right):

https://codereview.adblockplus.org/29858555/diff/29860563/test/runners/firefo...
test/runners/firefox_process.js:27: const FIREFOX_VERSION = "57.0";
On 2018/08/22 17:02:04, Sebastian Noack wrote:
> From the issue description it isn't clear to me why this change bumps up the
> Firefox version. Can you elaborate?
> 
> FWIW, at some point we'd would want to run the tests on the oldest and newest
> supported version, out of scope for now, but is there a chance that this will
> ever work with Firefox 51 (current oldest supported version)?

On *some* systems (Debian 9, TravisCI, even some macOS) Firefox 56 fail. Bumping
to 57 works. That's what Tristan did for
https://codereview.adblockplus.org/29860555/

BTW https://adblockplus.org/en/requirements show Firefox 60 as the minimum,
which contradict the manifest that say 51.

WebDriver support is buggy in older releases.

I'll add a comment to explain.

[Sebastian Noack, 2018-08-22 18:29:05]

LGTM

https://codereview.adblockplus.org/29858555/diff/29860563/test/runners/firefo...
File test/runners/firefox_process.js (right):

https://codereview.adblockplus.org/29858555/diff/29860563/test/runners/firefo...
test/runners/firefox_process.js:27: const FIREFOX_VERSION = "57.0";
On 2018/08/22 17:36:55, hub wrote:
> On 2018/08/22 17:02:04, Sebastian Noack wrote:
> > From the issue description it isn't clear to me why this change bumps up the
> > Firefox version. Can you elaborate?
> > 
> > FWIW, at some point we'd would want to run the tests on the oldest and
newest
> > supported version, out of scope for now, but is there a chance that this
will
> > ever work with Firefox 51 (current oldest supported version)?
> 
> On *some* systems (Debian 9, TravisCI, even some macOS) Firefox 56 fail.
Bumping
> to 57 works. That's what Tristan did for
> https://codereview.adblockplus.org/29860555/
> 
> BTW https://adblockplus.org/en/requirements show Firefox 60 as the minimum,
> which contradict the manifest that say 51.
> 
> WebDriver support is buggy in older releases.
> 
> I'll add a comment to explain.

There are two level of support. We discourage users to not use anything older
than the previous release version of their browser, and this is what the
requirements page reflects (it is auto generated based on real-time release
information of those browsers). However, technically we support a wider range of
browser versions (sometimes exposing limitations of older browser versions but
with graceful degradation), as reflected in the metadata.* files. This is also
the minimum version our testers do manual testing on, and ideally our automated
tests would run as well on the oldest and newest support browser version. It's
not a priority for now, but we might have to revisit this.

[hub, 2018-08-22 19:05:35]

On 2018/08/22 18:29:05, Sebastian Noack wrote:

> There are two level of support. We discourage users to not use anything older
> than the previous release version of their browser, and this is what the
> requirements page reflects (it is auto generated based on real-time release
> information of those browsers). However, technically we support a wider range
of
> browser versions (sometimes exposing limitations of older browser versions but
> with graceful degradation), as reflected in the metadata.* files.

Make sense.

> This is also
> the minimum version our testers do manual testing on, and ideally our
automated
> tests would run as well on the oldest and newest support browser version. It's
> not a priority for now, but we might have to revisit this.

I agree, but I'm not sure there is an easy to work around (or fix) all of these
issues.