Repository navigation
test(cli): wait for the lookup child to die after the CLI exits - #6357
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Verdict: APPROVE · 0 blocker · 0 important · 0 nits · head 3ce5697b
Regression signal is kept: the fixture's stalled child runs setInterval forever and the CLI's execFile timeout dies with the CLI, so if killLiveLookups regressed the child would outlive the 5 s wait and the test would still fail. CI is green on this head.
Scope
Read publicNetwork.process.test.ts, processTestUtils.ts (waitForTestCondition throws on timeout; testProcessIsAlive treats Z/X as dead), publicNetwork.fixture.ts, and systemLookupInKillableChild/killLiveLookups in publicNetwork.ts. Windows render verification was still running at review time; it does not cover this test.
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.
Problem
publicNetwork.process.test.ts"leaves no system lookup running when the CLI exits mid-lookup" fails now and then under load withexpected true to be false.Cause
The CLI kills its lookup child with SIGKILL as it exits. A kill takes effect a moment after the signal is sent, and the test sampled the child's state the instant the CLI's exit was seen. On a busy machine the child was still alive at that instant. The product behaviour is right; the test asserted too early.
Fix
The test waits, up to 5 s, for the child to die (the same
waitForTestConditionthe neighbouring process tests use). It still fails if the child is never killed: the stalled lookup outlives the wait.Repro counts (2-CPU devbox, current main)
cli,coreandenginesuites together: 1 failure in 5 runs, this test.clisuite alone: 0 of 6 failed. The test file alone under 8 CPU burners: 0 of 150.clisuite alone: 0 of 6 runs failed.The failure only shows up in the three-suite shape, so there is no clean red-to-green; the evidence is the one sighting plus the cause above.