Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
docs(auth): running Lighthouse on authenticated pages #9628
docs(auth): running Lighthouse on authenticated pages #9628
Changes from 27 commits
aecfd9f
6aa9c08
69adf71
5757985
0ae9040
5469beb
59aa12f
eadc8e0
6ac8d37
f268134
06dd0b0
5e460a5
b54a4ac
ae5f2e5
d9feca1
3d171bf
60cef1d
d7b0688
a3425e7
e36b5e1
25ff0f9
ec7f7bb
836e64a
915ad58
6a20cb1
b4f456e
56f59c0
57f2bd2
59bd132
cb64b52
80e8fb0
a917c02
fec4450
41e85cf
1de9c39
a86d68b
d5f9b5b
4a3bc3b
ed6bc34
558ab99
c2669ff
b4afaf7
05fb347
012c5b2
0a25f61
e43436b
51c5bb6
5e1d786
dfe971b
a9da287
0e43b70
d9d6c67
cfa14b9
826f855
3dc265b
c5d3cda
e3b520f
e389e28
97b4dac
a6ceaa2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mentioned this in another comment, but we should pull in the authentication advice in https://github.com/GoogleChrome/lighthouse/blob/master/docs/readme.md#testing-on-a-site-with-authentication over to here since people are using this method to set up a primed user profile
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODOs in the docs make the doc writers look bad when really in this case it's a functionality TODO and when it's done the docs can follow :)
Maybe after mentioning the existing workflow above, it could say that an easier method is being considered for implementation and you can learn more at [issue link] (and that the reader can weigh in there)? I'm not sure how much detail is important to include here vs redirecting over there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe move this to the top of the page alongside the
If you just want to view the code
? By the time I got down to the bottom through the otherexample-lh-auth.js
example it wasn't clear this was a whole different thing.And if I'm a new or repeat visitor looking specifically for an integration test example, it's helpful to have the link off at the top rather than deep in the page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
your more recent comment suggests pulling the jest integration thing into its own recipe. I like and agree with that.
@connorjclark i'm thinking a recipes/integration-test folder. the readme would add in "btw: we're building on the [/auth recipe]". i think it makes sense to do this bit in a followup PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still think the layers of nesting here make it difficult to understand as a first time puppeteer example but perhaps it's also just a style choice of making
it
s be minimal assertions so 🤷♂There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah I agree and would like to reduce the nesting too
#9628 (comment) (i think you missed this, github cut off the discussion)