chore: print stdout/stderr when dev server fails in tests #4918
Conversation
…stdout/stderr on failure
danez
added
the
type: chore
📊 Benchmark resultsComparing with 91f4ab2 Package size: 228 MB⬇️ 0.00% decrease vs. 91f4ab2 Legend
|
this way if the error is catched we do not print anything
| ps.stdout.on('data', (data) => { | ||
| outputBuffer.push(data) | ||
| if (!expectFailure && data.includes('Server now ready on')) { | ||
| if (!expectFailure && data.includes('Edge functions server running')) { |
There was a problem hiding this comment.
My understanding is that we were looking for Server now ready on because that's a line we always print in the dev server. We're now changing that to a string that only applies when we use edge functions? Can you add a bit more context here?
There was a problem hiding this comment.
The main problem here is that when we print Server now ready on the edge function server is not yet up and running, because the start function is async but we do not await it.. This makes total sense for users, not so much the tests where we pretty much instantly start the assertions/requests/etc of the tests once this is printed.
Somehow I had the perception yesterday that this message is printed after the edge function proxy server is started. Looking at the code today that is not the case and this change does not make any sense. The heat is slowly (or fastly) making my brain not really work the last days.🥵
Still the question is how can we wait in tests for edge functions to be up before starting requests? Conditionally awaiting the edge functions seems one possibility.
const server = prepareServer()
if (NODE_ENV === test) {
await server;
}There was a problem hiding this comment.
I don't like the idea of adding test-specific logic to the runtime code. Yes, it would be cool to have a reliable way for the tests to wait for the edge functions server, but if we can't do it without changing the runtime code itself, I'd rather continue with polling and flaky tests, personally.
There was a problem hiding this comment.
Yeah, I reverted this change and now this PR is only printing stdout and stderr on dev server failure.
The only other thing I added is to resolve in the next event loop iteration, to give the edge function server slightly more time to boot.
| @@ -12,9 +12,6 @@ const { normalize } = require('./utils/snapshots') | |||
|
|
|||
| const content = 'Hello World!' | |||
|
|
|||
| // Remove extra '.'s resulting from different load times which lead to flaky snapshots | |||
| const frameworkDetectionNormalizer = (output) => normalize(output.replace(/\.+(?=\.)/, '')) | |||
|
|
|||
There was a problem hiding this comment.
Removed because we do not use the dot reporter in wait-port #4730
danez
changed the title
Summary
This adds a new log message once the edge function server is up and running. The tests wait for this before doing any requests, so we avoid timing issues when we request edge functions but the edge function server is not yet up.
The other improvement is that we log stdout and stderr when a test fails. This gives a lot more information and usually includes the reason why a request failed.