Refactor parsing of options for waitForNavigation

browser/mapping.go

@@ -423,14 +423,19 @@ func mapFrame(vu moduleVU, f *common.Frame) mapping {
 			})
 		},
 		"waitForLoadState": f.WaitForLoadState,
-		"waitForNavigation": func(opts goja.Value) *goja.Promise {
+		"waitForNavigation": func(opts goja.Value) (*goja.Promise, error) {
+			popts := common.NewFrameWaitForNavigationOptions(f.Timeout())
+			if err := popts.Parse(vu.Context(), opts); err != nil {
+				return nil, fmt.Errorf("parsing frame wait for navigation options: %w", err)
+			}
+
 			return k6ext.Promise(vu.Context(), func() (result any, reason error) {
-				resp, err := f.WaitForNavigation(opts)
+				resp, err := f.WaitForNavigation(popts)
 				if err != nil {
 					return nil, err //nolint:wrapcheck
 				}
 				return mapResponse(vu, resp), nil
-			})
+			}), nil
 		},
 		"waitForSelector": func(selector string, opts goja.Value) (mapping, error) {
 			eh, err := f.WaitForSelector(selector, opts)
@@ -629,14 +634,19 @@ func mapPage(vu moduleVU, p *common.Page) mapping {
 			})
 		},
 		"waitForLoadState": p.WaitForLoadState,
-		"waitForNavigation": func(opts goja.Value) *goja.Promise {
+		"waitForNavigation": func(opts goja.Value) (*goja.Promise, error) {
+			popts := common.NewFrameWaitForNavigationOptions(p.Timeout())
+			if err := popts.Parse(vu.Context(), opts); err != nil {
+				return nil, fmt.Errorf("parsing page wait for navigation options: %w", err)
+			}
+
 			return k6ext.Promise(vu.Context(), func() (result any, reason error) {
-				resp, err := p.WaitForNavigation(opts)
+				resp, err := p.WaitForNavigation(popts)
 				if err != nil {
 					return nil, err //nolint:wrapcheck
 				}
 				return mapResponse(vu, resp), nil
-			})
+			}), nil
 		},
 		"waitForRequest":  p.WaitForRequest,
 		"waitForResponse": p.WaitForResponse,

common/frame.go

