report: create faux psi report #12815

paulirish · 2021-07-21T17:03:39Z

I wanted a test page where there are two LHRs, like in PSI. This helps surface the peculiar behavior we get with tabs and element-screenshot-overlay and how it's gotta live outside the tab body.

here 'tis: https://lighthouse-git-faux-psi-googlechrome.vercel.app/%E2%8C%A3.psi.english

…uirrely.

adamraine · 2021-07-21T18:42:40Z

I noticed clicking on an audit filter in the desktop tab doesn't do anything on the desktop audits, but does change the filter for the mobile tab.

Edit: ope this is a PSI bug

paulirish · 2021-07-21T22:58:48Z

I noticed clicking on an audit filter in the desktop tab doesn't do anything on the desktop audits, but does change the filter for the mobile tab.

Edit: ope this is a PSI bug

heh. well. thats exactly what this lil stupid report is for. :)
kinda perfect, tbh.

update: PR to fix up in #12817

connorjclark

I wanted a test page where there are two LHRs, like in PSI. This helps surface the peculiar behavior we get with tabs and element-screenshot-overlay and how it's gotta live outside the tab body.

Can you put that as a comment in one of the PSI test "asset" files?

connorjclark · 2021-07-21T23:09:38Z

lighthouse-core/scripts/build-report-for-autodeployment.js

 const lhr = /** @type {LH.Result} */ (require('../../lighthouse-core/test/results/sample_v2.json'));
 const {LH_ROOT} = require('../../root.js');

 const DIST = path.join(LH_ROOT, `dist/now`);

 (async function() {
+  // Must build before importing report-generator (or indirectly through lighthouse)
+  await buildreport.buildStandaloneReport();


why remove yarn build-report && from yarn now-build ? it's simpler than doing the imports here in the right order.

the CLI options could be expanded to build psi report on a flag

hmm.. i don't know what you do, but if i'm working on the report this is my watch equivalent:

find report | entr node lighthouse-core/scripts/build-report-for-autodeployment.js

i heart entr. (but i can't && in that command or whatever so... the combo of the two commands is a problem).

i guess an alternative would be a new yarn build-dist-reports script that does build-report followed by this. that'd be fine.

wdyt?

but i can't && in that command or whatever so.

Do it like this:

find report | entr bash -c "ls && echo hi"

wdyt?

the extra commands make it hard to follow. go back to yarn build-report && node lighthouse-core/scripts/build-report-for-autodeployment.js && yarn build-viewer && yarn build-treemap && cp -r dist/gh-pages dist/now/gh-pages and just use the bash trick above ?

I assume you don't just do find report | yarn now-build because of all the extra stuff going on there that it takes a bit longer... 7s (vs 2s) for a watch is rough

lighthouse-core/scripts/build-report-for-autodeployment.js

report/clients/psi.js

report/report-generator.js

report/assets/faux-psi-template.html

report/clients/psi.js

build/build-report.js

connorjclark · 2021-08-03T20:11:46Z

package.json

@@ -54,6 +55,7 @@
    "cli-unit": "yarn unit-cli",
    "viewer-unit": "yarn unit-viewer",
    "watch": "yarn unit-core --watch",
+    "watch:report": "find report | entr yarn build-dist-reports",


brendankenny · 2021-08-03T19:15:03Z

lighthouse-core/scripts/build-report-for-autodeployment.js

+/**
+ * @return {string}
+ */
+function generatePsiReportHtml() {


this definitely seems kind of weird in build-report-for-autodeployment.js and possibly hard to keep up to date when changes happen to report-generator, but I also like keeping it simple like this while we iterate on report/overall build story

brendankenny · 2021-08-03T19:16:47Z

lighthouse-core/scripts/build-report-for-autodeployment.js

+ * Drops the LHR to only one, solo category (performance), and removes budgets.
+ * @param {LH.Result} lhr
+ */
+function tweakLhrForPsi(lhr) {


should this matter? Shouldn't it just be able to take a regular LHR and just ignore the stuff it doesn't want? Or is this more a matter of giving it the data it will actually get in practice for a more realistic deployment?

It shouldnt matter much, but I just wanted to the data to match what PSI would expect to see. I can't think of any real reasons why it'd matter (like full-page-screenshot being in another category or something).

brendankenny · 2021-08-03T21:59:08Z

package.json

@@ -54,6 +55,7 @@
    "cli-unit": "yarn unit-cli",
    "viewer-unit": "yarn unit-viewer",
    "watch": "yarn unit-core --watch",
+    "watch:report": "find report | entr yarn build-dist-reports",


what happened to

sure lets just leave the entr stuff out of our scripts for now

?

brendankenny · 2021-08-03T22:00:18Z

package.json

@@ -24,6 +24,7 @@
    "build-lr": "yarn reset-link && node ./build/build-lightrider-bundles.js",
    "build-pack": "bash build/build-pack.sh",
    "build-report": "node build/build-report.js",
+    "build-dist-reports": "yarn build-report && node lighthouse-core/scripts/build-report-for-autodeployment.js",


bike shed: what is build-dist-reports referring to? Just build reports that will be in dist/? Because it's also sample reports in particular. build-sample-reports?

hehe me and paul were talking about the naming here...

build-dist-reports: Builds the report renderer and the reports for the Vercel deployment

build-report-for-autodeployment.js: Builds the reports for the Vercel deployment

now-build: The Vercel deployment used to be called "now". not anymore

some renaming is in order all around.

brendankenny · 2021-08-03T22:03:44Z

report/report-generator.js

@@ -28,15 +28,23 @@ class ReportGenerator {
  }

  /**
-   * Returns the report HTML as a string with the report JSON and renderer JS inlined.
+   * @param {Object} object


Suggested change

* @param {Object} object

* @param {unknown} object

brendankenny · 2021-08-03T22:04:06Z

report/report-generator.js

+  }
+
+  /**
+   * Returns the standaloe report HTML as a string with the report JSON and renderer JS inlined.


Suggested change

* Returns the standaloe report HTML as a string with the report JSON and renderer JS inlined.

* Returns the standalone report HTML as a string with the report JSON and renderer JS inlined.

brendankenny · 2021-08-03T22:09:24Z

report/test-assets/faux-psi.js

+ * @param {LH.Result} lhr
+ * @param {string} tabId
+ */
+async function distinguishLHR(lhr, tabId) {


can this be combined with what happens in tweakLhrForPsi?

i could but then i wouldn't have the single-category dist-report. but i really want that for local report dev. :)

…ing no mo

paulirish and others added 10 commits July 9, 2021 16:43

report api. (and psi thinger)

60e38ee

cleanup

9d0326b

'proper' psi variant

f059abd

Merge remote-tracking branch 'origin/master' into reportapi

da84011

fixup psi dist

c18bd48

new tabs. distinguished screenshots

349a655

Merge remote-tracking branch 'origin/master' into reportapi

f6003b6

cleanup psi more

4c2f18f

report: create faux psi report

27e21f4

drop types

1bdd6e3

paulirish requested a review from a team as a code owner July 21, 2021 17:03

paulirish requested review from adamraine and removed request for a team July 21, 2021 17:03

google-cla bot added the cla: yes label Jul 21, 2021

devtools-bot assigned adamraine Jul 21, 2021

devtools-bot added the waiting4reviewer label Jul 21, 2021

await the buildpsi

47e6f15

vercel bot deployed to Preview July 21, 2021 17:10 View deployment

no need to doublebuild

2003340

vercel bot had a problem deploying to Preview July 21, 2021 17:12 Failure

html-report-assets is a runtime dep that depends on a built asset. sq…

9df8212

…uirrely.

vercel bot deployed to Preview July 21, 2021 17:26 View deployment

paulirish added a commit that referenced this pull request Jul 21, 2021

moved that other stuff to #12815

743b1ea

vercel bot deployed to Preview July 21, 2021 17:49 View deployment

tweak psi lhr

131ef40

paulirish mentioned this pull request Jul 21, 2021

report(metric-filter): handle multiple reports in the same DOM #12817

Merged

connorjclark requested changes Jul 21, 2021

View reviewed changes

revert generateReportHTML changes

a769ffc

vercel bot deployed to Preview August 3, 2021 19:48 View deployment

connorjclark approved these changes Aug 3, 2021

View reviewed changes

lint

1364af2

vercel bot deployed to Preview August 3, 2021 20:20 View deployment

paulirish added 2 commits August 3, 2021 13:25

PrepareLabDataResult

51d832b

flags and logs

ffa6ec3

vercel bot deployed to Preview August 3, 2021 20:33 View deployment

connorjclark approved these changes Aug 3, 2021

View reviewed changes

brendankenny reviewed Aug 3, 2021

View reviewed changes

connorjclark added waiting4committer and removed waiting4reviewer labels Aug 6, 2021

rm watch:report

0dc332f

vercel bot deployed to Preview August 10, 2021 20:30 View deployment

connorjclark approved these changes Aug 10, 2021

View reviewed changes

brendan's feedback

3d02808

vercel bot deployed to Preview August 10, 2021 21:03 View deployment

Merge remote-tracking branch 'origin/master' into faux-psi

2474bd4

vercel bot deployed to Preview August 10, 2021 21:08 View deployment

paulirish mentioned this pull request Aug 10, 2021

misc(build): normalize sample-report build naming #12901

Merged

paulirish added the land-when-ci-is-green label Aug 10, 2021

adamraine removed the land-when-ci-is-green label Aug 10, 2021

Merge remote-tracking branch 'origin/master' into faux-psi

bcdc3d8

paulirish added the land-when-ci-is-green label Aug 11, 2021

vercel bot deployed to Preview August 11, 2021 15:52 View deployment

paulirish added 2 commits August 11, 2021 15:13

Merge remote-tracking branch 'origin/master' into faux-psi

21fcf51

remove references to LIGHTHOUSE_TEMPLATES as templates.html aint a th…

77a488d

…ing no mo

vercel bot deployed to Preview August 11, 2021 22:18 View deployment

devtools-bot merged commit e6a6b61 into master Aug 11, 2021

devtools-bot deleted the faux-psi branch August 11, 2021 22:39

devtools-bot removed the land-when-ci-is-green label Aug 11, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

report: create faux psi report #12815

report: create faux psi report #12815

paulirish commented Jul 21, 2021 •

edited

Loading

adamraine commented Jul 21, 2021 •

edited

Loading

paulirish commented Jul 21, 2021 •

edited

Loading

connorjclark left a comment

connorjclark Jul 21, 2021

paulirish Jul 23, 2021 •

edited

Loading

connorjclark Jul 23, 2021 •

edited

Loading

connorjclark Aug 3, 2021

brendankenny Aug 3, 2021

brendankenny Aug 3, 2021

paulirish Aug 10, 2021

brendankenny Aug 3, 2021

brendankenny Aug 3, 2021

connorjclark Aug 6, 2021

brendankenny Aug 3, 2021

brendankenny Aug 3, 2021

brendankenny Aug 3, 2021

paulirish Aug 10, 2021

	* Returns the standaloe report HTML as a string with the report JSON and renderer JS inlined.
	* Returns the standalone report HTML as a string with the report JSON and renderer JS inlined.

report: create faux psi report #12815

report: create faux psi report #12815

Conversation

paulirish commented Jul 21, 2021 • edited Loading

adamraine commented Jul 21, 2021 • edited Loading

paulirish commented Jul 21, 2021 • edited Loading

connorjclark left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulirish Jul 23, 2021 • edited Loading

Choose a reason for hiding this comment

connorjclark Jul 23, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulirish commented Jul 21, 2021 •

edited

Loading

adamraine commented Jul 21, 2021 •

edited

Loading

paulirish commented Jul 21, 2021 •

edited

Loading

paulirish Jul 23, 2021 •

edited

Loading

connorjclark Jul 23, 2021 •

edited

Loading