Opened on 01/13/2017 at 01:40:48 PM
Closed on 05/05/2017 at 07:38:54 AM
Last modified on 10/08/2019 at 05:50:43 PM
#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): |
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.
Attachments (0)
Change History (21)
comment:1 Changed on 01/13/2017 at 01:44:18 PM by fhd
comment:2 Changed on 01/23/2017 at 04:22:21 PM 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 on 02/13/2017 at 06:35:22 AM by kzar
- Cc trev kzar added
- Description modified (diff)
- Keywords externaldependency added
comment:4 Changed on 03/07/2017 at 06:29:01 PM by hfiguiere
- Cc hfiguiere added
comment:5 Changed on 03/08/2017 at 11:12:07 AM 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 on 04/13/2017 at 02:25:36 PM by kzar
Since headless Chrome is coming and the PhantomJS maintainer is stepping down perhaps we need to rethink this issue?
comment:7 Changed on 04/13/2017 at 02:26:14 PM by kzar
- Ready unset
comment:8 Changed on 04/13/2017 at 02:51:49 PM 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 on 04/13/2017 at 03:43:28 PM 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 on 04/13/2017 at 06:49:28 PM by trev
- Description modified (diff)
- Priority changed from P5 to P3
comment:11 Changed on 04/25/2017 at 10:39:21 AM by trev
- Keywords externaldependency removed
- Ready set
comment:12 Changed on 04/25/2017 at 02:34:09 PM by sergz
- Cc sergz added
comment:13 Changed on 04/26/2017 at 09:55:40 AM by trev
- Blocking 3143 added
- Priority changed from P3 to P2
comment:14 Changed on 04/26/2017 at 11:07:31 AM by trev
- Description modified (diff)
comment:15 Changed on 04/26/2017 at 12:32:21 PM 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.
comment:16 Changed on 04/26/2017 at 01:48:39 PM by trev
- Owner set to trev
comment:17 Changed on 04/27/2017 at 11:47:00 AM by trev
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:18 Changed on 05/05/2017 at 07:32:35 AM by abpbot
A commit referencing this issue has landed:
Issue 4796 - Use a modern JS engine in the browser tests and convert all files to ECMAScript 6
comment:19 Changed on 05/05/2017 at 07:37:01 AM by abpbot
A commit referencing this issue has landed:
Issue 4796 - Use a modern JS engine in the browser tests and convert all files to ECMAScript 6
comment:20 Changed on 05/05/2017 at 07:38:54 AM by trev
- Resolution set to fixed
- Status changed from reviewing to closed
This landed both on master and emscripten branch.
comment:21 Changed on 03/28/2019 at 09:10:18 AM by simmon
spam
How I see it, we have three options for dealing with this:
I'm a big fan of #1, hence I will not mark the issue as Ready at this time.