feat(test-runner): add httpCache config option#41687
Conversation
e9ef052 to
1ff8228
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Skn0tt
left a comment
There was a problem hiding this comment.
I'm not seeing typical caching headers like Vary or Accept-Encoding be mentioned, which seems fishy. Am I misunderstanding something?
This is intentional, typical caching headers in dev / testing flow are not aggressive enough. We might want to have some finer grained controls for that though. |
| hasTouch: [({ contextOptions }, use) => use(contextOptions.hasTouch ?? false), { option: true, box: true }], | ||
| httpCredentials: [({ contextOptions }, use) => use(contextOptions.httpCredentials), { option: true, box: true }], | ||
| ignoreHTTPSErrors: [({ contextOptions }, use) => use(contextOptions.ignoreHTTPSErrors ?? false), { option: true, box: true }], | ||
| ignoreHTTPSErrors: [({ contextOptions }, use) => use(process.env.PLAYWRIGHT_TEST_CACHE_PROXY ? true : (contextOptions.ignoreHTTPSErrors ?? false)), { option: true, box: true }], |
There was a problem hiding this comment.
There is a way to start Chromium so that it trusts our own root certificate, so that we can sign a certificate for any domain dynamically.
There was a problem hiding this comment.
What about the other browsers?
| these; consumers are responsible for slicing and interpreting them with | ||
| any argument-parsing library. | ||
|
|
||
| ## property: FullConfig.httpCache |
There was a problem hiding this comment.
Who is reading this one? Let's not add to the FullConfig just yet.
There was a problem hiding this comment.
Overall, fine with not listing. But I have a general distaste towards the almost identical interfaces, so I'd prefer to keep it.
| res.end(); | ||
| return; | ||
| } | ||
| if (response.status >= 200 && response.status <= 299) |
There was a problem hiding this comment.
This is a subject for discussion, what would you like it to cache?
| return; | ||
| } | ||
| if (response.status >= 200 && response.status <= 299) | ||
| await this._cache.set(key, url, response).catch(() => {}); |
There was a problem hiding this comment.
Shouldn't we respect things like Expires, Age, Cache-Control, etc?
There was a problem hiding this comment.
I don't this we should respect those - documentes and subresources are delivered as no-cache in the dev / testing environments.
Records browser responses to disk and replays them on later runs. When
`httpCache: { dir, match }` is set, the runner starts a single caching
MITM proxy and routes all browsers through it. GET/2xx responses are
stored verbatim (raw bytes + headers) and matched by an optional URL
glob/regex; https is intercepted with a self-signed cert. One proxy owns
the cache for the whole run, so all workers share it and concurrent
identical misses coalesce into one upstream fetch.
1ff8228 to
0d1d6c4
Compare
Test results for "MCP"7735 passed, 1248 skipped Merge workflow run. |
Test results for "tests 1"3 flaky49549 passed, 1161 skipped Merge workflow run. |
Summary
httpCache: { dir, match }test config option. When set, the runner starts a single caching MITM proxy and routes every browser through it: matching responses are recorded todiron the first run and replayed from disk on later runs.match(glob/RegExp) selects which request URLs are cached; https is intercepted with a self-signed cert.Fixes: #22865