@@ -1787,19 +1787,13 @@ func (f *Frame) WaitForLoadState(state string, opts goja.Value) {
 // WaitForNavigation waits for the given navigation lifecycle event to happen.
 //
 //nolint:funlen,cyclop
-func (f *Frame) WaitForNavigation(opts goja.Value) (*Response, error) {
+func (f *Frame) WaitForNavigation(opts *FrameWaitForNavigationOptions) (*Response, error) {
 	f.log.Debugf("Frame:WaitForNavigation",
 		"fid:%s furl:%s", f.ID(), f.URL())
 	defer f.log.Debugf("Frame:WaitForNavigation:return",
 		"fid:%s furl:%s", f.ID(), f.URL())
 
-	parsedOpts := NewFrameWaitForNavigationOptions(
-		f.manager.timeoutSettings.timeout())
-	if err := parsedOpts.Parse(f.ctx, opts); err != nil {
-		k6ext.Panic(f.ctx, "parsing wait for navigation options: %w", err)
-	}
-
-	timeoutCtx, timeoutCancel := context.WithTimeout(f.ctx, parsedOpts.Timeout)
+	timeoutCtx, timeoutCancel := context.WithTimeout(f.ctx, opts.Timeout)
 
 	navEvtCh, navEvtCancel := createWaitForEventHandler(timeoutCtx, f, []string{EventFrameNavigation},
 		func(data any) bool {
@@ -1810,7 +1804,7 @@ func (f *Frame) WaitForNavigation(opts goja.Value) (*Response, error) {
 		timeoutCtx, f, []string{EventFrameAddLifecycle},
 		func(data any) bool {
 			if le, ok := data.(FrameLifecycleEvent); ok {
-				return le.Event == parsedOpts.WaitUntil
+				return le.Event == opts.WaitUntil
 			}
 			return false
 		})
@@ -1821,7 +1815,7 @@ func (f *Frame) WaitForNavigation(opts goja.Value) (*Response, error) {
 		if err != nil {
 			e := &k6ext.UserFriendlyError{
 				Err:     err,
-				Timeout: parsedOpts.Timeout,
+				Timeout: opts.Timeout,
 			}
 			return fmt.Errorf("waiting for navigation: %w", e)
 		}
@@ -1861,7 +1855,7 @@ func (f *Frame) WaitForNavigation(opts goja.Value) (*Response, error) {
 	// A lifecycle event won't be received when navigating within the same
 	// document, so don't wait for it. The event might've also already been
 	// fired once we're here, so also skip waiting in that case.
-	if !sameDocNav && !f.hasLifecycleEventFired(parsedOpts.WaitUntil) {
+	if !sameDocNav && !f.hasLifecycleEventFired(opts.WaitUntil) {
 		select {
 		case <-lifecycleEvtCh:
 		case <-timeoutCtx.Done():

common/page.go

@@ -1307,7 +1307,7 @@ func (p *Page) WaitForLoadState(state string, opts goja.Value) {
 }
 
 // WaitForNavigation waits for the given navigation lifecycle event to happen.
-func (p *Page) WaitForNavigation(opts goja.Value) (*Response, error) {
+func (p *Page) WaitForNavigation(opts *FrameWaitForNavigationOptions) (*Response, error) {
 	p.logger.Debugf("Page:WaitForNavigation", "sid:%v", p.sessionID())
 	_, span := TraceAPICall(p.ctx, p.targetID.String(), "page.waitForNavigation")
 	defer span.End()

tests/frame_manager_test.go

@@ -42,9 +42,7 @@ func TestWaitForFrameNavigationWithinDocument(t *testing.T) {
 			require.NotNil(t, resp)
 
 			waitForNav := func() error {
-				opts := tb.toGojaValue(&common.FrameWaitForNavigationOptions{
-					Timeout: time.Duration(timeout.Milliseconds()), // interpreted as ms
-				})
+				opts := &common.FrameWaitForNavigationOptions{Timeout: timeout}
 				_, err := p.WaitForNavigation(opts)
 				return err
 			}
@@ -98,9 +96,9 @@ func TestWaitForFrameNavigation(t *testing.T) {
 	require.NoError(t, err)
 
 	waitForNav := func() error {
-		opts := tb.toGojaValue(&common.FrameWaitForNavigationOptions{
-			Timeout: 5000, // interpreted as ms
-		})
+		opts := &common.FrameWaitForNavigationOptions{
+			Timeout: 5000 * time.Millisecond,
+		}
 		_, err := p.WaitForNavigation(opts)
 		return err
 	}

tests/frame_test.go

@@ -137,7 +137,12 @@ func TestFrameNoPanicNavigateAndClickOnPageWithIFrames(t *testing.T) {
 	err = tb.run(
 		ctx,
 		func() error { return p.Click(`a[href="/iframeSignIn"]`, common.NewFrameClickOptions(p.Timeout())) },
-		func() error { _, err := p.WaitForNavigation(nil); return err },
+		func() error {
+			_, err := p.WaitForNavigation(
+				common.NewFrameWaitForNavigationOptions(p.Timeout()),
+			)
+			return err
+		},
 	)
 	require.NoError(t, err)
 

tests/lifecycle_wait_test.go

@@ -128,10 +128,10 @@ func TestLifecycleWaitForNavigation(t *testing.T) {
 				result := p.TextContent("#pingRequestText", nil)
 				assert.EqualValues(t, "Waiting... pong 10 - for loop complete", result)
 
-				opts := tb.toGojaValue(&common.FrameWaitForNavigationOptions{
-					Timeout:   1000,
+				opts := &common.FrameWaitForNavigationOptions{
+					Timeout:   1000 * time.Millisecond,
 					WaitUntil: common.LifecycleEventNetworkIdle,
-				})
+				}
 				_, err := p.WaitForNavigation(opts)
 
 				return err
@@ -167,10 +167,10 @@ func TestLifecycleWaitForNavigation(t *testing.T) {
 				tt.pingJSTextAssert(result)
 
 				waitForNav := func() error {
-					opts := tb.toGojaValue(&common.FrameWaitForNavigationOptions{
-						Timeout:   30000,
+					opts := &common.FrameWaitForNavigationOptions{
+						Timeout:   30000 * time.Millisecond,
 						WaitUntil: tt.waitUntil,
-					})
+					}
 					_, err := p.WaitForNavigation(opts)
 					return err
 				}

tests/page_test.go

@@ -737,7 +737,9 @@ func TestPageWaitForNavigationErrOnCtxDone(t *testing.T) {
 	p := b.NewPage(nil)
 	go b.cancelContext()
 	<-b.context().Done()
-	_, err := p.WaitForNavigation(nil)
+	_, err := p.WaitForNavigation(
+		common.NewFrameWaitForNavigationOptions(p.Timeout()),
+	)
 	require.ErrorContains(t, err, "canceled")
 }
 

tests/webvital_test.go

@@ -70,7 +70,12 @@ func TestWebVitalMetric(t *testing.T) {
 	err = browser.run(
 		ctx,
 		func() error { return page.Click("#clickMe", common.NewFrameClickOptions(page.Timeout())) },
-		func() error { _, err := page.WaitForNavigation(nil); return err },
+		func() error {
+			_, err := page.WaitForNavigation(
+				common.NewFrameWaitForNavigationOptions(page.Timeout()),
+			)
+			return err
+		},
 	)
 	require.NoError(t, err)