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

[paulirish]

fixes #14099

We currently just assert that there's 70+ requests. I've seen CI failures for this with 69, 67 and 41 requests. Here's a diff of the "correct" run vs one of the failing ones: https://diff.googleplex.com/#key=AfquQTaPCU4B

In the cases where it fails... its either missing subresources from paulirish.com or items from the youtube iframe. (images and JS, generally). These missing requests don't indicate (to me) any oopif instrumentation issue. I suspect the failures just indicate slow outbound network on GHA's runners. (i guess)

IMO this assertion feels stronger to me. yah?

[adamraine]

These missing requests don't indicate (to me) any oopif instrumentation issue. I suspect the failures just indicate slow outbound network on GHA's runners.

This hasn't been the case when it comes to our other, more reproducible oopif test failures (perf-diagnostics-third-party and oopif-scripts). In those cases there definitely is an oopif instrumentation problem because we verified that the network requests happened, and LH just didn't catch them. For this reason, I don't want to assume that the condition is too strict.

I might actually prefer making the condition more strict if the current failures are indeed caused by an instrumentation failure because of how infrequent oopif-requests failures are.

[paulirish]

@adamraine when you say "strict" are you referring to the >70 requests assertion? (cuz to me, it is, but also it isn't)

I'm very fine with asserting more than what I put in this PR. But I do feel like we should have a test page that we can put extremely firm assertions on. (Aka not depending on youtube and disqus asset URLs)

[adamraine]

Yeah, I mean having more concrete assertions, not necessarily increasing the >70 resource count.

[connorjclark]

why have all the finished/statusCode/resourceType stuff?

[paulirish]

generally: why not?

specifically:

finished tells us it ... finished. When working on this I was seeing cases where in an iframe we didn't get the loadingFinished event for that document.

However a request that gets Network.loadingFailed is marked as finished and thus the check of statusCode. it's -1 in bad cases.

and resourceType is there to basically verify it was transferred the way we expect. i don't expect it to catch much, but i don't see a reason why not.

[adamraine]

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

[connorjclark]

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

[adamraine]

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.

[paulirish]

@adamraine added.

[adamraine]

This is good, I think oopif-requests is much less flaky now.

The problem is that it's still failing, so we might need to disable this smoke test and add it to the backlog.