Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Map [Page|Frame].WaitForNavigation and fix tests #717

Merged
merged 1 commit into from
Jan 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/frame.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
2 changes: 1 addition & 1 deletion api/page.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
54 changes: 35 additions & 19 deletions browser/mapping.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
80 changes: 43 additions & 37 deletions common/frame.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion common/page.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
77 changes: 27 additions & 50 deletions tests/frame_manager_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package tests

import (
"context"
"fmt"
"net/http"
"os"
Expand All @@ -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) {
Expand Down Expand Up @@ -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)
})
}
}
Expand Down Expand Up @@ -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())
}
34 changes: 25 additions & 9 deletions tests/lifecycle_wait_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package tests

import (
"context"
"fmt"
"net/http"
"sync"
Expand All @@ -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
Expand Down Expand Up @@ -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)
},
Expand Down Expand Up @@ -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)
Expand Down
7 changes: 4 additions & 3 deletions tests/page_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down