8000 browser: race fixes by mstoykov · Pull Request #4523 · grafana/k6 · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

browser: race fixes #4523

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

Merged
merged 1 commit into from
Feb 10, 2025
Merged

browser: race fixes #4523

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
85 changes: 59 additions & 26 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 @@ -45,18 +45,23 @@ func mapFrame(vu moduleVU, f *common.Frame) mapping {
return f.Content() //nolint:wrapcheck
})
},
"dblclick": func(selector string, opts sobek.Value) *sobek.Promise {
"dblclick": func(selector string, opts sobek.Value) (*sobek.Promise, error) {
popts := common.NewFrameDblClickOptions(f.Timeout())
Copy link
Contributor
@codebien codebien Feb 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just as a comment, in this scope, what is the meaning of pops? Parsed options? It sounds a bit ambiguous to me. We should change to something else where it is or Frame Options or the specific DB Options, which converts to fopts or dbopts. I prefer the latter.

Of course, all of this, not in this pull request.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is parsed options and was what was used in the code that we pull out so it isn't in a separate goroutine.

if err := popts.Parse(vu.Context(), opts); err != nil {
return nil, fmt.Errorf("parsing double click options: %w", err)
}
return k6ext.Promise(vu.Context(), func() (any, error) {
return nil, f.Dblclick(selector, opts) //nolint:wrapcheck
})
return nil, f.Dblclick(selector, popts) //nolint:wrapcheck
}), nil
},
"dispatchEvent": func(selector, typ string, eventInit, opts sobek.Value) (*sobek.Promise, error) {
popts := common.NewFrameDispatchEventOptions(f.Timeout())
if err := popts.Parse(vu.Context(), opts); err != nil {
return nil, fmt.Errorf("parsing frame dispatch event options: %w", err)
}
earg := exportArg(eventInit)
return k6ext.Promise(vu.Context(), func() (any, error) {
return nil, f.DispatchEvent(selector, typ, exportArg(eventInit), popts) //nolint:wrapcheck
return nil, f.DispatchEvent(selector, typ, earg, popts) //nolint:wrapcheck
}), nil
},
"evaluate": func(pageFunc sobek.Value, gargs ...sobek.Value) (*sobek.Promise, error) {
Expand All @@ -79,15 +84,23 @@ func mapFrame(vu moduleVU, f *common.Frame) mapping {
return mapJSHandle(vu, jsh), nil
}), nil
},
"fill": func(selector, value string, opts sobek.Value) *sobek.Promise {
"fill": func(selector, value string, opts sobek.Value) (*sobek.Promise, error) {
popts := common.NewFrameFillOptions(f.Timeout())
if err := popts.Parse(vu.Context(), opts); err != nil {
return nil, fmt.Errorf("parsing fill options: %w", err)
}
return k6ext.Promise(vu.Context(), func() (any, error) {
return nil, f.Fill(selector, value, opts) //nolint:wrapcheck
})
return nil, f.Fill(selector, value, popts) //nolint:wrapcheck
}), nil
},
"focus": func(selector string, opts sobek.Value) *sobek.Promise {
"focus": func(selector string, opts sobek.Value) (*sobek.Promise, error) {
popts := common.NewFrameBaseOptions(f.Timeout())
if err := popts.Parse(vu.Context(), opts); err != nil {
return nil, fmt.Errorf("parsing focus options: %w", err)
}
return k6ext.Promise(vu.Context(), func() (any, error) {
return nil, f.Focus(selector, opts) //nolint:wrapcheck
})
return nil, f.Focus(selector, popts) //nolint:wrapcheck
}), nil
},
"frameElement": func() *sobek.Promise {
return k6ext.Promise(vu.Context(), func() (any, error) {
Expand All @@ -98,17 +111,21 @@ func mapFrame(vu moduleVU, f *common.Frame) mapping {
return mapElementHandle(vu, fe), nil
})
},
"getAttribute": func(selector, name string, opts sobek.Value) *sobek.Promise {
"getAttribute": func(selector, name string, opts sobek.Value) (*sobek.Promise, error) {
popts := common.NewFrameBaseOptions(f.Timeout())
if err := popts.Parse(vu.Context(), opts); err != nil {
return nil, fmt.Errorf("parsing getAttribute options: %w", err)
}
return k6ext.Promise(vu.Context(), func() (any, error) {
s, ok, err := f.GetAttribute(selector, name, opts)
s, ok, err := f.GetAttribute(selector, name, popts)
if err != nil {
return nil, err //nolint:wrapcheck
}
if !ok {
return nil, nil
return nil, nil //nolint:nilnil
}
return s, nil
})
}), nil
},
"goto": func(url string, opts sobek.Value) (*sobek.Promise, error) {
gopts := common.NewFrameGotoOptions(
Expand All @@ -127,25 +144,41 @@ func mapFrame(vu moduleVU, f *common.Frame) mapping {
return mapResponse(vu, resp), nil
}), nil
},
"hover": func(selector string, opts sobek.Value) *sobek.Promise {
"hover": func(selector string, opts sobek.Value) (*sobek.Promise, error) {
popts := common.NewFrameHoverOptions(f.Timeout())
if err := popts.Parse(vu.Context(), opts); err != nil {
return nil, fmt.Errorf("parsing hover options: %w", err)
}
return k6ext.Promise(vu.Context(), func() (any, error) {
return nil, f.Hover(selector, opts) //nolint:wrapcheck
})
return nil, f.Hover(selector, popts) //nolint:wrapcheck
}), nil
},
"innerHTML": func(selector string, opts sobek.Value) *sobek.Promise {
"innerHTML": func(selector string, opts sobek.Value) (*sobek.Promise, error) {
popts := common.NewFrameInnerHTMLOptions(f.Timeout())
if err := popts.Parse(vu.Context(), opts); err != nil {
return nil, fmt.Errorf("parsing inner HTML options: %w", err)
}
return k6ext.Promise(vu.Context(), func() (any, error) {
return f.InnerHTML(selector, opts) //nolint:wrapcheck
})
return f.InnerHTML(selector, popts) //nolint:wrapcheck
}), nil
},
"innerText": func(selector string, opts sobek.Value) *sobek.Promise {
"innerText": func(selector string, opts sobek.Value) (*sobek.Promise, error) {
popts := common.NewFrameInnerTextOptions(f.Timeout())
if err := popts.Parse(vu.Context(), opts); err != nil {
return nil, fmt.Errorf("parsing inner text options: %w", err)
}
return k6ext.Promise(vu.Context(), func() (any, error) {
return f.InnerText(selector, opts) //nolint:wrapcheck
})
return f.InnerText(selector, popts) //nolint:wrapcheck
}), nil
},
"inputValue": func(selector string, opts sobek.Value) *sobek.Promise {
"inputValue": func(selector string, opts sobek.Value) (*sobek.Promise, error) {
popts := common.NewFrameInputValueOptions(f.Timeout())
if err := popts.Parse(vu.Context(), opts); err != nil {
return nil, fmt.Errorf("parsing input value options: %w", err)
}
return k6ext.Promise(vu.Context(), func() (any, error) {
return f.InputValue(selector, opts) //nolint:wrapcheck
})
return f.InputValue(selector, popts) //nolint:wrapcheck
}), nil
},
"isChecked": func(selector string, opts sobek.Value) *sobek.Promise {
return k6ext.Promise(vu.Context(), func() (any, error) {
Expand Down
114 changes: 72 additions & 42 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 @@ -29,7 +29,7 @@ func mapPage(vu moduleVU, p *common.Page) mapping { //nolint:gocognit,cyclop
})
},
"click": func(selector string, opts sobek.Value) (*sobek.Promise, error) {
popts, err := parseFrameClickOptions(vu.Context(), opts, p.Timeout())
popts, err := parseFrameClickOptions(vu.Context(), opts, p.MainFrame().Timeout())
if err != nil {
return nil, err
}
Expand All @@ -40,13 +40,14 @@ func mapPage(vu moduleVU, p *common.Page) mapping { //nolint:gocognit,cyclop
}), nil
},
"close": func(opts sobek.Value) *sobek.Promise {
// TODO when opts are implemented for this function pares them here before calling k6ext.Promise and doing it
// in a goroutine off the event loop. As that will race with anything running on the event loop.
return k6ext.Promise(vu.Context(), func() (any, error) {
// It's safe to close the taskqueue for this targetID (if one
// exists).
vu.taskQueueRegistry.close(p.TargetID())

// TODO(@mstoykov): don't use sobek Values in a separate goroutine
return nil, p.Close(opts) //nolint:wrapcheck
return nil, p.Close() //nolint:wrapcheck
})
},
"content": func() *sobek.Promise {
Expand All @@ -57,26 +58,33 @@ func mapPage(vu moduleVU, p *common.Page) mapping { //nolint:gocognit,cyclop
"context": func() mapping {
return mapBrowserContext(vu, p.Context())
},
"dblclick": func(selector string, opts sobek.Value) *sobek.Promise {
"dblclick": func(selector string, opts sobek.Value) (*sobek.Promise, error) {
popts := common.NewFrameDblClickOptions(p.MainFrame().Timeout())
if err := popts.Parse(vu.Context(), opts); err != nil {
return nil, fmt.Errorf("parsing double click options: %w", err)
}
return k6ext.Promise(vu.Context(), func() (any, error) {
// TODO(@mstoykov): don't use sobek Values in a separate goroutine
return nil, p.Dblclick(selector, opts) //nolint:wrapcheck
})
return nil, p.Dblclick(selector, popts) //nolint:wrapcheck
}), nil
},
"dispatchEvent": func(selector, typ string, eventInit, opts sobek.Value) (*sobek.Promise, error) {
popts := common.NewFrameDispatchEventOptions(p.Timeout())
popts := common.NewFrameDispatchEventOptions(p.MainFrame().Timeout())
if err := popts.Parse(vu.Context(), opts); err != nil {
return nil, fmt.Errorf("parsing page dispatch event options: %w", err)
}
earg := exportArg(eventInit)
return k6ext.Promise(vu.Context(), func() (any, error) {
return nil, p.DispatchEvent(selector, typ, exportArg(eventInit), popts) //nolint:wrapcheck
return nil, p.DispatchEvent(selector, typ, earg, popts) //nolint:wrapcheck
}), nil
},
"emulateMedia": func(opts sobek.Value) *sobek.Promise {
"emulateMedia": func(opts sobek.Value) (*sobek.Promise, error) {
popts := common.NewPageEmulateMediaOptions(p)
if err := popts.Parse(vu.Context(), opts); err != nil {
return nil, fmt.Errorf("parsing emulateMedia options: %w", err)
}
return k6ext.Promise(vu.Context(), func() (any, error) {
// TODO(@mstoykov): don't use sobek Values in a separate goroutine
return nil, p.EmulateMedia(opts) //nolint:wrapcheck
})
return nil, p.EmulateMedia(popts) //nolint:wrapcheck
}), nil
},
"emulateVisionDeficiency": func(typ string) *sobek.Promise {
return k6ext.Promise(vu.Context(), func() (any, error) {
Expand Down Expand Up @@ -108,17 +116,23 @@ func mapPage(vu moduleVU, p *common.Page) mapping { //nolint:gocognit,cyclop
return mapJSHandle(vu, jsh), nil
}), nil
},
"fill": func(selector string, value string, opts sobek.Value) *sobek.Promise {
"fill": func(selector string, value string, opts sobek.Value) (*sobek.Promise, error) {
popts := common.NewFrameFillOptions(p.MainFrame().Timeout())
if err := popts.Parse(vu.Context(), opts); err != nil {
return nil, fmt.Errorf("parsing fill options: %w", err)
}
return k6ext.Promise(vu.Context(), func() (any, error) {
// TODO(@mstoykov): don't use sobek Values in a separate goroutine
return nil, p.Fill(selector, value, opts) //nolint:wrapcheck
})
return nil, p.Fill(selector, value, popts) //nolint:wrapcheck
}), nil
},
"focus": func(selector string, opts sobek.Value) *sobek.Promise {
"focus": func(selector string, opts sobek.Value) (*sobek.Promise, error) {
popts := common.NewFrameBaseOptions(p.MainFrame().Timeout())
if err := popts.Parse(vu.Context(), opts); err != nil {
return nil, fmt.Errorf("parsing focus options: %w", err)
}
return k6ext.Promise(vu.Context(), func() (any, error) {
// TODO(@mstoykov): don't use sobek Values in a separate goroutine
return nil, p.Focus(selector, opts) //nolint:wrapcheck
})
return nil, p.Focus(selector, popts) //nolint:wrapcheck
}), nil
},
"frames": func() *sobek.Object {
var (
Expand All @@ -130,18 +144,21 @@ func mapPage(vu moduleVU, p *common.Page) mapping { //nolint:gocognit,cyclop
}
return rt.ToValue(mfrs).ToObject(rt)
},
"getAttribute": func(selector string, name string, opts sobek.Value) *sobek.Promise {
"getAttribute": func(selector string, name string, opts sobek.Value) (*sobek.Promise, error) {
popts := common.NewFrameBaseOptions(p.MainFrame().Timeout())
if err := popts.Parse(vu.Context(), opts); err != nil {
return nil, fmt.Errorf("parsing getAttribute options: %w", err)
}
return k6ext.Promise(vu.Context(), func() (any, error) {
// TODO(@mstoykov): don't use sobek Values in a separate goroutine
s, ok, err := p.GetAttribute(selector, name, opts)
s, ok, err := p.GetAttribute(selector, name, popts)
if err != nil {
return nil, err //nolint:wrapcheck
}
if !ok {
return nil, nil
return nil, nil //nolint:nilnil
}
return s, nil
})
}), nil
},
"goto": func(url string, opts sobek.Value) (*sobek.Promise, error) {
gopts := common.NewFrameGotoOptions(
Expand All @@ -157,32 +174,45 @@ 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
},
"hover": func(selector string, opts sobek.Value) *sobek.Promise {
"hover": func(selector string, opts sobek.Value) (*sobek.Promise, error) {
popts := common.NewFrameHoverOptions(p.MainFrame().Timeout())
if err := popts.Parse(vu.Context(), opts); err != nil {
return nil, fmt.Errorf("parsing hover options: %w", err)
}
return k6ext.Promise(vu.Context(), func() (any, error) {
// TODO(@mstoykov): don't use sobek Values in a separate goroutine
return nil, p.Hover(selector, opts) //nolint:wrapcheck
})
return nil, p.Hover(selector, popts) //nolint:wrapcheck
}), nil
},
"innerHTML": func(selector string, opts sobek.Value) *sobek.Promise {
"innerHTML": func(selector string, opts sobek.Value) (*sobek.Promise, error) {
popts := common.NewFrameInnerHTMLOptions(p.MainFrame().Timeout())
if err := popts.Parse(vu.Context(), opts); err != nil {
return nil, fmt.Errorf("parsing inner HTML options: %w", err)
}
return k6ext.Promise(vu.Context(), func() (any, error) {
// TODO(@mstoykov): don't use sobek Values in a separate goroutine
return p.InnerHTML(selector, opts) //nolint:wrapcheck
})
return p.InnerHTML(selector, popts) //nolint:wrapcheck
}), nil
},
"innerText": func(selector string, opts sobek.Value) *sobek.Promise {
"innerText": func(selector string, opts sobek.Value) (*sobek.Promise, error) {
popts := common.NewFrameInnerTextOptions(p.MainFrame().Timeout())
if err := popts.Parse(vu.Context(), opts); err != nil {
return nil, fmt.Errorf("parsing inner text options: %w", err)
}
return k6ext.Promise(vu.Context(), func() (any, error) {
// TODO(@mstoykov): don't use sobek Values in a separate goroutine
return p.InnerText(selector, opts) //nolint:wrapcheck
})
return p.InnerText(selector, popts) //nolint:wrapcheck
}), nil
},
"inputValue": func(selector string, opts sobek.Value) *sobek.Promise {
"inputValue": func(selector string, opts sobek.Value) (*sobek.Promise, error) {
popts := common.NewFrameInputValueOptions(p.MainFrame().Timeout())
if err := popts.Parse(vu.Context(), opts); err != nil {
return nil, fmt.Errorf("parsing input value options: %w", err)
}
return k6ext.Promise(vu.Context(), func() (any, error) {
// TODO(@mstoykov): don't use sobek Values in a separate goroutine
return p.InputValue(selector, opts) //nolint:wrapcheck
})
return p.InputValue(selector, popts) //nolint:wrapcheck
}), nil
},
"isChecked": func(selector string, opts sobek.Value) *sobek.Promise {
return k6ext.Promise(vu.Context(), func() (any, error) {
Expand Down
Loading
Loading
0