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.

Sign up for GitHub

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

browser: fix possible races #4528

Merged
merged 6 commits into from
Feb 10, 2025
Merged

browser: fix possible races #4528

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e3eb8e5
browser: partially get rid of sobek.Values usages in selectOption
olegbespalov Feb 10, 2025
f868a5f
browser: get rid of sobek.Values usages in SetChecked
olegbespalov Feb 10, 2025
631853c
browser: partially get rid of sobek.Values usages in SetInputFiles
olegbespalov Feb 10, 2025
036442f
browser: get rid of sobek.Values usages in TextContent
olegbespalov Feb 10, 2025
743d2f3
chore: omit cyclop in mapFrame
olegbespalov Feb 10, 2025
5a802dd
chore: omit nilnil in textContent
olegbespalov Feb 10, 2025
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
Next Next commit
browser: partially get rid of sobek.Values usages in selectOption
  • Loading branch information
@olegbespalov
olegbespalov committed Feb 10, 2025
commit e3eb8e5f5a4a6e2bc478bb4f59247841be0e7cd0
12 changes: 9 additions & 3 deletions internal/js/modules/k6/browser/browser/frame_mapping.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,10 +193,16 @@ func mapFrame(vu moduleVU, f *common.Frame) mapping {
return nil, f.Press(selector, key, opts) //nolint:wrapcheck
})
},
"selectOption": func(selector string, values sobek.Value, opts sobek.Value) *sobek.Promise {
"selectOption": func(selector string, values sobek.Value, opts sobek.Value) (*sobek.Promise, error) {
popts := common.NewFrameSelectOptionOptions(f.Timeout())
if err := popts.Parse(vu.Context(), opts); err != nil {
return nil, fmt.Errorf("parsing select option options: %w", err)
}

// TODO: don't use sobek Values in a separate goroutine: finish migrating values
return k6ext.Promise(vu.Context(), func() (any, error) {
return f.SelectOption(selector, values, opts) //nolint:wrapcheck
})
return f.SelectOption(selector, values, popts) //nolint:wrapcheck
}), nil
},
"setChecked": func(selector string, checked bool, opts sobek.Value) *sobek.Promise {
return k6ext.Promise(vu.Context(), func() (any, error) {
Expand Down
13 changes: 9 additions & 4 deletions internal/js/modules/k6/browser/browser/page_mapping.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,11 +276,16 @@ func mapPage(vu moduleVU, p *common.Page) mapping { //nolint:gocognit,cyclop
return &ab, nil
}), nil
},
"selectOption": func(selector string, values sobek.Value, opts sobek.Value) *sobek.Promise {
"selectOption": func(selector string, values sobek.Value, opts sobek.Value) (*sobek.Promise, error) {
popts := common.NewFrameSelectOptionOptions(p.MainFrame().Timeout())
if err := popts.Parse(vu.Context(), opts); err != nil {
return nil, fmt.Errorf("parsing select option options: %w", err)
}

// TODO: don't use sobek Values in a separate goroutine: finish migrating values
return k6ext.Promise(vu.Context(), func() (any, error) {
// TODO(@mstoykov): don't use sobek Values in a separate goroutine
return p.SelectOption(selector, values, opts) //nolint:wrapcheck
})
return p.SelectOption(selector, values, popts) //nolint:wrapcheck
}), nil
},
"setChecked": func(selector string, checked bool, opts sobek.Value) *sobek.Promise {
return k6ext.Promise(vu.Context(), func() (any, error) {
Expand Down
6 changes: 1 addition & 5 deletions internal/js/modules/k6/browser/common/frame.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1488,13 +1488,9 @@ func (f *Frame) press(selector, key string, opts *FramePressOptions) error {

// SelectOption selects the given options and returns the array of
// option values of the first element found that matches the selector.
func (f *Frame) SelectOption(selector string, values sobek.Value, opts sobek.Value) ([]string, error) {
func (f *Frame) SelectOption(selector string, values sobek.Value, popts *FrameSelectOptionOptions) ([]string, error) {
f.log.Debugf("Frame:SelectOption", "fid:%s furl:%q sel:%q", f.ID(), f.URL(), selector)

popts := NewFrameSelectOptionOptions(f.defaultTimeout())
if err := popts.Parse(f.ctx, opts); err != nil {
return nil, fmt.Errorf("parsing select option options: %w", err)
}
v, err := f.selectOption(selector, values, popts)
if err != nil {
return nil, fmt.Errorf("selecting option on %q: %w", selector, err)
Expand Down
4 changes: 2 additions & 2 deletions internal/js/modules/k6/browser/common/page.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1340,10 +1340,10 @@ func (p *Page) Screenshot(opts *PageScreenshotOptions, sp ScreenshotPersister) (

// SelectOption selects the given options and returns the array of
// option values of the first element found that matches the selector.
func (p *Page) SelectOption(selector string, values sobek.Value, opts sobek.Value) ([]string, error) {
func (p *Page) SelectOption(selector string, values sobek.Value, popts *FrameSelectOptionOptions) ([]string, error) {
p.logger.Debugf("Page:SelectOption", "sid:%v selector:%s", p.sessionID(), selector)

return p.MainFrame().SelectOption(selector, values, opts)
return p.MainFrame().SelectOption(selector, values, popts)
}

// SetContent replaces the entire HTML document content.
Expand Down
4 changes: 2 additions & 2 deletions internal/js/modules/k6/browser/tests/launch_options_slowmo_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func TestBrowserOptionsSlowMo(t *testing.T) {
t.Parallel()
tb := newTestBrowser(t, withFileServer())
testPageSlowMoImpl(t, tb, func(_ *testBrowser, p *common.Page) {
_, err := p.SelectOption("select", tb.toSobekValue("foo"), nil)
_, err := p.SelectOption("select", tb.toSobekValue("foo"), common.NewFrameSelectOptionOptions(p.MainFrame().Timeout()))
require.NoError(t, err)
})
})
Expand Down Expand Up @@ -281,7 +281,7 @@ func TestBrowserOptionsSlowMo(t *testing.T) {
t.Parallel()
tb := newTestBrowser(t, withFileServer())
testFrameSlowMoImpl(t, tb, func(_ *testBrowser, f *common.Frame) {
_, err := f.SelectOption("select", tb.toSobekValue("foo"), nil)
_, err := f.SelectOption("select", tb.toSobekValue("foo"), common.NewFrameSelectOptionOptions(f.Timeout()))
require.NoError(t, err)
})
})
Expand Down