Skip to content

fix(browser): unflake TestPageOnResponse nil panic - #6258

Open
alirezaedalat wants to merge 1 commit into
grafana:masterfrom
alirezaedalat:fix/5124-test-page-on-response-flaky
Open

fix(browser): unflake TestPageOnResponse nil panic#6258
alirezaedalat wants to merge 1 commit into
grafana:masterfrom
alirezaedalat:fix/5124-test-page-on-response-flaky

Conversation

@alirezaedalat

Copy link
Copy Markdown

Summary

  • Stop launching an unused Go-level Chromium in TestPageOnResponse via withSkipLaunch(); the test only needs the HTTP server and the managed browser from StartIteration. Double launches contributed to flaky error building browser on IterStart: canceled under CI load.
  • Harden k6test.ToPromise against a nil sobek.Value so interrupted event loops fail the test cleanly instead of panicking on Export() (the panic in TestPageOnResponse is flaky #5124).
  • Switch the test to RunPromise and add a regression unit test for the nil-value path.

Fixes #5124

Test plan

  • go test -run TestToPromiseNilValue ./internal/js/modules/k6/browser/k6ext/k6test/
  • go test -run '^TestPageOnResponse$' ./internal/js/modules/k6/browser/tests/
  • go test -count=10 -parallel 4 -run '^TestPageOnResponse$' ./internal/js/modules/k6/browser/tests/
  • CI browser tests on this PR

@alirezaedalat
alirezaedalat requested a review from a team as a code owner August 6, 2026 17:48
@alirezaedalat
alirezaedalat requested review from inancgumus and mstoykov and removed request for a team August 6, 2026 17:48
@cla-assistant

cla-assistant Bot commented Aug 6, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@cla-assistant

cla-assistant Bot commented Aug 6, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


alirezaedalat seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown

Signed commits report

1 of 1 commit between master and fix/5124-test-page-on-response-flaky could not be fully verified:

Commit Author Reason Message
e1b4ea67 alirezaedalat unsigned fix(browser): unflake TestPageOnResponse nil panic

This repository requires all commits to be signed. See GitHub docs on commit signature verification.

@alirezaedalat
alirezaedalat force-pushed the fix/5124-test-page-on-response-flaky branch from 4d0f395 to 2b309c4 Compare August 7, 2026 04:40
Avoid launching an unused Go-level Chromium in TestPageOnResponse so
StartIteration is the only browser build, reducing CI IterStart
cancellations. Harden ToPromise against nil sobek values and use
RunPromise so interrupted runs fail cleanly instead of panicking.

Fixes grafana#5124
@alirezaedalat
alirezaedalat force-pushed the fix/5124-test-page-on-response-flaky branch from 2b309c4 to e1b4ea6 Compare August 7, 2026 04:42
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.

TestPageOnResponse is flaky

1 participant