core: warn if document was redirected #10157
Conversation
connorjclark
requested review from
brendankenny,
patrickhulce and
paulirish
as code owners
| @@ -491,6 +504,12 @@ class GatherRunner { | |||
|
|
|||
| // Copy redirected URL to artifact. | |||
| baseArtifacts.URL.finalUrl = passContext.url; | |||
| if (baseArtifacts.URL.requestedUrl !== baseArtifacts.URL.finalUrl) { | |||
There was a problem hiding this comment.
should probably use equal without fragments?
| @@ -27,6 +27,9 @@ const expectations = [ | |||
| }, | |||
| }, | |||
| }, | |||
| runWarnings: [ | |||
| /The page may not be loading as expected because your test URL \(.*online-only.html.*\) was redirected to .*redirects-final.html. Try testing the second URL directly./, | |||
There was a problem hiding this comment.
is there a non-deterministic component of this that warrants a regex?
There was a problem hiding this comment.
this first one - yes, cacheBuster is a new Date. The second - no. Still good for eliding the URL query params that don't matter.
| @@ -102,7 +102,7 @@ module.exports = [ | |||
| score: 1, | |||
| }, | |||
| 'works-offline': { | |||
| score: 0, | |||
| score: 1, | |||
There was a problem hiding this comment.
This does work offline. Was this meant to be testing something subtle re: redirection? I'm assuming not, since no comments.
There was a problem hiding this comment.
I'm guessing it didn't always work offline, this test might be 4 years old :)
| @@ -102,7 +102,7 @@ module.exports = [ | |||
| score: 1, | |||
| }, | |||
| 'works-offline': { | |||
| score: 0, | |||
| score: 1, | |||
There was a problem hiding this comment.
I'm guessing it didn't always work offline, this test might be 4 years old :)
| @@ -491,6 +505,13 @@ class GatherRunner { | |||
|
|
|||
| // Copy redirected URL to artifact. | |||
| baseArtifacts.URL.finalUrl = passContext.url; | |||
| /* eslint-disable max-len */ | |||
| if (!URL.equalWithExcludedFragments(baseArtifacts.URL.requestedUrl, baseArtifacts.URL.finalUrl)) { | |||
There was a problem hiding this comment.
#10218 is going to throw a major wrench into all the places where we detect redirects like this 😆
but a problem for another day
fixes #1917