Repository: GoogleChrome/lighthouse
Pull request #9877 (2019-10-23 06:33:56)

tests(viewer): don't override puppeteer's chromium
This test is currently failing in master.

The viewer's puppeteer test doesn't work with Chrome 78, and neither does the latest puppeteer (1.20.0).

Puppeteer ships with a specific Chromium revision, and is only meant to work with that, so we shouldn't override it.


patrickhulce: that didn't seem to do the trick yet

viewer tests failed with

>  Chromium revision is not downloaded. Run "npm install" or "yarn install"
connorjclark: MaybeTravis doesn't clear env variables between runs?
brendankenny: > Puppeteer ships with a specific Chromium revision, and is only meant to work with that, so we shouldn't override it.

this was partly to avoid redownloading Chromium every time (it may have been skipping `node_modules/` caching since pptr downloads it itself on install?) and making the travis runs even slower. Ideally someone in the world has figured that out since #4640?

Otherwise, is there something specific that broke with m78 and is it something we can get fixed upstream? pptr is going to have to update its version of Chromium at some point... 
connorjclark: > Otherwise, is there something specific that broke with m78 and is it something we can get fixed upstream? pptr is going to have to update its version of Chromium at some point...

@mathiasbynens might know. [We could bisect it](https://docs.google.com/document/d/1GXpxna_EGRC8TkN6tLseuabfaxQLlWP2pQuQ73T-lUc/edit?ts=5daec46c).
connorjclark: ```
tools/bisect-builds.py --use-local-cache -a linux64 -g 674921 -b 693954 --not-interactive -c 'bash -c "cd ~/code/lighthouse && CHROME_PATH=%p yarn jest viewer -t PSI"' --verify-range
```

Can't get the bad revision (`693954` is current stable) to fail, even though using the stable channel binary directly _does_ fail. Weird.
connorjclark: For travis, had to `unset` the puppeteer env variable. I guess it stuck around from previous runs.
connorjclark: > a major bummer to our build times but a necessary evil to be able to keep merging I suppose

We could consider removing the other Chrome install, and just use Puppeteer's instead. We would have to manage updating Puppeteer on a regular basis. note: I don't love this idea.
connorjclark: One unexpected thing is that https://omahaproxy.appspot.com/ gives me `78.0.3904.70` as the stable version and `693954` as its base revision, but when the bisect script downloads `693954` it's actually version `78.0.3904.0`.

```
tools/bisect-builds.py --use-local-cache -a mac -g 674921 -b 693954 --not-interactive -c '%p --version' --verify-range
```
![image](https://user-images.githubusercontent.com/4071474/67513896-a35b5880-f650-11e9-81e7-ebe04c9ccd81.png)

connorjclark: > does it work with the new 2.0.0 pptr release?

I don't think it will, since the issue is with Chromium and not Puppeteer. Let's see: https://github.com/GoogleChrome/lighthouse/pull/9886

Patch:
diff --git a/.travis.yml b/.travis.yml
index f5cf3ae9a..2cbbbf2b5 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -17,10 +17,10 @@ cache:
 install:
   # Ensure we have the latest Chrome stable.
   - google-chrome-stable --version
-  # if our e2e tests fail in the future it might be that we are not compatible
-  # with the latest puppeteer api so we probably need to run on chromimum
-  # @see https://github.com/GoogleChrome/lighthouse/pull/4640/files#r171425004
-  - export PUPPETEER_SKIP_CHROMIUM_DOWNLOAD=1
+  # We used to set PUPPETEER_SKIP_CHROMIUM_DOWNLOAD, but we don't want to anymore.
+  # Explicitly unset b/c environment variables seem to linger in travis.
+  # see https://github.com/GoogleChrome/lighthouse/pull/9877#discussion_r338389765
+  - unset PUPPETEER_SKIP_CHROMIUM_DOWNLOAD
   - yarn --frozen-lockfile
 before_script:
   # see comment above about puppeteer


Test patch:
diff --git a/lighthouse-viewer/test/viewer-test-pptr.js b/lighthouse-viewer/test/viewer-test-pptr.js
index 5781fd63e..f504c3609 100644
--- a/lighthouse-viewer/test/viewer-test-pptr.js
+++ b/lighthouse-viewer/test/viewer-test-pptr.js
@@ -62,7 +62,6 @@ describe('Lighthouse Viewer', () => {
     // start puppeteer
     browser = await puppeteer.launch({
       headless: true,
-      executablePath: process.env.CHROME_PATH,
     });
     viewerPage = await browser.newPage();
     viewerPage.on('pageerror', pageError => pageErrors.push(pageError));