Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: upgrade to lighthouse 9.3.0 #726

Merged
merged 22 commits into from
Feb 1, 2022
Merged

feat: upgrade to lighthouse 9.3.0 #726

merged 22 commits into from
Feb 1, 2022

Conversation

connorjclark
Copy link
Collaborator

@connorjclark connorjclark commented Nov 16, 2021

include in commit message:

BREAKING CHANGE: upgrade to lighthouse v9.3.0

BREAKING CHANGE: upgrade to lighthouse v9.0.0
@connorjclark
Copy link
Collaborator Author

WIP, tests aren't running / snapshots aren't updating as expected locally for me.

@nicksrandall
Copy link
Contributor

FYI, lighthouse 9.1 was released so it would be nice if we could get this PR all the way to the latest version

@theoludwig
Copy link
Contributor

There is now even lighthouse v9.2.0, is there is anything blocking this PR?
Is the failure of the tests are not fixable in CI?

@connorjclark connorjclark changed the title feat: upgrade to lighthouse 9.0 feat: upgrade to lighthouse 9.2.0 Jan 11, 2022
@connorjclark
Copy link
Collaborator Author

seems the node-gyp errors originate from a parcel v1 dep. parcel-bundler/parcel#5294 (comment)

@chrillep
Copy link

chrillep commented Jan 20, 2022

seems the node-gyp errors originate from a parcel v1 dep. parcel-bundler/parcel#5294 (comment)

parcel-bundler/parcel#5294 (comment) 🤷‍♂️

@@ -409,15 +399,6 @@ describe('Lighthouse CI CLI', () => {
found: XXXX
all values: XXXX, XXXX


X performance-budget.script.size failure for maxNumericValue assertion
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@connorjclark connorjclark changed the title feat: upgrade to lighthouse 9.2.0 feat: upgrade to lighthouse 9.3.0 Jan 31, 2022
@@ -53,6 +57,7 @@ const auditPairs62 = createAuditPairs(lhr62A, lhr62B);
const auditPairs641 = createAuditPairs(lhr641A, lhr641B);
const auditPairs700 = createAuditPairs(lhr700A, lhr700B);
const auditPairs800 = createAuditPairs(lhr800A, lhr800B);
const auditPairs930 = createAuditPairs(lhr930A, lhr930B);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

image

https://github.com/GoogleChrome/lighthouse-ci/blob/main/packages/server/src/ui/routes/build-view/audit-detail/audit-detail-pane.stories.jsx#L166

Not sure why the audit pairs for the other versions don't have this problem, but for the 9.3.0 LHR I'm getting a diff at screenshot-thumbnails, as you'd expect. But the viz is non existent so we just print the JSON.

Not sure what the best approach to deal with this is. Is it problematic that this audit is presented this way? How would we prevent it from ever being displayed? Should I just skip it in the createAuditPairs like uses-long-cache-ttl is skipped?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

confirmed this is existing behavior in https://googlechrome.github.io/lighthouse-ci/viewer/

@connorjclark connorjclark merged commit 6e4585d into main Feb 1, 2022
@connorjclark connorjclark deleted the lh9 branch February 1, 2022 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants