Migrate Lighthouse data-warning test from webtests to e2e [devtools/devtools-frontend : main]

Paul Irish (Gerrit)

unread,

Jun 1, 2022, 10:50:12 PM6/1/22

to Connor Clark, devtools-rev...@chromium.org, lighthouse-en...@google.com, Benedikt Meurer

Attention is currently required from: Benedikt Meurer, Connor Clark.

Paul Irish would like Connor Clark to review this change.

Migrate Lighthouse data-warning test from webtests to e2e

Change-Id: Ic43ae8597fbc7bd0a7e610a78a2feeb6fa886361
Bug: 1331155
---
M test/e2e/README.md
M test/e2e/helpers/lighthouse-helpers.ts
M test/e2e/lighthouse/BUILD.gn
R test/e2e/lighthouse/generate-report_test.ts
A test/e2e/lighthouse/indexeddb-warning_test.ts
M test/e2e/resources/BUILD.gn
A test/e2e/resources/lighthouse/BUILD.gn
A test/e2e/resources/lighthouse/lighthouse-storage.html
D test/webtests/http/tests/devtools/lighthouse/lighthouse-clear-data-warning.js
D test/webtests/http/tests/devtools/lighthouse/resources/lighthouse-storage.html
D test/webtests/platform/generic/http/tests/devtools/lighthouse/lighthouse-clear-data-warning-expected.txt
11 files changed, 89 insertions(+), 84 deletions(-)

To view, visit change 3682120. To unsubscribe, or for help writing mail filters, visit settings.

Paul Irish (Gerrit)

unread,

Jun 1, 2022, 10:50:27 PM6/1/22

to devtools-rev...@chromium.org, lighthouse-en...@google.com, Connor Clark, Benedikt Meurer

Attention is currently required from: Benedikt Meurer, Connor Clark.

View Change

1 comment:

File front_end/panels/lighthouse/LighthousePanel.ts:
- Patch Set #1, Line 283: globalThis.__lighthouseResult = lighthouseResult;
  cc @bmeurer […]
  Actually, we'll hold off on the question. I'm going to kick off this migration with a simpler test.

To view, visit change 3682120. To unsubscribe, or for help writing mail filters, visit settings.

Paul Irish (Gerrit)

unread,

Jun 1, 2022, 10:52:05 PM6/1/22

to devtools-rev...@chromium.org, lighthouse-en...@google.com, Connor Clark, Benedikt Meurer

Attention is currently required from: Benedikt Meurer, Connor Clark.

View Change

4 comments:

File test/e2e/lighthouse/block-url_test.ts:
- Patch Set #1, Line 1: // Copyright 2020 The Chromium Authors. All rights reserved.
  year
  Done
- Patch Set #1, Line 16: it('is open by default when devtools initializes', async () => {
  what does this mean? open by default... […]
  i stole the test from the Security panel tests. it means "you can click to the Lighthouse tab (from the tab strip) and then the panel shows up"
  but i've removed it now.
- Patch Set #1, Line 34: if (!button) {
  prefer single line assertion: […]
  Ack
- Patch Set #1, Line 65: await frontend.evaluate(() => {
  should this be in `before`?
  Ack

To view, visit change 3682120. To unsubscribe, or for help writing mail filters, visit settings.

Adam Raine (Gerrit)

unread,

Jun 2, 2022, 12:49:49 PM6/2/22

to Paul Irish, devtools-rev...@chromium.org, lighthouse-en...@google.com, Connor Clark, Benedikt Meurer

Attention is currently required from: Benedikt Meurer, Connor Clark, Paul Irish.

View Change

2 comments:

File test/e2e/resources/lighthouse/BUILD.gn:
- Patch Set #9, Line 1: # Copyright 2021 The Chromium Authors. All rights reserved.
  2022
File test/e2e/resources/lighthouse/lighthouse-storage.html:
- Patch Set #9, Line 6: window.indexedDB.open("DataBase", 3);
  Is it possible to keep the websql stuff? Would be nice to leverage multiple data sources. Maybe localstorage would work instead?

