Skip to content

tests(smoke): make oopif-requests assertions more specific #14100

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.

Sign up for GitHub

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

Merged
merged 10 commits into from
Jun 22, 2022
Merged

tests(smoke): make oopif-requests assertions more specific #14100

Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8996387
tests: udpate oopif-requests-assertion
paulirish Jun 8, 2022
238b082
ensure we dont have TWO network requests for youtube
paulirish Jun 8, 2022
62fa189
note about double yt
paulirish Jun 8, 2022
a965ea5
subresources: gtag and player.css
paulirish Jun 8, 2022
b039b3e
Merge remote-tracking branch 'origin/master' into oopifreqs-asssertion
paulirish Jun 8, 2022
105d9d9
Merge branch 'master' into oopifreqs-asssertion
paulirish Jun 8, 2022
d793c3f
Merge remote-tracking branch 'origin/master' into oopifreqs-asssertion
paulirish Jun 10, 2022
35e1f5e
two more commonly missing requests
paulirish Jun 10, 2022
6f4581e
Merge remote-tracking branch 'origin/oopifreqs-asssertion' into oopif…
paulirish Jun 10, 2022
2cf684e
Merge remote-tracking branch 'origin/master' into oopifreqs-asssertion
connorjclark Jun 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 22 additions & 5 deletions lighthouse-cli/test/smokehouse/test-definitions/oopif-requests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,28 @@ const expectations = {
'network-requests': {
details: {
items: {
Copy link
Member

Choose a reason for hiding this comment

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

Can you add the first non-document request for each iframe as well. This would be a check to ensure we start listening for OOPIF requests in time. For me, those were:

  • paulirish.com: https://www.googletagmanager.com/gtag/js?id=G-PGXNGYWP8E
  • YT embed: https://www.youtube.com/s/player/966d033c/www-player.css

Copy link
Collaborator

Choose a reason for hiding this comment

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

gtag: ok
yt: that's gonna go stale real quick, just check for any css ?

Copy link
Member

Choose a reason for hiding this comment

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

that's gonna go stale real quick, just check for any css ?

Maybe look for https://www.youtube.com/s/player/*/www-player.css. I think looking for this specific CSS file is important. It was frequently one of the "missing" requests over in perf-diagnostics-third-party which also has a YT embed.

Copy link
Member Author

Choose a reason for hiding this comment

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

@adamraine added.

// We want to make sure we are finding the iframe's requests (paulirish.com) *AND*
// the iframe's iframe's iframe's requests (youtube.com/doubleclick/etc).
// - paulirish.com ~40-60 requests
// - paulirish.com + all descendant iframes ~80-90 requests
length: '>70',
// We want to make sure we are finding the iframe's requests (paulirish.com) *AND*
// the iframe's iframe's iframe's requests (youtube.com/doubleclick/etc).
_includes: [
{url: 'http://localhost:10200/oopif-requests.html', finished: true, statusCode: 200, resourceType: 'Document'},

// Paulirish iframe and subresource
{url: 'https://www.paulirish.com/2012/why-moving-elements-with-translate-is-better-than-posabs-topleft/', finished: true, statusCode: 200, resourceType: 'Document'},
{url: 'https://www.paulirish.com/avatar150.jpg', finished: true, statusCode: 200, resourceType: 'Image'},
{url: 'https://www.googletagmanager.com/gtag/js?id=G-PGXNGYWP8E', finished: true, statusCode: 200, resourceType: 'Script'},
{url: /^https:\/\/fonts\.googleapis\.com\/css/, finished: true, statusCode: 200, resourceType: 'Stylesheet'},

// Youtube iframe (OOPIF) and some subresources
// FYI: Youtube has a ServiceWorker which sometimes cancels the document request. As a result, there will sometimes be multiple requests for this file.
{url: 'https://www.youtube.com/embed/NZelrwd_iRs', finished: true, statusCode: 200, resourceType: 'Document'},
{url: /^https:\/\/www\.youtube\.com\/.*?player.*?css/, finished: true, statusCode: 200, resourceType: 'Stylesheet'},
{url: /^https:\/\/www\.youtube\.com\/.*?\/embed.js/, finished: true, statusCode: 200, resourceType: 'Script'},

// Disqus iframe (OOPIF)
{url: /^https:\/\/disqus\.com\/embed\/comments\//, finished: true, statusCode: 200, resourceType: 'Document'},
// Disqus subframe (that's a new OOPIF)
{url: 'https://accounts.google.com/o/oauth2/iframe', finished: true, statusCode: 200, resourceType: 'Document'},
],
},
},
},
Expand Down