Opened 3 years ago

Closed 3 years ago

Last modified 5 weeks ago

#4796 closed change (fixed)

Use a modern JS engine in the browser tests

Reported by: fhd Assignee: trev
Priority: P2 Milestone:
Module: Core Keywords:
Cc: trev, kzar, hfiguiere, sergz Blocked By:
Blocking: #3143 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29423569/

Description (last modified by trev)

Background

As of #4726, we have automated browser tests in Core. We are using PhantomJS 2.1.7 for these, which is based on a version of WebKit that doesn't support ECMAScript 2015 features (such as let). With PhantomJS being unmaintained, we shouldn't expect updates.

What to change

  • Replace PhantomJS by headless Chromium (headless mode available starting with Chromium 59).
  • Have test runner download a platform-specific Chromium build from https://commondatastorage.googleapis.com/chromium-browser-snapshots/index.html.
  • We need to know which snapshot revision to download. Headless landed in Chrome 59, and Chrome 59.0.3071.28 was branched off revision 464641. There are no snapshots for this revision however, but we can simply grab a more recent snapshot which exists for all platforms - that's 467222:
  • Chromium should be downloaded and unpacked under test/chromium/chromium-{platform}-{revision} - only if this directory doesn't exist yet. Having Chromium revision in directory name allows us to change to a different revision later. The test/chromium directory should be added to .hgignore/.gitignore.
  • Test runner should run tests by instrumenting this Chromium instance via chrome-remote-interface package.
  • Remove the following files from adblockpluscore/.eslintignore and update those files so that linting passes: chrome/content/elemHideEmulation.js, lib/common.js and test/browser/elemHideEmulation.js.
  • Remove the comments that mention this issue.

Change History (21)

comment:1 Changed 3 years ago by fhd

How I see it, we have three options for dealing with this:

  1. We wait. PhantomJS 2.5 is currently in beta and has full ES2015 support. Shouldn't take that much longer for them to release this now.
  1. We switch to a different headless browser that has better ES2015 support. Based on some quick research I haven't seen anything promising, but there could be something.
  1. We transpile all the code used in browser tests to ES2015, e.g. using Babel.

I'm a big fan of #1, hence I will not mark the issue as Ready at this time.

comment:2 Changed 3 years ago by trev

  • Description modified (diff)
  • Priority changed from Unknown to P5
  • Ready set

I agree, just waiting for PhantomJS 2.5 looks like the best solution to me.

comment:3 Changed 3 years ago by kzar

  • Cc trev kzar added
  • Description modified (diff)
  • Keywords externaldependency added

comment:4 Changed 3 years ago by hfiguiere

  • Cc hfiguiere added

comment:5 Changed 3 years ago by trev

For reference, https://bitbucket.org/ariya/phantomjs/downloads/ has PhantomJS 2.5.0 betas which could theoretically be used. You can force a particular download location by running npm install phantomjs2 --phantomjs_downloadurl=... which might even work. On Linux however it doesn't, first because the packages are compressed with gzip rather than bzip2 that the installer expects. More importantly however, these aren't static builds so the Xenial build expects libpng 1.2 - my system has only libpng 1.6 to offer. On Windows or OS X it might work however.

comment:6 Changed 3 years ago by kzar

Since headless Chrome is coming and the PhantomJS maintainer is stepping down perhaps we need to rethink this issue?

comment:7 Changed 3 years ago by kzar

  • Ready unset

comment:8 Changed 3 years ago by trev

That's quite a pity. I don't think that we should require a system-wide Chrome/Chromium installation for the browser tests to work. I think that we will need to choose some Chromium revision and download binaries from https://commondatastorage.googleapis.com/chromium-browser-snapshots/, then unpack them within the repository. I don't see any npm package already doing that, so we will have to write that code ourselves. Also, using remote debugging isn't as straightforward as PhantomJS but I think there are packages for that already.

comment:9 Changed 3 years ago by fhd

Bah :/ Yeah I guess we have to move away from PhantomJS then, the thread doesn't leave me with any hope that 2.5 is ever going to be released.

comment:10 Changed 3 years ago by trev

  • Description modified (diff)
  • Priority changed from P5 to P3

comment:11 Changed 3 years ago by trev

  • Keywords externaldependency removed
  • Ready set

comment:12 Changed 3 years ago by sergz

  • Cc sergz added

comment:13 Changed 3 years ago by trev

  • Blocking 3143 added
  • Priority changed from P3 to P2

comment:14 Changed 3 years ago by trev

  • Description modified (diff)

comment:15 Changed 3 years ago by trev

  • Description modified (diff)

At least on Linux running Chrome headless (tried two different snapshots) complains about missing libosmesa.so - the solution from https://stackoverflow.com/a/39060739/785541 fixes this. There is also an NSS error however.

Edit: As noted under https://stackoverflow.com/a/42811360/785541, the simpler solution is to pass --disable-gpu flag.

Last edited 3 years ago by trev (previous) (diff)

comment:16 Changed 3 years ago by trev

  • Owner set to trev

comment:17 Changed 3 years ago by trev

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:20 Changed 3 years ago by trev

  • Resolution set to fixed
  • Status changed from reviewing to closed

This landed both on master and emscripten branch.

comment:21 Changed 8 months ago by simmon

spam

Last edited 5 weeks ago by kzar (previous) (diff)
Note: See TracTickets for help on using tickets.