To view, visit change 3682120. To unsubscribe, or for help writing mail filters, visit settings.

Paul Irish (Gerrit)

unread,

Jun 2, 2022, 2:42:11 PM6/2/22

to devtools-rev...@chromium.org, lighthouse-en...@google.com, Adam Raine, Connor Clark, Benedikt Meurer

Attention is currently required from: Adam Raine, Benedikt Meurer, Connor Clark.

View Change

2 comments:

File test/e2e/resources/lighthouse/BUILD.gn:

- Patch Set #9, Line 1: # Copyright 2021 The Chromium Authors. All rights reserved.
  2022

- Done
File test/e2e/resources/lighthouse/lighthouse-storage.html:

- Patch Set #9, Line 6: window.indexedDB.open("DataBase", 3);

- Is it possible to keep the websql stuff? Would be nice to leverage multiple data sources. […]
  discussed offline.
  there's some serious issues with how websql interacts with the getUsageAndQuota implementation. it's impossible to get a reliable and correct response without adding arbitrary ~500ms timeouts.
  and as websql has been deprecated for ages, it's not worth escalating those bugs.
  so we'll go without. :)
  --
  Luckily, the exercise helped me find a few improvements to the test script which I've now added.

To view, visit change 3682120. To unsubscribe, or for help writing mail filters, visit settings.

Connor Clark (Gerrit)

unread,

Jun 2, 2022, 3:53:48 PM6/2/22

to Paul Irish, devtools-rev...@chromium.org, lighthouse-en...@google.com, Adam Raine, Benedikt Meurer

Attention is currently required from: Adam Raine, Benedikt Meurer, Paul Irish.

View Change

2 comments:

File test/e2e/helpers/lighthouse-helpers.ts:
- Patch Set #10, Line 9: lighthousePanelContentIsLoaded
  waitForLighthousePanelContentLoaded ?
File test/e2e/lighthouse/indexeddb-warning_test.ts:
- Patch Set #10, Line 3: // found in the LICENSE file.
  seems like you accidentally deleted the old generate-report_test ?

To view, visit change 3682120. To unsubscribe, or for help writing mail filters, visit settings.

Paul Irish (Gerrit)

unread,

Jun 2, 2022, 4:03:34 PM6/2/22

to devtools-rev...@chromium.org, lighthouse-en...@google.com, Adam Raine, Connor Clark, Benedikt Meurer

Attention is currently required from: Adam Raine, Benedikt Meurer, Connor Clark.

View Change

2 comments:

File test/e2e/helpers/lighthouse-helpers.ts:

- Patch Set #10, Line 9: lighthousePanelContentIsLoaded
  waitForLighthousePanelContentLoaded ?

- Done
File test/e2e/lighthouse/indexeddb-warning_test.ts:

- Patch Set #10, Line 3: // found in the LICENSE file.
  seems like you accidentally deleted the old generate-report_test ?

- it was named
  test/e2e/lighthouse/generate-report.ts
  but i renamed it
  test/e2e/lighthouse/generate-report_test.ts
  dunno what happened, but the fact that it didnt have the `_test` suffix meant it wasn't included in the run of all e2e tests. (so i fixed that)

To view, visit change 3682120. To unsubscribe, or for help writing mail filters, visit settings.

Benedikt Meurer (Gerrit)

unread,

Jun 3, 2022, 5:30:00 AM6/3/22

to Danil Somsikov, devtools-rev...@chromium.org, lighthouse-en...@google.com, Paul Irish, Connor Clark

Attention is currently required from: Adam Raine, Connor Clark, Danil Somsikov.

Benedikt Meurer would like Danil Somsikov to review this change authored by Paul Irish.

View Change

Migrate Lighthouse data-warning test from webtests to e2e

