Repository navigation
test(server): show what the browser and the failed server printed when the flaky startup checks fail - #223
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #209.
This does not fix a root cause. I could not reproduce either failure, so I changed the two tests so that the next failure shows what actually happened.
What I established
Error\n at startServer (...)stack. The first stack line had no message, althoughstartServeralways passes a non-empty message tonew Error. So the read was not truncated, and the cause of the empty first line is not established. The test asserted on that first line. The issue says the test failed in 6 of 7 failed runs; it failed once. In the other four runs listed in the issue (39715621460, 39713535549, 39381580281, 39144653837) it passed, and other tests failed.'error'listener on the child, so a failed spawn would also surface only as "browser did not start". Which of these happened is not established.What changed
diagnostics-service.test.ts: the test reads theserver_uncaught_exceptionevent fromdiagnostics.jsonland asserts itssummaryisFailed to start server. Is port <port> in use?, plus exit code 1, empty stdout and the[Server] Uncaught exception:label on stderr. The assertion is onetoMatchObjecton{ exitCode, stdout, stderr, summary }, so a failure prints stderr. The test no longer requires the message on stderr, because the stack's first line is not a reliable place for it.show-ui-isolation.test.ts: the "browser did not start" and "browser exited" errors include Chrome's stderr (and exit code and signal), and a spawn error rejects the hook at once.How I tested
Error\n at startServer ...) and the new test passes. Withsummarytemporarily hardcoded, the new test fails and prints the received object. Both changes were reverted.DREAMCODER_TEST_CHROMEpointing at scripts that (a) print a line and hang, (b) print and exit 3, (c) are not executable, the hook errors now showstderr: "fake chrome says hi\n",exited with code 3 ... stderr: "boom\n"andfailed to spawn: EACCES. Before, all three showedbrowser did not startwith nothing else.bun test --randomize --seed=1on both files: 10 pass. 8 more randomized runs of both files on macOS: 0 failures.Not verified
Errorwith no message on stderr) is open.