Skip to content

Commit e1b4ea6

Browse files
committed
fix(browser): unflake TestPageOnResponse nil panic
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 #5124
1 parent 53b5727 commit e1b4ea6

4 files changed

Lines changed: 83 additions & 7 deletions

File tree

internal/js/modules/k6/browser/k6ext/k6test/vu.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,11 @@ func (v *VU) SetVar(tb testing.TB, name string, value any) {
165165
func ToPromise(tb testing.TB, gv sobek.Value) *sobek.Promise {
166166
tb.Helper()
167167

168+
// RunAsync can return a nil value when the event loop is interrupted
169+
// (e.g. Abortf during IterStart). Guard before Export to avoid a nil
170+
// pointer panic and surface a clear assertion failure instead.
171+
require.NotNil(tb, gv, "expected a Promise, got nil sobek.Value (event loop likely interrupted)")
172+
168173
p, ok := gv.Export().(*sobek.Promise)
169174
require.True(tb, ok, "got: %T, want *sobek.Promise", gv.Export())
170175
return p
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
package k6test
2+
3+
import (
4+
"runtime"
5+
"testing"
6+
7+
"github.com/stretchr/testify/require"
8+
)
9+
10+
// recordingTB is a minimal testing.TB that records failures without
11+
// failing the surrounding real test.
12+
type recordingTB struct {
13+
testing.TB
14+
failed bool
15+
}
16+
17+
func (r *recordingTB) Helper() {}
18+
19+
func (r *recordingTB) Errorf(string, ...any) { r.failed = true }
20+
21+
func (r *recordingTB) FailNow() {
22+
r.failed = true
23+
runtime.Goexit()
24+
}
25+
26+
// TestToPromiseNilValue guards against the nil-pointer panic from #5124:
27+
// when RunAsync is interrupted (e.g. Abortf on IterStart failure), the
28+
// returned sobek.Value can be nil. ToPromise must fail the test cleanly
29+
// instead of panicking on gv.Export().
30+
func TestToPromiseNilValue(t *testing.T) {
31+
t.Parallel()
32+
33+
tb := &recordingTB{TB: t}
34+
done := make(chan struct{})
35+
go func() {
36+
defer close(done)
37+
defer func() { _ = recover() }()
38+
ToPromise(tb, nil)
39+
}()
40+
<-done
41+
42+
require.True(t, tb.failed, "ToPromise(nil) should fail the test without panicking")
43+
}

internal/js/modules/k6/browser/tests/page_test.go

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2569,8 +2569,12 @@ type response struct {
25692569
func TestPageOnResponse(t *testing.T) {
25702570
t.Parallel()
25712571

2572-
// Start and setup a webserver to test the page.on('request') handler.
2573-
tb := newTestBrowser(t, withHTTPServer())
2572+
// Skip the Go-level Chromium launch: this test only needs the HTTP server
2573+
// and drives the browser through the JS module after StartIteration.
2574+
// Launching both (newTestBrowser + IterStart) doubles Chromium processes
2575+
// and has caused flaky "error building browser on IterStart: canceled"
2576+
// failures under CI load (see #5124).
2577+
tb := newTestBrowser(t, withHTTPServer(), withSkipLaunch())
25742578

25752579
tb.withHandler("/home", func(w http.ResponseWriter, _ *http.Request) {
25762580
_, err := fmt.Fprintf(w, `<!DOCTYPE html>
@@ -2620,7 +2624,9 @@ func TestPageOnResponse(t *testing.T) {
26202624
//
26212625
// The code below is the JavaScript code that is executed in the k6 iteration.
26222626
// It will wait for all requests to be captured in returnValue, before returning.
2623-
gv, err := tb.vu.RunAsync(t, `
2627+
// Use RunPromise so a failed/interrupted RunAsync fails fast instead of
2628+
// panicking in ToPromise on a nil sobek.Value (#5124).
2629+
got := tb.vu.RunPromise(t, `
26242630
const context = await browser.newContext({locale: 'en-US', userAgent: 'some-user-agent'});
26252631
const page = await context.newPage();
26262632
@@ -2660,13 +2666,10 @@ func TestPageOnResponse(t *testing.T) {
26602666
26612667
return JSON.stringify(returnValue, null, 2);
26622668
`, tb.url("/home"))
2663-
require.NoError(t, err)
2664-
2665-
got := k6test.ToPromise(t, gv)
26662669

26672670
// Convert the result to a string and then to a slice of requests.
26682671
var responses []response
2669-
err = json.Unmarshal([]byte(got.Result().String()), &responses)
2672+
err := json.Unmarshal([]byte(got.Result().String()), &responses)
26702673
require.NoError(t, err)
26712674

26722675
// Normalize the date

internal/js/modules/k6/browser/tests/test_browser.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ type testBrowser struct {
5757
samples chan k6metrics.SampleContainer
5858
// skipClose is set by the withSkipClose option.
5959
skipClose bool
60+
// skipLaunch is set by the withSkipLaunch option.
61+
skipLaunch bool
6062
}
6163

6264
// newTestBrowser configures and launches a new chrome browser.
@@ -70,6 +72,10 @@ type testBrowser struct {
7072
// - withLogCache: enables the log cache.
7173
// - withSamples: provides a channel to receive the browser metrics.
7274
// - withSkipClose: skips closing the browser when the test finishes.
75+
// - withSkipLaunch: skips launching the Go-level browser. Use this when the
76+
// test only needs the VU + HTTP server and will build a managed browser via
77+
// StartIteration (e.g. scripts that call browser.newContext()). Launching
78+
// both browsers doubles Chromium processes and contributes to CI flakiness.
7379
func newTestBrowser(tb testing.TB, opts ...func(*testBrowser)) *testBrowser {
7480
tb.Helper()
7581

@@ -82,6 +88,13 @@ func newTestBrowser(tb testing.TB, opts ...func(*testBrowser)) *testBrowser {
8288
tbr.isBrowserTypeInitialized = true // some option require the browser type to be initialized.
8389
tbr.applyOptions(opts...) // apply post-init stage options.
8490

91+
if tbr.skipLaunch {
92+
// Keep a usable context for helpers that call testBrowser.context()
93+
// even when no Go-level browser is launched.
94+
tbr.ctx = tbr.vu.Context()
95+
return tbr
96+
}
97+
8598
b, pid, err := tbr.browserType.Launch(context.Background(), tbr.vu.Context())
8699
if err != nil {
87100
tb.Fatalf("testBrowser: %v", err)
@@ -303,6 +316,18 @@ func withSkipClose() func(*testBrowser) {
303316
return func(tb *testBrowser) { tb.skipClose = true }
304317
}
305318

319+
// withSkipLaunch skips launching the Go-level Chromium process in
320+
// newTestBrowser. Use when the test drives the browser only through the JS
321+
// module (browser.newContext / newPage) after StartIteration, and only needs
322+
// the VU + optional HTTP test server from newTestBrowser.
323+
//
324+
// example:
325+
//
326+
// b := newTestBrowser(t, withHTTPServer(), withSkipLaunch())
327+
func withSkipLaunch() func(*testBrowser) {
328+
return func(tb *testBrowser) { tb.skipLaunch = true }
329+
}
330+
306331
// GotoNewPage is a wrapper around testBrowser.NewPage and Page.Goto that fails
307332
// the test if an error occurs. Added this helper to avoid boilerplate code in tests.
308333
func (b *testBrowser) GotoNewPage(url string) *common.Page {

0 commit comments

Comments
 (0)