diff --git a/api/frame.go b/api/frame.go index 19d78a450..798790338 100644 --- a/api/frame.go +++ b/api/frame.go @@ -51,7 +51,7 @@ type Frame interface { URL() string WaitForFunction(pageFunc, opts goja.Value, args ...goja.Value) *goja.Promise WaitForLoadState(state string, opts goja.Value) - WaitForNavigation(opts goja.Value) *goja.Promise + WaitForNavigation(opts goja.Value) (Response, error) WaitForSelector(selector string, opts goja.Value) ElementHandle WaitForTimeout(timeout int64) } diff --git a/api/page.go b/api/page.go index e117e1f3e..5c6ab6f5c 100644 --- a/api/page.go +++ b/api/page.go @@ -72,7 +72,7 @@ type Page interface { WaitForEvent(event string, optsOrPredicate goja.Value) *goja.Promise WaitForFunction(fn, opts goja.Value, args ...goja.Value) *goja.Promise WaitForLoadState(state string, opts goja.Value) - WaitForNavigation(opts goja.Value) *goja.Promise + WaitForNavigation(opts goja.Value) (Response, error) WaitForRequest(urlOrPredicate, opts goja.Value) *goja.Promise WaitForResponse(urlOrPredicate, opts goja.Value) *goja.Promise WaitForSelector(selector string, opts goja.Value) ElementHandle diff --git a/browser/mapping.go b/browser/mapping.go index ae022f5f8..934a45fdc 100644 --- a/browser/mapping.go +++ b/browser/mapping.go @@ -287,19 +287,27 @@ func mapFrame(ctx context.Context, vu k6modules.VU, f api.Frame) mapping { mf := mapFrame(ctx, vu, f.ParentFrame()) return rt.ToValue(mf).ToObject(rt) }, - "press": f.Press, - "selectOption": f.SelectOption, - "setContent": f.SetContent, - "setInputFiles": f.SetInputFiles, - "tap": f.Tap, - "textContent": f.TextContent, - "title": f.Title, - "type": f.Type, - "uncheck": f.Uncheck, - "url": f.URL, - "waitForFunction": f.WaitForFunction, - "waitForLoadState": f.WaitForLoadState, - "waitForNavigation": f.WaitForNavigation, + "press": f.Press, + "selectOption": f.SelectOption, + "setContent": f.SetContent, + "setInputFiles": f.SetInputFiles, + "tap": f.Tap, + "textContent": f.TextContent, + "title": f.Title, + "type": f.Type, + "uncheck": f.Uncheck, + "url": f.URL, + "waitForFunction": f.WaitForFunction, + "waitForLoadState": f.WaitForLoadState, + "waitForNavigation": func(opts goja.Value) *goja.Promise { + return k6ext.Promise(ctx, func() (result any, reason error) { + resp, err := f.WaitForNavigation(opts) + if err != nil { + return nil, err //nolint:wrapcheck + } + return mapResponse(ctx, vu, resp), nil + }) + }, "waitForSelector": func(selector string, opts goja.Value) *goja.Object { eh := f.WaitForSelector(selector, opts) ehm := mapElementHandle(ctx, vu, eh) @@ -427,12 +435,20 @@ func mapPage(ctx context.Context, vu k6modules.VU, p api.Page) mapping { "waitForEvent": p.WaitForEvent, "waitForFunction": p.WaitForFunction, "waitForLoadState": p.WaitForLoadState, - "waitForNavigation": p.WaitForNavigation, - "waitForRequest": p.WaitForRequest, - "waitForResponse": p.WaitForResponse, - "waitForSelector": p.WaitForSelector, - "waitForTimeout": p.WaitForTimeout, - "workers": p.Workers, + "waitForNavigation": func(opts goja.Value) *goja.Promise { + return k6ext.Promise(ctx, func() (result any, reason error) { + resp, err := p.WaitForNavigation(opts) + if err != nil { + return nil, err //nolint:wrapcheck + } + return mapResponse(ctx, vu, resp), nil + }) + }, + "waitForRequest": p.WaitForRequest, + "waitForResponse": p.WaitForResponse, + "waitForSelector": p.WaitForSelector, + "waitForTimeout": p.WaitForTimeout, + "workers": p.Workers, } maps["$"] = func(selector string) *goja.Object { eh := p.Query(selector) diff --git a/common/frame.go b/common/frame.go index df7c89345..f1f02d1e6 100644 --- a/common/frame.go +++ b/common/frame.go @@ -1744,7 +1744,9 @@ func (f *Frame) WaitForLoadState(state string, opts goja.Value) { } // WaitForNavigation waits for the given navigation lifecycle event to happen. -func (f *Frame) WaitForNavigation(opts goja.Value) *goja.Promise { +// +//nolint:funlen,cyclop +func (f *Frame) WaitForNavigation(opts goja.Value) (api.Response, error) { f.log.Debugf("Frame:WaitForNavigation", "fid:%s furl:%s", f.ID(), f.URL()) defer f.log.Debugf("Frame:WaitForNavigation:return", @@ -1786,49 +1788,53 @@ func (f *Frame) WaitForNavigation(opts goja.Value) *goja.Promise { return nil } - return k6ext.Promise(f.ctx, func() (result any, reason error) { - defer func() { - timeoutCancel() - navEvtCancel() - lifecycleEvtCancel() - }() + defer func() { + timeoutCancel() + navEvtCancel() + lifecycleEvtCancel() + }() - var ( - resp *Response - sameDocNav bool - ) - select { - case evt := <-navEvtCh: - if e, ok := evt.(*NavigationEvent); ok { - if e.newDocument == nil { - sameDocNav = true - break - } - // request could be nil if navigating to e.g. about:blank - req := e.newDocument.request - if req != nil { - req.responseMu.RLock() - resp = req.response - req.responseMu.RUnlock() - } + var ( + resp *Response + sameDocNav bool + ) + select { + case evt := <-navEvtCh: + if e, ok := evt.(*NavigationEvent); ok { + if e.newDocument == nil { + sameDocNav = true + break } + // request could be nil if navigating to e.g. about:blank + req := e.newDocument.request + if req != nil { + req.responseMu.RLock() + resp = req.response + req.responseMu.RUnlock() + } + } + case <-timeoutCtx.Done(): + return nil, handleTimeoutError(timeoutCtx.Err()) + } + + // 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) { + select { + case <-lifecycleEvtCh: case <-timeoutCtx.Done(): return nil, handleTimeoutError(timeoutCtx.Err()) } + } - // 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) { - select { - case <-lifecycleEvtCh: - case <-timeoutCtx.Done(): - return nil, handleTimeoutError(timeoutCtx.Err()) - } - } + // Since response will be in an interface, it will never be nil, + // so we need to return nil explicitly. + if resp == nil { + return nil, nil + } - return resp, nil - }) + return resp, nil } // WaitForSelector waits for the given selector to match the waiting criteria. diff --git a/common/page.go b/common/page.go index 445d722d7..c82b4a2de 100644 --- a/common/page.go +++ b/common/page.go @@ -904,7 +904,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) *goja.Promise { +func (p *Page) WaitForNavigation(opts goja.Value) (api.Response, error) { p.logger.Debugf("Page:WaitForNavigation", "sid:%v", p.sessionID()) return p.frameManager.MainFrame().WaitForNavigation(opts) diff --git a/tests/frame_manager_test.go b/tests/frame_manager_test.go index 73cdad5e5..18a18a169 100644 --- a/tests/frame_manager_test.go +++ b/tests/frame_manager_test.go @@ -1,6 +1,7 @@ package tests import ( + "context" "fmt" "net/http" "os" @@ -11,7 +12,6 @@ import ( "github.com/stretchr/testify/require" "github.com/grafana/xk6-browser/common" - "github.com/grafana/xk6-browser/k6ext" ) func TestWaitForFrameNavigationWithinDocument(t *testing.T) { @@ -46,31 +46,20 @@ func TestWaitForFrameNavigationWithinDocument(t *testing.T) { require.NoError(t, err) require.NotNil(t, resp) - var done bool - err = tb.awaitWithTimeout(timeout, func() error { - // TODO - // remove this once we have finished our work on the mapping layer. - // for now: provide a fake promise - fakePromise := k6ext.Promise(tb.vu.Context(), func() (result any, reason error) { - return nil, nil - }) - tb.promise(fakePromise).then(func() testPromise { - opts := tb.toGojaValue(&common.FrameWaitForNavigationOptions{ - Timeout: time.Duration(timeout.Milliseconds()), // interpreted as ms - }) - waitForNav := p.WaitForNavigation(opts) - click := k6ext.Promise(tb.vu.Context(), func() (result any, reason error) { - return nil, p.Click(tc.selector, nil) - }) - return tb.promiseAll(waitForNav, click) - }).then(func() { - done = true + waitForNav := func() error { + opts := tb.toGojaValue(&common.FrameWaitForNavigationOptions{ + Timeout: time.Duration(timeout.Milliseconds()), // interpreted as ms }) - - return nil - }) + _, err := p.WaitForNavigation(opts) + return err + } + click := func() error { + return p.Click(tc.selector, nil) + } + ctx, cancel := context.WithTimeout(tb.ctx, timeout) + defer cancel() + err = tb.run(ctx, waitForNav, click) require.NoError(t, err) - require.True(t, done) }) } } @@ -111,32 +100,20 @@ func TestWaitForFrameNavigation(t *testing.T) { _, err := p.Goto(tb.URL("/first"), opts) require.NoError(t, err) - var done bool - err = tb.await(func() error { - // TODO - // remove this once we have finished our work on the mapping layer. - // for now: provide a fake promise - fakePromise := k6ext.Promise(tb.vu.Context(), func() (result any, reason error) { - return nil, nil + waitForNav := func() error { + opts := tb.toGojaValue(&common.FrameWaitForNavigationOptions{ + Timeout: 5000, // interpreted as ms }) - tb.promise(fakePromise). - then(func() testPromise { - opts := tb.toGojaValue(&common.FrameWaitForNavigationOptions{ - Timeout: 5000, // interpreted as ms - }) - waitForNav := p.WaitForNavigation(opts) - click := k6ext.Promise(tb.vu.Context(), func() (result any, reason error) { - return nil, p.Click(`a`, nil) - }) - return tb.promiseAll(waitForNav, click) - }). - then(func() { - assert.Equal(t, "Second page", p.Title()) - done = true - }) - - return nil - }) + _, err := p.WaitForNavigation(opts) + return err + } + click := func() error { + return p.Click(`a`, nil) + } + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + err = tb.run(ctx, waitForNav, click) require.NoError(t, err) - require.True(t, done) + + assert.Equal(t, "Second page", p.Title()) } diff --git a/tests/lifecycle_wait_test.go b/tests/lifecycle_wait_test.go index 323036c01..cc4ac96fd 100644 --- a/tests/lifecycle_wait_test.go +++ b/tests/lifecycle_wait_test.go @@ -1,6 +1,7 @@ package tests import ( + "context" "fmt" "net/http" "sync" @@ -15,6 +16,9 @@ import ( "github.com/grafana/xk6-browser/k6ext" ) +// TODO +// Remove the promises. We don't need them anymore. + // General guidelines on lifecycle events: // // load @@ -126,10 +130,13 @@ func TestLifecycleWaitForNavigation(t *testing.T) { result := p.TextContent("#pingRequestText", nil) assert.EqualValues(t, "Waiting... pong 10 - for loop complete", result) - waitForNav := p.WaitForNavigation(tb.toGojaValue(&common.FrameWaitForNavigationOptions{ + opts := tb.toGojaValue(&common.FrameWaitForNavigationOptions{ Timeout: 1000, WaitUntil: common.LifecycleEventNetworkIdle, - })) + }) + waitForNav := k6ext.Promise(tb.vu.Context(), func() (result any, reason error) { + return p.WaitForNavigation(opts) + }) return tb.promise(waitForNav) }, @@ -163,15 +170,24 @@ func TestLifecycleWaitForNavigation(t *testing.T) { result = p.TextContent("#pingJSText", nil) tt.pingJSTextAssert(result) - waitForNav := p.WaitForNavigation(tb.toGojaValue(&common.FrameWaitForNavigationOptions{ - Timeout: 30000, - WaitUntil: tt.waitUntil, - })) - click := k6ext.Promise(tb.vu.Context(), func() (result any, reason error) { - return nil, p.Click(`a`, nil) + waitForNav := func() error { + opts := tb.toGojaValue(&common.FrameWaitForNavigationOptions{ + Timeout: 30000, + WaitUntil: tt.waitUntil, + }) + _, err := p.WaitForNavigation(opts) + return err + } + click := func() error { + return p.Click(`a`, nil) + } + runAll := k6ext.Promise(tb.ctx, func() (result any, reason error) { + ctx, cancel := context.WithTimeout(tb.ctx, 5*time.Second) + defer cancel() + return nil, tb.run(ctx, waitForNav, click) }) - return tb.promiseAll(waitForNav, click) + return tb.promise(runAll) }, func() { result := p.TextContent("#pingRequestText", nil) tt.pingRequestTextAssert(result, 20) diff --git a/tests/page_test.go b/tests/page_test.go index bbd2932ef..0950c61f0 100644 --- a/tests/page_test.go +++ b/tests/page_test.go @@ -654,12 +654,13 @@ func TestPageWaitForLoadState(t *testing.T) { } // See: The issue #187 for details. -func TestPageWaitForNavigationShouldNotPanic(t *testing.T) { +func TestPageWaitForNavigationErrOnCtxDone(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) - p := newTestBrowser(t, withContext(ctx)).NewPage(nil) + p := newTestBrowser(t, ctx).NewPage(nil) go cancel() <-ctx.Done() - require.NotPanics(t, func() { p.WaitForNavigation(nil) }) + _, err := p.WaitForNavigation(nil) + require.ErrorContains(t, err, "canceled") } func TestPagePress(t *testing.T) {