fix(e2e): make the debugger assertions fail when the debugger is not connected - #704
Draft
latekvo wants to merge 1 commit into
Draft
fix(e2e): make the debugger assertions fail when the debugger is not connected#704latekvo wants to merge 1 commit into
latekvo wants to merge 1 commit into
Conversation
…connected
`assert_ok` records PASS on the exit code alone, and both of these tools
answer 200 while reporting that the debugger is not connected — so the two
assertions that exist to check the debugger chain pass over a dead session.
`debugger-status` reaches that state on main today: half-close the CDP
socket server-side and the cached service still resolves, so the tool
returns 200 with `connected:false` and `argent run` exits 0.
ground truth: {"connected":false}
shipped: ✓ debugger-status status ✓ debugger-log-registry logs
patched: ✗ debugger-status [status] expected .connected to be true, got 'false'
`debugger-log-registry` has no `connected` field on its success path in
either build, so it is gated on its own `status` discriminant instead.
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 for that review: the weaker of the two assertions is a live defect on
maintoday, the other becomes one when #610 lands.The defect
assert_ok(lib/common.sh:122-130) records PASS onRT_RC == 0and never readsRT_JSON. Both tools on lines 84 and 86 answer HTTP 200 while reporting that the debugger is not connected, so the two assertions that exist to check the debugger chain pass over a dead session.Line 84 is broken on main today. Half-close the CDP socket server-side (the socket lands in CLOSING with no
closeevent, so no terminated cascade) and the cached service still resolves:Line 86 breaks once #610 lands, which turns the unreachable-runtime 500 into a 200
not_connectedpayload. Same two lines against a build of that branch with nothing listening on the Metro port:Why these two jq paths
Measured against live tool-servers built from both
mainand #610, in both the connected and the not-connected state:debugger-status.connected=true.connected=false.connected=true.connected=falsedebugger-log-registry.connected.connected,.status="connected".status="not_connected".connectedis absent on log-registry's success path in both builds, soassert_true '.connected'would fail a healthy run - that is why the second line uses(.status // "connected"), which is"connected"on main (field absent) and on #610's success, and"not_connected"on its failure. Both assertions therefore work on main and keep working after #610.No new helper:
assert_true/assert_fieldalready exist and the harness already gateslaunch-app '.launched',restart-app '.restarted',gesture-tap '.tapped'the same way.debugger-status's.connectedwas the one flag of that shape left bare.Verification
Repro is a real tool-server per build plus the mock Metro/CDP target, driving the assertion lines lifted verbatim out of the patched phase file. The unpatched
assert_okpasses in both states - that is the mutation control. Timing note for anyone re-running it: the half-closed socket self-heals after ~30s when thewsclose timeout fires, so steps 5 and 6 have to run back to back.Deliberately not in this PR
The same sweep found ~20 sibling
assert_oksites whose tool reports failure inside a 200 payload -await-ui-element{success:false},await-screen-idle{settled:false},run-sequence{completed:0}with the error insteps[],stop-metro(whose entire body is inside atry/catchreturning{stopped:false}, soassert_okon it can never fail),chromium-cookies set{set:false},react-profiler-status{session_status:"no_react_runtime"}, and the string-returning tools that answer"No network traffic captured..."at 200. Those verdicts are read off each tool's result contract, not driven live - confirming them needs the Android and Chromium tiers - so they are left for #595, which is already rewriting this harness with the same intent.One adjacent bug worth recording while it is fresh:
50-rn-bluesky.sh:96extractsRIDwithjq '(.requests // .entries // [])[0].requestId'fromview-network-logs, which returns a formatted string, not an object.RIDis therefore always empty, line 98 is dead and line 100's skip is always taken.Merges cleanly with #595 (test-merged:
Automatic merge went well; that PR rewrites thelocal D=line above but touches neither assertion).