tests(smoke): remove external domains from perf-preload #14289
Conversation
| @@ -12,12 +12,12 @@ document.write('<script src="/perf/level-2.js?warning&delay=500"></script>'); | |||
| // delay our preconnect-candidates so that they're not assumed to be working already | |||
| setTimeout(() => { | |||
| // load another origin | |||
| fetch('http://localhost:10503/preload.html'); | |||
| // fetch('http://localhost:10503/preload.html'); | |||
There was a problem hiding this comment.
Oh, I forgot about this. I think I can just drop this line?
There was a problem hiding this comment.
I think I can drop this without losing coverage of anything, but @paulirish or @adamraine please lmk if you agree
There was a problem hiding this comment.
Yeah I think it should be fine without
|
Shot in the dark, but does this work on devtools? We're skipping it right now in devtools CI. |
nope |
| serverForOnline._server.on('error', e => console.error(e.code, e)); | ||
| serverForOffline._server.on('error', e => console.error(e.code, e)); | ||
| async function createServers() { | ||
| const servers = [10200, 10503, 10420].map(port => { |
There was a problem hiding this comment.
Did you also wanna do the thing where yarn smoke doesn't break if we're already manually running static-server?
maybe catch on the listen and if it rejects we issue a warning but keep going anyway?
There was a problem hiding this comment.
I'd want something more accurate than that - it should verify that the port is actually our static server not just that something happens to be on the port already.
So I'm punting for this PR, but I'll file an issue. #14302
| this.server = new Server(); | ||
| this.secondaryServer = new Server(); | ||
| this.server = new Server(0); | ||
| this.secondaryServer = new Server(0); |
There was a problem hiding this comment.
Might be nice to use createServers here and expose a this.servers for pptr tests.
There was a problem hiding this comment.
createServers hardcodes the ports, so can't use that here.
There was a problem hiding this comment.
There's def. room for a better Server refactor here, but I don't wanna get bogged down on it.
| <!-- PASS(preconnect): This uses crossorigin correctly --> | ||
| <link rel="preconnect" href="https://fonts.gstatic.com" crossorigin /> | ||
| <link rel="preconnect" href="http://localhost:10420" crossorigin /> |
| @@ -12,12 +12,12 @@ document.write('<script src="/perf/level-2.js?warning&delay=500"></script>'); | |||
| // delay our preconnect-candidates so that they're not assumed to be working already | |||
| setTimeout(() => { | |||
| // load another origin | |||
| fetch('http://localhost:10503/preload.html'); | |||
| // fetch('http://localhost:10503/preload.html'); | |||
There was a problem hiding this comment.
Yeah I think it should be fine without
This PR removes dependencies on external domains (google font domains) for the
perf-preloadsmoke test. By request of @exterkamp :)I needed to introduce a third static server to pull off a 1:1 transition, so in the process I also cleaned up that code a bit.