Skip to content

Fix CDP monitor test navigation readiness - #391

Merged
rgarcia merged 1 commit into
mainfrom
rgarcia/fix-cdpmonitor-navigation-race
Sep 14, 2026
Merged

rgarcia merged 1 commit into
mainfrom
rgarcia/fix-cdpmonitor-navigation-race

Conversation

@rgarcia

@rgarcia rgarcia commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Summary

  • wait for Target.createTarget navigation to commit to the expected stub URL before treating the page as ready
  • apply the same deterministic readiness condition to the lifecycle/reconnect and worker network-capture regressions
  • preserve the existing injection, worker, telemetry, and reconnect assertions without adding sleeps or operation retries

Root cause

Target.createTarget({url: stub.URL}) can return while the new target still exposes its initial about:blank document. Both tests considered document.readyState === "complete" && window.__kernelEventInjected === true sufficient, but the monitor can inject into that initial document before the requested navigation commits.

On unmodified origin/main (f33a66e), the exact CI command reproduced TestNetworkCaptureFromWorkers locally. All worker cases executed against the opaque initial document:

  • dedicated worker: '/worker.js' is not a valid URL
  • shared worker: access denied to origin 'null'
  • service worker: navigator.serviceWorker was undefined

The linked CI run also timed out at the identical predicate in TestTelemetryConnectionOwnershipAndReconnect. Requiring location.href === stub.URL + "/" prevents the initial document from satisfying readiness and synchronizes both tests on the intended committed document.

Failing run: https://github.com/kernel/kernel-images/actions/runs/34779734931/job/103785618145

Validation

From server/:

  • 10 consecutive independent invocations passed (10/10, one fresh go test process each):
    KERNEL_CDPMONITOR_CHROME_E2E=1 go test -race ./lib/cdpmonitor -run '^(TestNetworkCapture|TestTelemetryConnection)' -count=1 -v
    Runs completed from 2026-09-14T09:38:08Z through 2026-09-14T09:41:12Z.
  • focused lifecycle + workers browser regression passed:
    KERNEL_CDPMONITOR_CHROME_E2E=1 go test -race ./lib/cdpmonitor -run '^(TestTelemetryConnectionOwnershipAndReconnect|TestNetworkCaptureFromWorkers)$' -count=1 -v
  • normal race tests passed:
    go test -race ./lib/cdpmonitor ./lib/browsersurface ./lib/cdpclient
  • git diff --check passed

Checklist

  • A link to the related failure is included above.
  • The proposed change and validation are described above.

@rgarcia
rgarcia requested a review from chruffins September 14, 2026 15:02
@rgarcia
rgarcia merged commit cab1ed2 into main Sep 14, 2026
12 checks passed
@rgarcia
rgarcia deleted the rgarcia/fix-cdpmonitor-navigation-race branch September 14, 2026 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants