Refactor page.waitForLoadState to return err

grafana · May 29, 2024 · 80bf1cb · 80bf1cb
1 parent 0dce501
commit 80bf1cb
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 26 deletions.
diff --git a/browser/mapping_test.go b/browser/mapping_test.go
@@ -350,7 +350,7 @@ type pageAPI interface {
 	URL() (string, error)
 	ViewportSize() map[string]float64
 	WaitForFunction(fn, opts goja.Value, args ...goja.Value) (any, error)
-	WaitForLoadState(state string, opts goja.Value)
+	WaitForLoadState(state string, opts goja.Value) error
 	WaitForNavigation(opts goja.Value) (*common.Response, error)
 	WaitForSelector(selector string, opts goja.Value) (*common.ElementHandle, error)
 	WaitForTimeout(timeout int64)
@@ -412,7 +412,7 @@ type frameAPI interface {
 	Uncheck(selector string, opts goja.Value) error
 	URL() string
 	WaitForFunction(pageFunc, opts goja.Value, args ...goja.Value) (any, error)
-	WaitForLoadState(state string, opts goja.Value)
+	WaitForLoadState(state string, opts goja.Value) error
 	WaitForNavigation(opts goja.Value) (*common.Response, error)
 	WaitForSelector(selector string, opts goja.Value) (*common.ElementHandle, error)
 	WaitForTimeout(timeout int64)

diff --git a/common/frame.go b/common/frame.go
@@ -1809,45 +1809,48 @@ func (f *Frame) waitForFunction(
 
 // WaitForLoadState waits for the given load state to be reached.
 // This will unblock if that lifecycle event has already been received.
-func (f *Frame) WaitForLoadState(state string, opts goja.Value) {
+func (f *Frame) WaitForLoadState(state string, opts goja.Value) error {
 	f.log.Debugf("Frame:WaitForLoadState", "fid:%s furl:%q state:%s", f.ID(), f.URL(), state)
 	defer f.log.Debugf("Frame:WaitForLoadState:return", "fid:%s furl:%q state:%s", f.ID(), f.URL(), state)
 
-	parsedOpts := NewFrameWaitForLoadStateOptions(f.defaultTimeout())
-	err := parsedOpts.Parse(f.ctx, opts)
-	if err != nil {
-		k6ext.Panic(f.ctx, "parsing waitForLoadState %q options: %v", state, err)
+	waitForLoadStateOpts := NewFrameWaitForLoadStateOptions(f.defaultTimeout())
+	if err := waitForLoadStateOpts.Parse(f.ctx, opts); err != nil {
+		return fmt.Errorf("parsing waitForLoadState %q options: %w", state, err)
 	}
 
-	timeoutCtx, timeoutCancel := context.WithTimeout(f.ctx, parsedOpts.Timeout)
+	timeoutCtx, timeoutCancel := context.WithTimeout(f.ctx, waitForLoadStateOpts.Timeout)
 	defer timeoutCancel()
 
 	waitUntil := LifecycleEventLoad
 	if state != "" {
-		if err = waitUntil.UnmarshalText([]byte(state)); err != nil {
-			k6ext.Panic(f.ctx, "waiting for load state: %v", err)
+		if err := waitUntil.UnmarshalText([]byte(state)); err != nil {
+			return fmt.Errorf("unmarshaling wait for load state %q: %w", state, err)
 		}
 	}
 
-	lifecycleEvtCh, lifecycleEvtCancel := createWaitForEventPredicateHandler(
-		timeoutCtx, f, []string{EventFrameAddLifecycle},
+	lifecycleEvent, lifecycleEventCancel := createWaitForEventPredicateHandler(
+		timeoutCtx,
+		f,
+		[]string{EventFrameAddLifecycle},
 		func(data any) bool {
 			if le, ok := data.(FrameLifecycleEvent); ok {
 				return le.Event == waitUntil
 			}
 			return false
 		})
-	defer lifecycleEvtCancel()
+	defer lifecycleEventCancel()
 
 	if f.hasLifecycleEventFired(waitUntil) {
-		return
+		return nil
 	}
 
 	select {
-	case <-lifecycleEvtCh:
+	case <-lifecycleEvent:
 	case <-timeoutCtx.Done():
-		k6ext.Panic(f.ctx, "waiting for load state %q: %v", state, err)
+		return fmt.Errorf("waiting for load state %q: %w", state, timeoutCtx.Err())
 	}
+
+	return nil
 }
 
 // WaitForNavigation waits for the given navigation lifecycle event to happen.

diff --git a/common/page.go b/common/page.go
@@ -1305,10 +1305,10 @@ func (p *Page) WaitForFunction(js string, opts *FrameWaitForFunctionOptions, jsA
 }
 
 // WaitForLoadState waits for the specified page life cycle event.
-func (p *Page) WaitForLoadState(state string, opts goja.Value) {
+func (p *Page) WaitForLoadState(state string, opts goja.Value) error {
 	p.logger.Debugf("Page:WaitForLoadState", "sid:%v state:%q", p.sessionID(), state)
 
-	p.frameManager.MainFrame().WaitForLoadState(state, opts)
+	return p.frameManager.MainFrame().WaitForLoadState(state, opts)
 }
 
 // WaitForNavigation waits for the given navigation lifecycle event to happen.

diff --git a/tests/lifecycle_wait_test.go b/tests/lifecycle_wait_test.go
@@ -278,7 +278,8 @@ func TestLifecycleWaitForLoadState(t *testing.T) {
 			pingJSSlow:   false,
 			waitUntil:    common.LifecycleEventDOMContentLoad,
 			assertFunc: func(p *common.Page) {
-				p.WaitForLoadState(common.LifecycleEventNetworkIdle.String(), nil)
+				err := p.WaitForLoadState(common.LifecycleEventNetworkIdle.String(), nil)
+				require.NoError(t, err)
 
 				result, err := p.TextContent("#pingRequestText", nil)
 				require.NoError(t, err)
@@ -321,9 +322,7 @@ func TestLifecycleWaitForLoadState(t *testing.T) {
 				tt.pingJSTextAssert(result)
 
 				// This shouldn't block and return after calling hasLifecycleEventFired.
-				p.WaitForLoadState(tt.waitUntil.String(), nil)
-
-				return nil
+				return p.WaitForLoadState(tt.waitUntil.String(), nil)
 			}, nil, "")
 		})
 	}

diff --git a/tests/page_test.go b/tests/page_test.go
@@ -865,10 +865,9 @@ func TestPageWaitForLoadState(t *testing.T) {
 		t.Parallel()
 
 		tb := newTestBrowser(t)
-		assertExceptionContains(t, tb.runtime(), func() {
-			p := tb.NewPage(nil)
-			p.WaitForLoadState("none", nil)
-		}, `invalid lifecycle event: "none"; must be one of: load, domcontentloaded, networkidle`)
+		p := tb.NewPage(nil)
+		err := p.WaitForLoadState("none", nil)
+		require.ErrorContains(t, err, `invalid lifecycle event: "none"; must be one of: load, domcontentloaded, networkidle`)
 	})
 }