Skip to content

Commit

Permalink
Refactor page.waitForFunction
Browse files Browse the repository at this point in the history 
This refactors the parsing of the arguments out of the promise and into
the mapping layer, just as we have done for frame.waitForFunction. This
will help mitigate any issues where we try to access the goja runtime
from multiple goroutines which is not thread safe.
  • Loading branch information
@ankur22
ankur22 committed Jan 23, 2024
1 parent 8d56855 commit ec39914
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 42 deletions.
13 changes: 10 additions & 3 deletions browser/mapping.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -650,10 +650,17 @@ func mapPage(vu moduleVU, p *common.Page) mapping {
"video": p.Video,
"viewportSize": p.ViewportSize,
"waitForEvent": p.WaitForEvent,
"waitForFunction": func(pageFunc, opts goja.Value, args ...goja.Value) *goja.Promise {
"waitForFunction": func(pageFunc, opts goja.Value, args ...goja.Value) (*goja.Promise, error) {
js, popts, pargs, err := parseWaitForFunctionArgs(
vu.Context(), p.Timeout(), pageFunc, opts, args...,
)
if err != nil {
return nil, fmt.Errorf("page waitForFunction: %w", err)
}

return k6ext.Promise(vu.Context(), func() (result any, reason error) {
return p.WaitForFunction(pageFunc, opts, args...) //nolint:wrapcheck
})
return p.WaitForFunction(js, popts, pargs...) //nolint:wrapcheck
}), nil
},
"waitForLoadState": p.WaitForLoadState,
"waitForNavigation": func(opts goja.Value) *goja.Promise {
Expand Down
22 changes: 2 additions & 20 deletions common/page.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1280,27 +1280,9 @@ func (p *Page) WaitForEvent(event string, optsOrPredicate goja.Value) any {
}

// WaitForFunction waits for the given predicate to return a truthy value.
func (p *Page) WaitForFunction(fn, opts goja.Value, args ...goja.Value) (any, error) {
func (p *Page) WaitForFunction(js string, opts *FrameWaitForFunctionOptions, jsArgs ...any) (any, error) {
p.logger.Debugf("Page:WaitForFunction", "sid:%v", p.sessionID())

popts := NewFrameWaitForFunctionOptions(p.Timeout())
err := popts.Parse(p.ctx, opts)
if err != nil {
return nil, fmt.Errorf("parsing waitForFunction options: %w", err)
}

js := fn.ToString().String()
_, isCallable := goja.AssertFunction(fn)
if !isCallable {
js = fmt.Sprintf("() => (%s)", js)
}

jsArgs := make([]any, 0, len(args))
for _, a := range args {
jsArgs = append(jsArgs, a.Export())
}

return p.frameManager.MainFrame().WaitForFunction(js, popts, jsArgs...)
return p.frameManager.MainFrame().WaitForFunction(js, opts, jsArgs...)
}

// WaitForLoadState waits for the specified page life cycle event.
Expand Down
43 changes: 24 additions & 19 deletions tests/page_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/grafana/xk6-browser/browser"
"github.com/grafana/xk6-browser/common"
)

Expand Down Expand Up @@ -522,17 +523,17 @@ func TestPageWaitForFunction(t *testing.T) {
// testing the polling functionality—not the response from
// waitForFunction.
script := `
let resp = page.waitForFunction(%s, %s, %s)
let resp = await page.waitForFunction(%s, %s, %s)
log('ok: '+resp);`

t.Run("ok_func_raf_default", func(t *testing.T) {
t.Parallel()

tb := newTestBrowser(t)
p := tb.NewPage(nil)
mp := browser.MapPage(tb.vu, tb.NewPage(nil))
var log []string
require.NoError(t, tb.runtime().Set("log", func(s string) { log = append(log, s) }))
require.NoError(t, tb.runtime().Set("page", p))
require.NoError(t, tb.runtime().Set("page", mp))

_, err := tb.runJavaScript(`fn = () => {
if (typeof window._cnt == 'undefined') window._cnt = 0;
Expand All @@ -542,7 +543,7 @@ func TestPageWaitForFunction(t *testing.T) {
}`)
require.NoError(t, err)

_, err = tb.runJavaScript(script, "fn", "{}", "null")
err = assertJSInEventLoop(t, tb.vu, fmt.Sprintf(script, "fn", "{}", "null"))
require.NoError(t, err)
assert.Contains(t, log, "ok: null")
})
Expand All @@ -552,7 +553,8 @@ func TestPageWaitForFunction(t *testing.T) {

tb := newTestBrowser(t)
p := tb.NewPage(nil)
require.NoError(t, tb.runtime().Set("page", p))
mp := browser.MapPage(tb.vu, p)
require.NoError(t, tb.runtime().Set("page", mp))
var log []string
require.NoError(t, tb.runtime().Set("log", func(s string) { log = append(log, s) }))

Expand All @@ -563,7 +565,7 @@ func TestPageWaitForFunction(t *testing.T) {
require.NoError(t, err)

arg := "raf_arg"
_, err = tb.runJavaScript(script, "fn", "{}", fmt.Sprintf("%q", arg))
err = assertJSInEventLoop(t, tb.vu, fmt.Sprintf(script, "fn", "{}", fmt.Sprintf("%q", arg)))
require.NoError(t, err)
assert.Contains(t, log, "ok: null")

Expand All @@ -579,7 +581,8 @@ func TestPageWaitForFunction(t *testing.T) {

tb := newTestBrowser(t)
p := tb.NewPage(nil)
require.NoError(t, tb.runtime().Set("page", p))
mp := browser.MapPage(tb.vu, p)
require.NoError(t, tb.runtime().Set("page", mp))
var log []string
require.NoError(t, tb.runtime().Set("log", func(s string) { log = append(log, s) }))

Expand All @@ -593,7 +596,7 @@ func TestPageWaitForFunction(t *testing.T) {
argsJS, err := json.Marshal(args)
require.NoError(t, err)

_, err = tb.runJavaScript(script, "fn", "{}", fmt.Sprintf("...%s", string(argsJS)))
err = assertJSInEventLoop(t, tb.vu, fmt.Sprintf(script, "fn", "{}", fmt.Sprintf("...%s", string(argsJS))))
require.NoError(t, err)
assert.Contains(t, log, "ok: null")

Expand All @@ -608,25 +611,25 @@ func TestPageWaitForFunction(t *testing.T) {
t.Parallel()

tb := newTestBrowser(t)
p := tb.NewPage(nil)
mp := browser.MapPage(tb.vu, tb.NewPage(nil))
rt := tb.vu.Runtime()
var log []string
require.NoError(t, rt.Set("log", func(s string) { log = append(log, s) }))
require.NoError(t, rt.Set("page", p))
require.NoError(t, rt.Set("page", mp))

_, err := tb.runJavaScript(script, "false", "{ polling: 'raf', timeout: 500, }", "null")
err := assertJSInEventLoop(t, tb.vu, fmt.Sprintf(script, "false", "{ polling: 'raf', timeout: 500, }", "null"))
require.ErrorContains(t, err, "timed out after 500ms")
})

t.Run("err_wrong_polling", func(t *testing.T) {
t.Parallel()

tb := newTestBrowser(t)
p := tb.NewPage(nil)
mp := browser.MapPage(tb.vu, tb.NewPage(nil))
rt := tb.vu.Runtime()
require.NoError(t, rt.Set("page", p))
require.NoError(t, rt.Set("page", mp))

_, err := tb.runJavaScript(script, "false", "{ polling: 'blah' }", "null")
err := assertJSInEventLoop(t, tb.vu, fmt.Sprintf(script, "false", "{ polling: 'blah' }", "null"))
require.Error(t, err)
assert.Contains(t, err.Error(),
`parsing waitForFunction options: wrong polling option value:`,
Expand All @@ -638,7 +641,8 @@ func TestPageWaitForFunction(t *testing.T) {

tb := newTestBrowser(t)
p := tb.NewPage(nil)
require.NoError(t, tb.runtime().Set("page", p))
mp := browser.MapPage(tb.vu, p)
require.NoError(t, tb.runtime().Set("page", mp))
var log []string
require.NoError(t, tb.runtime().Set("log", func(s string) { log = append(log, s) }))

Expand All @@ -651,15 +655,15 @@ func TestPageWaitForFunction(t *testing.T) {
}`))

script := `
let resp = page.waitForFunction(%s, %s, %s);
let resp = await page.waitForFunction(%s, %s, %s);
if (resp) {
log('ok: '+resp.innerHTML());
} else {
log('err: '+err);
}`

s := fmt.Sprintf(script, `"document.querySelector('h1')"`, "{ polling: 100, timeout: 2000, }", "null")
_, err := tb.runJavaScript(s)
err := assertJSInEventLoop(t, tb.vu, s)
require.NoError(t, err)
assert.Contains(t, log, "ok: Hello")
})
Expand All @@ -669,7 +673,8 @@ func TestPageWaitForFunction(t *testing.T) {

tb := newTestBrowser(t)
p := tb.NewPage(nil)
require.NoError(t, tb.runtime().Set("page", p))
mp := browser.MapPage(tb.vu, p)
require.NoError(t, tb.runtime().Set("page", mp))
var log []string
require.NoError(t, tb.runtime().Set("log", func(s string) { log = append(log, s) }))

Expand All @@ -687,7 +692,7 @@ func TestPageWaitForFunction(t *testing.T) {
}`))

s := fmt.Sprintf(script, "fn", "{ polling: 'mutation', timeout: 2000, }", "null")
_, err = tb.runJavaScript(s)
err = assertJSInEventLoop(t, tb.vu, s)
require.NoError(t, err)
assert.Contains(t, log, "ok: null")
})
Expand Down

0 comments on commit ec39914

Please sign in to comment.