Change-Id: Ic43ae8597fbc7bd0a7e610a78a2feeb6fa886361
Bug: 1331155
---
M test/e2e/README.md
M test/e2e/helpers/lighthouse-helpers.ts
M test/e2e/lighthouse/BUILD.gn
R test/e2e/lighthouse/generate-report_test.ts
A test/e2e/lighthouse/indexeddb-warning_test.ts
M test/e2e/resources/BUILD.gn
A test/e2e/resources/lighthouse/BUILD.gn
A test/e2e/resources/lighthouse/lighthouse-storage.html
D test/webtests/http/tests/devtools/lighthouse/lighthouse-clear-data-warning.js
D test/webtests/http/tests/devtools/lighthouse/resources/lighthouse-storage.html
D test/webtests/platform/generic/http/tests/devtools/lighthouse/lighthouse-clear-data-warning-expected.txt
11 files changed, 89 insertions(+), 84 deletions(-)

To view, visit change 3682120. To unsubscribe, or for help writing mail filters, visit settings.

Danil Somsikov (Gerrit)

unread,

Jun 3, 2022, 5:33:52 AM6/3/22

to Paul Irish, devtools-rev...@chromium.org, lighthouse-en...@google.com, Adam Raine, Connor Clark

Attention is currently required from: Adam Raine, Connor Clark, Paul Irish.

Patch set 11:Code-Review +1

View Change

To view, visit change 3682120. To unsubscribe, or for help writing mail filters, visit settings.

Adam Raine (Gerrit)

unread,

Jun 3, 2022, 1:06:42 PM6/3/22

to Paul Irish, devtools-rev...@chromium.org, lighthouse-en...@google.com, Danil Somsikov, Connor Clark

Attention is currently required from: Connor Clark, Paul Irish.

Patch set 11:Code-Review +1

View Change

2 comments:

File test/e2e/lighthouse/indexeddb-warning_test.ts:
- Patch Set #11, Line 24: await panel.$eval('[aria-label="Categories"] [is=dt-checkbox]', async elem => {
  So the warning doesn't change when the user navigates to a new URL? We should probably fix this at some point.
File test/e2e/resources/lighthouse/lighthouse-storage.html:
- Patch Set #11, Line 5: // Indexeddb
  super nit: this comment isn't very helpful without other storage methods

To view, visit change 3682120. To unsubscribe, or for help writing mail filters, visit settings.

Paul Irish (Gerrit)

unread,

Jun 3, 2022, 3:37:43 PM6/3/22

to devtools-rev...@chromium.org, lighthouse-en...@google.com, Adam Raine, Danil Somsikov, Connor Clark

Attention is currently required from: Connor Clark, Paul Irish.

Patch set 11:Commit-Queue +2

View Change

To view, visit change 3682120. To unsubscribe, or for help writing mail filters, visit settings.

Paul Irish (Gerrit)

unread,

Jun 3, 2022, 3:41:13 PM6/3/22

to devtools-rev...@chromium.org, lighthouse-en...@google.com, Devtools-frontend LUCI CQ, Adam Raine, Danil Somsikov, Connor Clark

Attention is currently required from: Adam Raine, Connor Clark.

View Change

2 comments:

File test/e2e/lighthouse/indexeddb-warning_test.ts:

- Patch Set #11, Line 24: await panel.$eval('[aria-label="Categories"] [is=dt-checkbox]', async elem => {

- So the warning doesn't change when the user navigates to a new URL? We should probably fix this at s […]
  that's a good point. the recomputeAuditability is setup to run again on InspectedURLChanged. I don't know when that evt fires relative to the iDB database being opened... but.. yeah there's a good chance we don't need this twiddle at all.
File test/e2e/resources/lighthouse/lighthouse-storage.html:

- Patch Set #11, Line 5: // Indexeddb
  super nit: this comment isn't very helpful without other storage methods

- lol true.

To view, visit change 3682120. To unsubscribe, or for help writing mail filters, visit settings.

Paul Irish (Gerrit)

unread,

Jun 3, 2022, 5:26:37 PM6/3/22

to devtools-rev...@chromium.org, lighthouse-en...@google.com, Devtools-frontend LUCI CQ, Adam Raine, Danil Somsikov, Connor Clark

Attention is currently required from: Adam Raine, Connor Clark.

Patch set 13:Commit-Queue +2

View Change

2 comments:

File test/e2e/lighthouse/indexeddb-warning_test.ts:

