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 fixes part6 #4536

Merged
merged 6 commits into from
Feb 11, 2025
Merged

Browser fixes part6 #4536

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
abc21a5
browser: race fix in page/frame.Check
mstoykov Feb 11, 2025
b3f2f00
browser: race fix in page.screenshot
mstoykov Feb 11, 2025
3df5119
Drop not true TODO comments
mstoykov Feb 11, 2025
b88ef89
browser: race fix in page.selectOptions and co
mstoykov Feb 11, 2025
7d88e09
browser: race fix in page.SetViewportSize
mstoykov Feb 11, 2025
4bc23d1
browser: race fix in page.setInputFiles
mstoykov Feb 11, 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
10 changes: 7 additions & 3 deletions internal/js/modules/k6/browser/browser/element_handle_mapping.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,14 @@ func mapElementHandle(vu moduleVU, eh *common.ElementHandle) mapping { //nolint:
return nil, eh.ScrollIntoViewIfNeeded(opts) //nolint:wrapcheck
})
},
"selectOption": func(values sobek.Value, opts sobek.Value) *sobek.Promise {
"selectOption": func(values sobek.Value, opts sobek.Value) (*sobek.Promise, error) {
convValues, err := common.ConvertSelectOptionValues(vu.Runtime(), values)
if err != nil {
return nil, fmt.Errorf("parsing select options values: %w", err)
}
return k6ext.Promise(vu.Context(), func() (any, error) {
return eh.SelectOption(values, opts) //nolint:wrapcheck
})
return eh.SelectOption(convValues, opts) //nolint:wrapcheck
}), nil
},
"selectText": func(opts sobek.Value) *sobek.Promise {
return k6ext.Promise(vu.Context(), func() (any, error) {
Expand Down
26 changes: 19 additions & 7 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 @@ -14,10 +14,14 @@ import (
//nolint:funlen,gocognit,cyclop
func mapFrame(vu moduleVU, f *common.Frame) mapping {
maps := mapping{
"check": func(selector string, opts sobek.Value) *sobek.Promise {
"check": func(selector string, opts sobek.Value) (*sobek.Promise, error) {
popts := common.NewFrameCheckOptions(f.Timeout())
if err := popts.Parse(vu.Context(), opts); err != nil {
return nil, fmt.Errorf("parsing new frame check options: %w", err)
}
return k6ext.Promise(vu.Context(), func() (any, error) {
return nil, f.Check(selector, opts) //nolint:wrapcheck
})
return nil, f.Check(selector, popts) //nolint:wrapcheck
}), nil
},
"childFrames": func() []mapping {
var (
Expand Down Expand Up @@ -260,9 +264,13 @@ func mapFrame(vu moduleVU, f *common.Frame) mapping {
return nil, fmt.Errorf("parsing select option options: %w", err)
}

// TODO: don't use sobek Values in a separate goroutine: finish migrating values
convValues, err := common.ConvertSelectOptionValues(vu.Runtime(), values)
if err != nil {
return nil, fmt.Errorf("parsing select options values: %w", err)
}

return k6ext.Promise(vu.Context(), func() (any, error) {
return f.SelectOption(selector, values, popts) //nolint:wrapcheck
return f.SelectOption(selector, convValues, popts) //nolint:wrapcheck
}), nil
},
"setChecked": func(selector string, checked bool, opts sobek.Value) (*sobek.Promise, error) {
Expand Down Expand Up @@ -290,9 +298,13 @@ func mapFrame(vu moduleVU, f *common.Frame) mapping {
return nil, fmt.Errorf("parsing setInputFiles options: %w", err)
}

pfiles := new(common.Files)
if err := pfiles.Parse(vu.Context(), files); err != nil {
return nil, fmt.Errorf("parsing setInputFiles parameter: %w", err)
}

return k6ext.Promise(vu.Context(), func() (any, error) {
// TODO: don't use sobek Values in a separate goroutine, complete files migration
return nil, f.SetInputFiles(selector, files, popts) //nolint:wrapcheck
return nil, f.SetInputFiles(selector, pfiles, popts) //nolint:wrapcheck
}), nil
},
"tap": func(selector string, opts sobek.Value) (*sobek.Promise, error) {
Expand Down
56 changes: 38 additions & 18 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 @@ -23,10 +23,14 @@ func mapPage(vu moduleVU, p *common.Page) mapping { //nolint:gocognit,cyclop
return nil, p.BringToFront() //nolint:wrapcheck
})
},
"check": func(selector string, opts sobek.Value) *sobek.Promise {
"check": func(selector string, opts sobek.Value) (*sobek.Promise, error) {
popts := common.NewFrameCheckOptions(p.MainFrame().Timeout())
if err := popts.Parse(vu.Context(), opts); err != nil {
return nil, fmt.Errorf("parsing new frame check options: %w", err)
}
return k6ext.Promise(vu.Context(), func() (any, error) {
return nil, p.Check(selector, opts) //nolint:wrapcheck
})
return nil, p.Check(selector, popts) //nolint:wrapcheck
}), nil
},
"click": func(selector string, opts sobek.Value) (*sobek.Promise, error) {
popts, err := parseFrameClickOptions(vu.Context(), opts, p.MainFrame().Timeout())
Expand Down Expand Up @@ -173,7 +177,6 @@ func mapPage(vu moduleVU, p *common.Page) mapping { //nolint:gocognit,cyclop
return nil, err //nolint:wrapcheck
}

// TODO(@mstoykov): don't use sobek Values in a separate goroutine - this uses the runtime in a lot of the cases
return mapResponse(vu, resp), nil
}), nil
},
Expand Down Expand Up @@ -270,7 +273,6 @@ func mapPage(vu moduleVU, p *common.Page) mapping { //nolint:gocognit,cyclop
},
"keyboard": mapKeyboard(vu, p.GetKeyboard()),
"locator": func(selector string, opts sobek.Value) *sobek.Object {
// TODO(@mstoykov): don't use sobek Values in a separate goroutine
ml := mapLocator(vu, p.Locator(selector, opts))
return rt.ToValue(ml).ToObject(rt)
},
Expand Down Expand Up @@ -316,27 +318,37 @@ func mapPage(vu moduleVU, p *common.Page) mapping { //nolint:gocognit,cyclop
return nil, fmt.Errorf("parsing page screenshot options: %w", err)
}

return k6ext.Promise(vu.Context(), func() (any, error) {
rt := vu.Runtime()
promise, res, rej := rt.NewPromise()
callback := vu.RegisterCallback()
go func() {
bb, err := p.Screenshot(popts, vu.filePersister)
if err != nil {
return nil, err //nolint:wrapcheck
callback(func() error {
return rej(err)
})
return
}

// TODO(@mstoykov): don't use sobek Values in a separate goroutine
ab := rt.NewArrayBuffer(bb)
callback(func() error {
return res(rt.NewArrayBuffer(bb))
})
}()

return &ab, nil
}), nil
return promise, nil
},
"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
convValues, err := common.ConvertSelectOptionValues(vu.Runtime(), values)
if err != nil {
return nil, fmt.Errorf("parsing select options values: %w", err)
}
return k6ext.Promise(vu.Context(), func() (any, error) {
return p.SelectOption(selector, values, popts) //nolint:wrapcheck
return p.SelectOption(selector, convValues, popts) //nolint:wrapcheck
}), nil
},
"setChecked": func(selector string, checked bool, opts sobek.Value) (*sobek.Promise, error) {
Expand Down Expand Up @@ -371,15 +383,23 @@ func mapPage(vu moduleVU, p *common.Page) mapping { //nolint:gocognit,cyclop
return nil, fmt.Errorf("parsing setInputFiles options: %w", err)
}

pfiles := new(common.Files)
if err := pfiles.Parse(vu.Context(), files); err != nil {
return nil, fmt.Errorf("parsing setInputFiles parameter: %w", err)
}

return k6ext.Promise(vu.Context(), func() (any, error) {
// TODO: don't use sobek Values in a separate goroutine, complete files migration
return nil, p.SetInputFiles(selector, files, popts) //nolint:wrapcheck
return nil, p.SetInputFiles(selector, pfiles, popts) //nolint:wrapcheck
}), nil
},
"setViewportSize": func(viewportSize sobek.Value) *sobek.Promise {
"setViewportSize": func(viewportSize sobek.Value) (*sobek.Promise, error) {
s := new(common.Size)
if err := s.Parse(vu.Context(), viewportSize); err != nil {
return nil, fmt.Errorf("parsing viewport size: %w", err)
}
return k6ext.Promise(vu.Context(), func() (any, error) {
return nil, p.SetViewportSize(viewportSize) //nolint:wrapcheck
})
return nil, p.SetViewportSize(s) //nolint:wrapcheck
}), nil
},
"tap": func(selector string, opts sobek.Value) (*sobek.Promise, error) {
popts := common.NewFrameTapOptions(p.Timeout())
Expand Down
142 changes: 68 additions & 74 deletions internal/js/modules/k6/browser/common/element_handle.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,88 +479,82 @@ func (h *ElementHandle) press(apiCtx context.Context, key string, opts KeyboardO
return nil
}

//nolint:funlen,gocognit
func (h *ElementHandle) selectOption(apiCtx context.Context, values sobek.Value) (any, error) {
convertSelectOptionValues := func(values sobek.Value) ([]any, error) {
if k6common.IsNullish(values) {
return nil, nil
}
func ConvertSelectOptionValues(rt *sobek.Runtime, values sobek.Value) ([]any, error) {
if k6common.IsNullish(values) {
return nil, nil
}

var (
opts []any
t = values.Export()
rt = h.execCtx.vu.Runtime()
)
switch values.ExportType().Kind() {
case reflect.Slice:
var sl []interface{}
if err := rt.ExportTo(values, &sl); err != nil {
return nil, fmt.Errorf("options: expected array, got %T", values)
}
var (
opts []any
t = values.Export()
)
switch values.ExportType().Kind() {
case reflect.Slice:
var sl []interface{}
if err := rt.ExportTo(values, &sl); err != nil {
return nil, fmt.Errorf("options: expected array, got %T", values)
}

for _, item := range sl {
switch item := item.(type) {
case string:
opt := SelectOption{Value: new(string)}
*opt.Value = item
opts = append(opts, &opt)
case map[string]interface{}:
opt, err := extractSelectOptionFromMap(item)
if err != nil {
return nil, err
}

opts = append(opts, opt)
default:
return nil, fmt.Errorf("options: expected string or object, got %T", item)
for _, item := range sl {
switch item := item.(type) {
case string:
opt := SelectOption{Value: new(string)}
*opt.Value = item
opts = append(opts, &opt)
case map[string]interface{}:
opt, err := extractSelectOptionFromMap(item)
if err != nil {
return nil, err
}
}
case reflect.Map:
var raw map[string]interface{}
if err := rt.ExportTo(values, &raw); err != nil {
return nil, fmt.Errorf("options: expected object, got %T", values)
}

opt, err := extractSelectOptionFromMap(raw)
if err != nil {
return nil, err
opts = append(opts, opt)
default:
return nil, fmt.Errorf("options: expected string or object, got %T", item)
}
}
case reflect.Map:
var raw map[string]interface{}
if err := rt.ExportTo(values, &raw); err != nil {
return nil, fmt.Errorf("options: expected object, got %T", values)
}

opts = append(opts, opt)
case reflect.TypeOf(&ElementHandle{}).Kind():
opts = append(opts, t.(*ElementHandle)) //nolint:forcetypeassert
case reflect.TypeOf(sobek.Object{}).Kind():
obj := values.ToObject(rt)
opt := SelectOption{}
for _, k := range obj.Keys() {
switch k {
case "value":
opt.Value = new(string)
*opt.Value = obj.Get(k).String()
case "label":
opt.Label = new(string)
*opt.Label = obj.Get(k).String()
case "index":
opt.Index = new(int64)
*opt.Index = obj.Get(k).ToInteger()
}
}
opts = append(opts, &opt)
case reflect.String:
opt := SelectOption{Value: new(string)}
*opt.Value = t.(string) //nolint:forcetypeassert
opts = append(opts, &opt)
default:
return nil, fmt.Errorf("options: unsupported type %T", values)
opt, err := extractSelectOptionFromMap(raw)
if err != nil {
return nil, err
}

return opts, nil
}
convValues, err := convertSelectOptionValues(values)
if err != nil {
return nil, err
opts = append(opts, opt)
case reflect.TypeOf(&ElementHandle{}).Kind():
opts = append(opts, t.(*ElementHandle)) //nolint:forcetypeassert
case reflect.TypeOf(sobek.Object{}).Kind():
obj := values.ToObject(rt)
opt := SelectOption{}
for _, k := range obj.Keys() {
switch k {
case "value":
opt.Value = new(string)
*opt.Value = obj.Get(k).String()
case "label":
opt.Label = new(string)
*opt.Label = obj.Get(k).String()
case "index":
opt.Index = new(int64)
*opt.Index = obj.Get(k).ToInteger()
}
}
opts = append(opts, &opt)
case reflect.String:
opt := SelectOption{Value: new(string)}
*opt.Value = t.(string) //nolint:forcetypeassert
opts = append(opts, &opt)
default:
return nil, fmt.Errorf("options: unsupported type %T", values)
}

return opts, nil
}

func (h *ElementHandle) selectOption(apiCtx context.Context, values []any) (any, error) {
fn := `
(node, injected, values) => {
return injected.selectOptions(node, values);
Expand All @@ -570,7 +564,7 @@ func (h *ElementHandle) selectOption(apiCtx context.Context, values sobek.Value)
forceCallable: true,
returnByValue: false,
}
result, err := h.evalWithScript(apiCtx, opts, fn, convValues) //nolint:asasalint
result, err := h.evalWithScript(apiCtx, opts, fn, values) //nolint:asasalint
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -1346,7 +1340,7 @@ func (h *ElementHandle) ScrollIntoViewIfNeeded(opts sobek.Value) error {
}

// SelectOption selects the options matching the given values.
func (h *ElementHandle) SelectOption(values sobek.Value, opts sobek.Value) ([]string, error) {
func (h *ElementHandle) SelectOption(values []any, opts sobek.Value) ([]string, error) {
aopts := NewElementHandleBaseOptions(h.defaultTimeout())
if err := aopts.Parse(h.ctx, opts); err != nil {
return nil, fmt.Errorf("parsing selectOption options: %w", err)
Expand Down
17 changes: 4 additions & 13 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 @@ -592,13 +592,9 @@ func (f *Frame) click(selector string, opts *FrameClickOptions) error {
}

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

popts := NewFrameCheckOptions(f.defaultTimeout())
if err := popts.Parse(f.ctx, opts); err != nil {
return fmt.Errorf("parsing new frame check options: %w", err)
}
if err := f.check(selector, popts); err != nil {
return fmt.Errorf("checking %q: %w", selector, err)
}
Expand Down Expand Up @@ -1420,7 +1416,7 @@ 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, popts *FrameSelectOptionOptions) ([]string, error) {
func (f *Frame) SelectOption(selector string, values []any, popts *FrameSelectOptionOptions) ([]string, error) {
f.log.Debugf("Frame:SelectOption", "fid:%s furl:%q sel:%q", f.ID(), f.URL(), selector)

v, err := f.selectOption(selector, values, popts)
Expand All @@ -1433,7 +1429,7 @@ func (f *Frame) SelectOption(selector string, values sobek.Value, popts *FrameSe
return v, nil
}

func (f *Frame) selectOption(selector string, values sobek.Value, opts *FrameSelectOptionOptions) ([]string, error) {
func (f *Frame) selectOption(selector string, values []any, opts *FrameSelectOptionOptions) ([]string, error) {
selectOption := func(apiCtx context.Context, handle *ElementHandle) (any, error) {
return handle.selectOption(apiCtx, values)
}
Expand Down Expand Up @@ -1500,14 +1496,9 @@ func (f *Frame) SetContent(html string, _ *FrameSetContentOptions) error {
}

// SetInputFiles sets input files for the selected element.
func (f *Frame) SetInputFiles(selector string, files sobek.Value, popts *FrameSetInputFilesOptions) error {
func (f *Frame) SetInputFiles(selector string, pfiles *Files, popts *FrameSetInputFilesOptions) error {
f.log.Debugf("Frame:SetInputFiles", "fid:%s furl:%q sel:%q", f.ID(), f.URL(), selector)

pfiles := &Files{}
if err := pfiles.Parse(f.ctx, files); err != nil {
return fmt.Errorf("parsing setInputFiles parameter: %w", err)
}

if err := f.setInputFiles(selector, pfiles, popts); err != nil {
return fmt.Errorf("setting input files on %q: %w", selector, err)
}
Expand Down
Loading
Loading