Skip to content

Commit

Permalink
Refactor parse out of frame.click
Browse files Browse the repository at this point in the history
This helps avoid using the goja runtime when it comes to parsing the
options. The options parsing has been moved to the mapping layer and so
has been moved back on to the main goja thread, which will mitigate the
possibility of a panic which could occur when multiple goroutines try
to work with the goja runtime (which is not thread safe).

There is another issue to tackle more of the goja refactoring in
#1170.

The change also adds a temporary fix for page.click which will also be
refactored at a later stage.
  • Loading branch information
ankur22 committed Jan 23, 2024
1 parent f70cf5e commit 0aabdc8
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 11 deletions.
11 changes: 8 additions & 3 deletions browser/mapping.go
Original file line number Diff line number Diff line change
Expand Up @@ -327,11 +327,16 @@ func mapFrame(vu moduleVU, f *common.Frame) mapping {
}
return rt.ToValue(mcfs).ToObject(rt)
},
"click": func(selector string, opts goja.Value) *goja.Promise {
"click": func(selector string, opts goja.Value) (*goja.Promise, error) {
popts, err := parseFrameClickOptions(vu.Context(), opts, f.Timeout())
if err != nil {
return nil, err
}

return k6ext.Promise(vu.Context(), func() (any, error) {
err := f.Click(selector, opts)
err := f.Click(selector, popts)
return nil, err //nolint:wrapcheck
})
}), nil
},
"content": f.Content,
"dblclick": f.Dblclick,
Expand Down
8 changes: 2 additions & 6 deletions common/frame.go
Original file line number Diff line number Diff line change
Expand Up @@ -545,14 +545,10 @@ func (f *Frame) ChildFrames() []*Frame {
}

// Click clicks the first element found that matches selector.
func (f *Frame) Click(selector string, opts goja.Value) error {
func (f *Frame) Click(selector string, opts *FrameClickOptions) error {
f.log.Debugf("Frame:Click", "fid:%s furl:%q sel:%q", f.ID(), f.URL(), selector)

popts := NewFrameClickOptions(f.defaultTimeout())
if err := popts.Parse(f.ctx, opts); err != nil {
k6ext.Panic(f.ctx, "parsing click options %q: %w", selector, err)
}
if err := f.click(selector, popts); err != nil {
if err := f.click(selector, opts); err != nil {
return fmt.Errorf("clicking on %q: %w", selector, err)
}

Expand Down
7 changes: 6 additions & 1 deletion common/page.go
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,12 @@ func (p *Page) IsChecked(selector string, opts goja.Value) bool {
func (p *Page) Click(selector string, opts goja.Value) error {
p.logger.Debugf("Page:Click", "sid:%v selector:%s", p.sessionID(), selector)

return p.MainFrame().Click(selector, opts) //nolint:wrapcheck
popts := NewFrameClickOptions(p.defaultTimeout())
if err := popts.Parse(p.ctx, opts); err != nil {
k6ext.Panic(p.ctx, "parsing click options %q: %w", selector, err)
}

return p.MainFrame().Click(selector, popts)
}

// Close closes the page.
Expand Down
2 changes: 1 addition & 1 deletion tests/launch_options_slowmo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ func TestBrowserOptionsSlowMo(t *testing.T) {
t.Parallel()
tb := newTestBrowser(t, withFileServer())
testFrameSlowMoImpl(t, tb, func(_ *testBrowser, f *common.Frame) {
err := f.Click("button", nil)
err := f.Click("button", common.NewFrameClickOptions(f.Timeout()))
assert.NoError(t, err)
})
})
Expand Down

0 comments on commit 0aabdc8

Please sign in to comment.