- Patch Set #11, Line 24: await panel.$eval('[aria-label="Categories"] [is=dt-checkbox]', async elem => {

- that's a good point. the recomputeAuditability is setup to run again on InspectedURLChanged. […]
  dropped the twiddle!
File test/e2e/resources/lighthouse/lighthouse-storage.html:

- Patch Set #11, Line 5: // Indexeddb

- lol true.
  removed.

To view, visit change 3682120. To unsubscribe, or for help writing mail filters, visit settings.

Connor Clark (Gerrit)

unread,

Jun 3, 2022, 6:23:07 PM6/3/22

to Paul Irish, devtools-rev...@chromium.org, lighthouse-en...@google.com, Devtools-frontend LUCI CQ, Adam Raine, Danil Somsikov

Attention is currently required from: Adam Raine, Paul Irish.

View Change

1 comment:

File test/webtests/http/tests/devtools/lighthouse/lighthouse-clear-data-warning.js:
- Patch Set #13, Line 4:
  deleting these is fine, but please wait for v9.6.2 to merge into devtools first.

To view, visit change 3682120. To unsubscribe, or for help writing mail filters, visit settings.

Paul Irish (Gerrit)

unread,

Jun 14, 2022, 10:12:16 PM6/14/22

to devtools-rev...@chromium.org, lighthouse-en...@google.com, Devtools-frontend LUCI CQ, Adam Raine, Danil Somsikov, Connor Clark

Attention is currently required from: Adam Raine, Paul Irish.

Patch set 15:Commit-Queue +2

View Change

To view, visit change 3682120. To unsubscribe, or for help writing mail filters, visit settings.

Devtools-frontend LUCI CQ (Gerrit)

unread,

Jun 14, 2022, 10:52:26 PM6/14/22

to Paul Irish, devtools-rev...@chromium.org, lighthouse-en...@google.com, Adam Raine, Danil Somsikov, Connor Clark

Devtools-frontend LUCI CQ submitted this change.

View Change



11 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

```
The name of the file: test/e2e/helpers/lighthouse-helpers.ts
Insertions: 27, Deletions: 4.

The diff is too large to show. Please review the diff.
```
```
The name of the file: front_end/panels/lighthouse/LighthouseStartView.ts
Insertions: 1, Deletions: 0.

The diff is too large to show. Please review the diff.
```
```
The name of the file: test/e2e/lighthouse/indexeddb-warning_test.ts
Insertions: 9, Deletions: 13.

The diff is too large to show. Please review the diff.
```

Approvals:
  Adam Raine: Looks good to me
  Danil Somsikov: Looks good to me
  Paul Irish: Commit

Migrate Lighthouse data-warning test from webtests to e2e

Change-Id: Ic43ae8597fbc7bd0a7e610a78a2feeb6fa886361
Bug: 1331155

Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/3682120
Reviewed-by: Danil Somsikov <d...@chromium.org>
Commit-Queue: Paul Irish <paul...@chromium.org>
Reviewed-by: Adam Raine <asr...@chromium.org>
---
M front_end/panels/lighthouse/LighthouseStartView.ts


M test/e2e/README.md
M test/e2e/helpers/lighthouse-helpers.ts
M test/e2e/lighthouse/BUILD.gn
R test/e2e/lighthouse/generate-report_test.ts
A test/e2e/lighthouse/indexeddb-warning_test.ts
M test/e2e/resources/BUILD.gn
A test/e2e/resources/lighthouse/BUILD.gn
A test/e2e/resources/lighthouse/lighthouse-storage.html
D test/webtests/http/tests/devtools/lighthouse/lighthouse-clear-data-warning.js
D test/webtests/http/tests/devtools/lighthouse/resources/lighthouse-storage.html
D test/webtests/platform/generic/http/tests/devtools/lighthouse/lighthouse-clear-data-warning-expected.txt

12 files changed, 112 insertions(+), 83 deletions(-)

To view, visit change 3682120. To unsubscribe, or for help writing mail filters, visit settings.

Reply all

Reply to author

Forward