test(debugger): pin the source-map wait debugger-status promises - #706
Draft
latekvo wants to merge 1 commit into
Draft
test(debugger): pin the source-map wait debugger-status promises#706latekvo wants to merge 1 commit into
latekvo wants to merge 1 commit into
Conversation
`sourceMapReady` is a hardcoded `true`, so the await above it is the entire content of the promise the tool's description makes — "always true — waits for pending source maps before returning". Deleting the await leaves the whole package green, which means nothing would notice a status that reports readiness it never established. The blueprint drains the registry once while the service is created, so the await in the tool covers the scripts parsed after that: a Fast Refresh, or a lazily loaded bundle chunk. These tests pin the ordering rather than the literal.
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.
Found while reviewing #610 and set aside as out of scope: the code and the prose it comes from are byte-identical at that PR's merge base, and the same mutant survives there too.
The gap
debugger-statusreturnssourceMapReady: trueas a hardcoded literal (debugger-status.ts:57). The only thing that makes the literal true is the line above it:and the description sells exactly that: "sourceMapReady (always true — waits for pending source maps before returning; no-op on Chromium)".
Nothing pins it. Deleting the await:
test/debugger test/metro test/flows test/utils)So a status that reports readiness it never established would ship green.
Scope, stated honestly
This is a test-only change; the shipped behaviour is correct. The impact of a regression is narrow but real:
js-runtime-debugger.ts:214already drains the registry once while the service is being created, so the await in the tool is what covers maps registered after that point - a Fast Refresh, or a lazily loaded bundle chunk. In that window a non-awaiting status would returnsourceMapReady: truewhileSourceMapsRegistry.pendingRegistrationsis non-empty, and adebugger-inspect-elementissued straight after can miss the file:line it would otherwise resolve.I did not manufacture a user-visible failure end to end, and I am not claiming one - the finding is that the single promised behaviour of this field has no test.
The tests
Both assert the ordering, not the literal - a test that only checked
sourceMapReady === truewould pass on the mutant, since the literal is unconditional.does not resolve until the pending source-map registration settles- hands the tool awaitForPendingthat returns a deferred promise, drains several macrotask turns, asserts the tool has not returned, then releases it.reports sourceMapReady only after the wait- records["maps-settled", "status-returned"]and asserts that exact order.mainas shippedeslint
--max-warnings 0,tsc --noEmit -p tsconfig.test.jsonand prettier all clean.