Skip to content

build(docs-infra): use latest Chrome (75) to reliably pass PWA score tests on localhost - #29921

Closed
gkalpak wants to merge 3 commits into
angular:masterfrom
gkalpak:build-aio-fix-pwa-test
Closed

build(docs-infra): use latest Chrome (75) to reliably pass PWA score tests on localhost#29921
gkalpak wants to merge 3 commits into
angular:masterfrom
gkalpak:build-aio-fix-pwa-test

Conversation

@gkalpak

@gkalpak gkalpak commented Apr 16, 2019

Copy link
Copy Markdown
Member

Fixes #29910.

@gkalpak gkalpak added area: build & ci Related the build and CI infrastructure of the project action: discuss comp: docs-infra target: patch This PR is targeted for the next patch release labels Apr 16, 2019
@gkalpak
gkalpak requested review from a team April 16, 2019 10:10
@ngbot ngbot Bot added this to the needsTriage milestone Apr 16, 2019
@mary-poppins

mary-poppins commented Apr 16, 2019

Copy link
Copy Markdown

You can preview c163aa5 at https://pr29921-c163aa5.ngbuilds.io/.

devversion
devversion reviewed Apr 16, 2019
View reviewed changes
Comment thread aio/scripts/test-pwa-score.js
const printer = require('lighthouse/lighthouse-cli/printer');
const config = require('lighthouse/lighthouse-core/config/default-config.js');
const logger = require('lighthouse-logger');
const puppeteer = require('puppeteer');

@devversion devversion Apr 16, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is still up for discuss, but just so that we don't forget then: Looks like the chrome-launcher dev dependency is no longer used anywhere then. So we can remove that right?

filipesilva
filipesilva approved these changes Apr 16, 2019
View reviewed changes
Comment thread aio/scripts/test-pwa-score.js
}

function launchChromeAndRunLighthouse(url, flags, config) {
return chromeLauncher.launch(CHROME_LAUNCH_OPTS).then(chrome => {

@filipesilva filipesilva Apr 16, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const chromeLauncher = require('chrome-launcher'); can be removed.

@gkalpak gkalpak Apr 16, 2019

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch 👍
(I also removed from devDependencies, because we are not using it elsewhere.)

@mary-poppins

mary-poppins commented Apr 16, 2019

Copy link
Copy Markdown

You can preview 3b87058 at https://pr29921-3b87058.ngbuilds.io/.

@gkalpak

gkalpak commented Apr 16, 2019 •
edited
Loading

Copy link
Copy Markdown
Member Author

It seems I spoke too soon: This still does not fix the problem entirely 😞

While I was logged in a CircleCI VM, the PWA tests passed 30 times in a row with no failure.
But running several jobs on CI, I still got 6/20 failures 😢: 285474, 285475, 285476, 285478, 285479, 285482

This was referenced Apr 16, 2019
Closed
Closed
@gkalpak

gkalpak commented Apr 16, 2019 •
edited
Loading

Copy link
Copy Markdown
Member Author

Comment moved to #29910 (comment).

@gkalpak

gkalpak commented Apr 17, 2019

Copy link
Copy Markdown
Member Author

Closing in favor of #29953.

@gkalpak gkalpak closed this Apr 17, 2019
@gkalpak
gkalpak deleted the build-aio-fix-pwa-test branch April 17, 2019 09:25
@angular-automatic-lock-bot

angular-automatic-lock-bot Bot commented Sep 14, 2019

Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Sep 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@devversion devversion devversion left review comments
+1 more reviewer
@filipesilva filipesilva filipesilva approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

action: discuss area: build & ci Related the build and CI infrastructure of the project cla: yes target: patch This PR is targeted for the next patch release

Projects

None yet

Milestone

needsTriage

Development

Successfully merging this pull request may close these issues.

ci(docs-infra): test_aio/test_aio_local jobs occasionally fail due to low PWA score

5 participants

@gkalpak @mary-poppins @filipesilva @devversion @googlebot