From 3f26d5cd37bac7a306a99e3e730ee093a7e8f12f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=B0nan=C3=A7=20G=C3=BCm=C3=BC=C5=9F?= Date: Fri, 21 Jun 2024 14:29:59 +0300 Subject: [PATCH 1/9] Desobekify Browser - Moves BrowserContextOptions out of the business logic. - Sets default options in NewBrowserContext to prevent nil pointer issues in the rest of the module. Later, we might think about turning some of these pointer fields into value types to take advantage of their zero values. --- browser/browser_mapping.go | 24 +++++++++++++---------- browser/mapping_test.go | 4 ++-- browser/sync_browser_mapping.go | 18 +++++++++++++++-- common/browser.go | 14 +++----------- common/browser_context.go | 7 ++++++- tests/browser_context_options_test.go | 28 +++++++++++++-------------- tests/element_handle_test.go | 18 ++++++++--------- tests/network_manager_test.go | 19 +++++++++--------- tests/test_browser.go | 2 +- 9 files changed, 72 insertions(+), 62 deletions(-) diff --git a/browser/browser_mapping.go b/browser/browser_mapping.go index bfab5190b..19569c09c 100644 --- a/browser/browser_mapping.go +++ b/browser/browser_mapping.go @@ -10,7 +10,7 @@ import ( ) // mapBrowser to the JS module. -func mapBrowser(vu moduleVU) mapping { //nolint:funlen,cyclop +func mapBrowser(vu moduleVU) mapping { //nolint:funlen,cyclop,gocognit return mapping{ "context": func() (mapping, error) { b, err := vu.browser() @@ -36,23 +36,24 @@ func mapBrowser(vu moduleVU) mapping { //nolint:funlen,cyclop return b.IsConnected(), nil }, "newContext": func(opts sobek.Value) (*sobek.Promise, error) { + popts := common.NewBrowserContextOptions() + if err := popts.Parse(vu.Context(), opts); err != nil { + return nil, fmt.Errorf("parsing browser.newContext options: %w", err) + } return k6ext.Promise(vu.Context(), func() (any, error) { b, err := vu.browser() if err != nil { return nil, err } - bctx, err := b.NewContext(opts) + bctx, err := b.NewContext(popts) if err != nil { return nil, err //nolint:wrapcheck } - if err := initBrowserContext(bctx, vu.testRunID); err != nil { return nil, err } - m := mapBrowserContext(vu, bctx) - - return m, nil + return mapBrowserContext(vu, bctx), nil }), nil }, "userAgent": func() (string, error) { @@ -69,23 +70,26 @@ func mapBrowser(vu moduleVU) mapping { //nolint:funlen,cyclop } return b.Version(), nil }, - "newPage": func(opts sobek.Value) *sobek.Promise { + "newPage": func(opts sobek.Value) (*sobek.Promise, error) { + popts := common.NewBrowserContextOptions() + if err := popts.Parse(vu.Context(), opts); err != nil { + return nil, fmt.Errorf("parsing browser.newPage options: %w", err) + } return k6ext.Promise(vu.Context(), func() (any, error) { b, err := vu.browser() if err != nil { return nil, err } - page, err := b.NewPage(opts) + page, err := b.NewPage(popts) if err != nil { return nil, err //nolint:wrapcheck } - if err := initBrowserContext(b.Context(), vu.testRunID); err != nil { return nil, err } return mapPage(vu, page), nil - }) + }), nil }, } } diff --git a/browser/mapping_test.go b/browser/mapping_test.go index ccea1f55c..87d7cce90 100644 --- a/browser/mapping_test.go +++ b/browser/mapping_test.go @@ -264,8 +264,8 @@ type browserAPI interface { Context() *common.BrowserContext CloseContext() IsConnected() bool - NewContext(opts sobek.Value) (*common.BrowserContext, error) - NewPage(opts sobek.Value) (*common.Page, error) + NewContext(opts *common.BrowserContextOptions) (*common.BrowserContext, error) + NewPage(opts *common.BrowserContextOptions) (*common.Page, error) On(string) (bool, error) UserAgent() string Version() string diff --git a/browser/sync_browser_mapping.go b/browser/sync_browser_mapping.go index 400e31ca6..689e610a1 100644 --- a/browser/sync_browser_mapping.go +++ b/browser/sync_browser_mapping.go @@ -1,7 +1,11 @@ package browser import ( + "fmt" + "github.com/grafana/sobek" + + "github.com/grafana/xk6-browser/common" ) // syncMapBrowser is like mapBrowser but returns synchronous functions. @@ -30,11 +34,16 @@ func syncMapBrowser(vu moduleVU) mapping { //nolint:funlen,cyclop return b.IsConnected(), nil }, "newContext": func(opts sobek.Value) (*sobek.Object, error) { + popts := common.NewBrowserContextOptions() + if err := popts.Parse(vu.Context(), opts); err != nil { + return nil, fmt.Errorf("parsing browser.newContext options: %w", err) + } + b, err := vu.browser() if err != nil { return nil, err } - bctx, err := b.NewContext(opts) + bctx, err := b.NewContext(popts) if err != nil { return nil, err //nolint:wrapcheck } @@ -62,11 +71,16 @@ func syncMapBrowser(vu moduleVU) mapping { //nolint:funlen,cyclop return b.Version(), nil }, "newPage": func(opts sobek.Value) (mapping, error) { + popts := common.NewBrowserContextOptions() + if err := popts.Parse(vu.Context(), opts); err != nil { + return nil, fmt.Errorf("parsing browser.newContext options: %w", err) + } + b, err := vu.browser() if err != nil { return nil, err } - page, err := b.NewPage(opts) + page, err := b.NewPage(popts) if err != nil { return nil, err //nolint:wrapcheck } diff --git a/common/browser.go b/common/browser.go index c1b9bc98c..c8e617c0c 100644 --- a/common/browser.go +++ b/common/browser.go @@ -14,7 +14,6 @@ import ( "github.com/chromedp/cdproto/cdp" "github.com/chromedp/cdproto/target" "github.com/gorilla/websocket" - "github.com/grafana/sobek" "github.com/grafana/xk6-browser/k6ext" "github.com/grafana/xk6-browser/log" @@ -569,7 +568,7 @@ func (b *Browser) IsConnected() bool { } // NewContext creates a new incognito-like browser context. -func (b *Browser) NewContext(opts sobek.Value) (*BrowserContext, error) { +func (b *Browser) NewContext(opts *BrowserContextOptions) (*BrowserContext, error) { _, span := TraceAPICall(b.ctx, "", "browser.newContext") defer span.End() @@ -588,14 +587,7 @@ func (b *Browser) NewContext(opts sobek.Value) (*BrowserContext, error) { return nil, err } - browserCtxOpts := NewBrowserContextOptions() - if err := browserCtxOpts.Parse(b.ctx, opts); err != nil { - err := fmt.Errorf("parsing newContext options: %w", err) - spanRecordError(span, err) - return nil, err - } - - browserCtx, err := NewBrowserContext(b.ctx, b, browserContextID, browserCtxOpts, b.logger) + browserCtx, err := NewBrowserContext(b.ctx, b, browserContextID, opts, b.logger) if err != nil { err := fmt.Errorf("new context: %w", err) spanRecordError(span, err) @@ -610,7 +602,7 @@ func (b *Browser) NewContext(opts sobek.Value) (*BrowserContext, error) { } // NewPage creates a new tab in the browser window. -func (b *Browser) NewPage(opts sobek.Value) (*Page, error) { +func (b *Browser) NewPage(opts *BrowserContextOptions) (*Page, error) { _, span := TraceAPICall(b.ctx, "", "browser.newPage") defer span.End() diff --git a/common/browser_context.go b/common/browser_context.go index 30a10a89a..b2a0ef8ab 100644 --- a/common/browser_context.go +++ b/common/browser_context.go @@ -90,6 +90,11 @@ type BrowserContext struct { func NewBrowserContext( ctx context.Context, browser *Browser, id cdp.BrowserContextID, opts *BrowserContextOptions, logger *log.Logger, ) (*BrowserContext, error) { + // set the default options if none provided. + if opts == nil { + opts = NewBrowserContextOptions() + } + b := BrowserContext{ BaseEventEmitter: NewBaseEventEmitter(ctx), ctx: ctx, @@ -101,7 +106,7 @@ func NewBrowserContext( timeoutSettings: NewTimeoutSettings(nil), } - if opts != nil && len(opts.Permissions) > 0 { + if len(opts.Permissions) > 0 { err := b.GrantPermissions(opts.Permissions, NewGrantPermissionsOptions()) if err != nil { return nil, err diff --git a/tests/browser_context_options_test.go b/tests/browser_context_options_test.go index 4741bb547..c74d5ac1d 100644 --- a/tests/browser_context_options_test.go +++ b/tests/browser_context_options_test.go @@ -51,14 +51,12 @@ func TestBrowserContextOptionsSetViewport(t *testing.T) { t.Parallel() tb := newTestBrowser(t) - bctx, err := tb.NewContext(tb.toSobekValue(struct { - Viewport common.Viewport `js:"viewport"` - }{ - Viewport: common.Viewport{ - Width: 800, - Height: 600, - }, - })) + opts := common.NewBrowserContextOptions() + opts.Viewport = &common.Viewport{ + Width: 800, + Height: 600, + } + bctx, err := tb.NewContext(opts) require.NoError(t, err) t.Cleanup(func() { if err := bctx.Close(); err != nil { @@ -77,19 +75,19 @@ func TestBrowserContextOptionsExtraHTTPHeaders(t *testing.T) { t.Parallel() tb := newTestBrowser(t, withHTTPServer()) - bctx, err := tb.NewContext(tb.toSobekValue(struct { - ExtraHTTPHeaders map[string]string `js:"extraHTTPHeaders"` - }{ - ExtraHTTPHeaders: map[string]string{ - "Some-Header": "Some-Value", - }, - })) + + opts := common.NewBrowserContextOptions() + opts.ExtraHTTPHeaders = map[string]string{ + "Some-Header": "Some-Value", + } + bctx, err := tb.NewContext(opts) require.NoError(t, err) t.Cleanup(func() { if err := bctx.Close(); err != nil { t.Log("closing browser context:", err) } }) + p, err := bctx.NewPage() require.NoError(t, err) diff --git a/tests/element_handle_test.go b/tests/element_handle_test.go index 89b966363..da6803fa5 100644 --- a/tests/element_handle_test.go +++ b/tests/element_handle_test.go @@ -181,17 +181,15 @@ func TestElementHandleClickConcealedLink(t *testing.T) { ) tb := newTestBrowser(t, withFileServer()) - bc, err := tb.NewContext( - tb.toSobekValue(struct { - Viewport common.Viewport `js:"viewport"` - }{ - Viewport: common.Viewport{ - Width: 500, - Height: 240, - }, - }), - ) + + bcopts := common.NewBrowserContextOptions() + bcopts.Viewport = &common.Viewport{ + Width: 500, + Height: 240, + } + bc, err := tb.NewContext(bcopts) require.NoError(t, err) + p, err := bc.NewPage() require.NoError(t, err) diff --git a/tests/network_manager_test.go b/tests/network_manager_test.go index 7416c1e90..0fb003154 100644 --- a/tests/network_manager_test.go +++ b/tests/network_manager_test.go @@ -112,21 +112,20 @@ func TestBasicAuth(t *testing.T) { tb.Helper() browser := newTestBrowser(t, withHTTPServer()) - bc, err := browser.NewContext( - browser.toSobekValue(struct { - HttpCredentials *common.Credentials `js:"httpCredentials"` //nolint:revive - }{ - HttpCredentials: &common.Credentials{ - Username: user, - Password: pass, - }, - })) + + bcopts := common.NewBrowserContextOptions() + bcopts.HttpCredentials = &common.Credentials{ + Username: validUser, + Password: validPassword, + } + bc, err := browser.NewContext(bcopts) require.NoError(t, err) + p, err := bc.NewPage() require.NoError(t, err) url := browser.url( - fmt.Sprintf("/basic-auth/%s/%s", validUser, validPassword), + fmt.Sprintf("/basic-auth/%s/%s", user, pass), ) opts := &common.FrameGotoOptions{ WaitUntil: common.LifecycleEventLoad, diff --git a/tests/test_browser.go b/tests/test_browser.go index d405d3665..767add7bf 100644 --- a/tests/test_browser.go +++ b/tests/test_browser.go @@ -253,7 +253,7 @@ func withSkipClose() func(*testBrowser) { // NewPage is a wrapper around Browser.NewPage that fails the test if an // error occurs. Added this helper to avoid boilerplate code in tests. -func (b *testBrowser) NewPage(opts sobek.Value) *common.Page { +func (b *testBrowser) NewPage(opts *common.BrowserContextOptions) *common.Page { b.t.Helper() p, err := b.Browser.NewPage(opts) From ef173dfdd165a4ea274aa7128944ce9b52bfc9dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=B0nan=C3=A7=20G=C3=BCm=C3=BC=C5=9F?= Date: Fri, 21 Jun 2024 14:56:13 +0300 Subject: [PATCH 2/9] Move out BrowserContextOptions parsing Moves BrowserContextOptions parsing out of the business logic. --- browser/browser_context_options.go | 105 ++++++++++++++++++ .../browser_context_options_test.go | 18 +-- browser/browser_mapping.go | 8 +- browser/sync_browser_mapping.go | 14 +-- common/browser_context_options.go | 88 --------------- 5 files changed, 123 insertions(+), 110 deletions(-) create mode 100644 browser/browser_context_options.go rename {common => browser}/browser_context_options_test.go (61%) diff --git a/browser/browser_context_options.go b/browser/browser_context_options.go new file mode 100644 index 000000000..afaee0c52 --- /dev/null +++ b/browser/browser_context_options.go @@ -0,0 +1,105 @@ +package browser + +import ( + "context" + "fmt" + + "github.com/grafana/sobek" + + "github.com/grafana/xk6-browser/common" + "github.com/grafana/xk6-browser/k6ext" +) + +// ParseBrowserContextOptions parses the browser context options. +func ParseBrowserContextOptions( //nolint:funlen,gocognit,cyclop + ctx context.Context, + opts sobek.Value, +) (*common.BrowserContextOptions, error) { + if !sobekValueExists(opts) { + return nil, nil //nolint:nilnil + } + + popts := common.NewBrowserContextOptions() + + rt := k6ext.Runtime(ctx) + o := opts.ToObject(rt) + for _, k := range o.Keys() { + switch k { + case "acceptDownloads": + popts.AcceptDownloads = o.Get(k).ToBoolean() + case "bypassCSP": + popts.BypassCSP = o.Get(k).ToBoolean() + case "colorScheme": + switch common.ColorScheme(o.Get(k).String()) { //nolint:exhaustive + case "light": + popts.ColorScheme = common.ColorSchemeLight + case "dark": + popts.ColorScheme = common.ColorSchemeDark + default: + popts.ColorScheme = common.ColorSchemeNoPreference + } + case "deviceScaleFactor": + popts.DeviceScaleFactor = o.Get(k).ToFloat() + case "extraHTTPHeaders": + headers := o.Get(k).ToObject(rt) + for _, k := range headers.Keys() { + popts.ExtraHTTPHeaders[k] = headers.Get(k).String() + } + case "geolocation": + geolocation := common.NewGeolocation() + if err := geolocation.Parse(ctx, o.Get(k).ToObject(rt)); err != nil { + return nil, fmt.Errorf("parsing geolocation options: %w", err) + } + popts.Geolocation = geolocation + case "hasTouch": + popts.HasTouch = o.Get(k).ToBoolean() + case "httpCredentials": + credentials := common.NewCredentials() + if err := credentials.Parse(ctx, o.Get(k).ToObject(rt)); err != nil { + return nil, fmt.Errorf("parsing httpCredentials options: %w", err) + } + popts.HttpCredentials = credentials + case "ignoreHTTPSErrors": + popts.IgnoreHTTPSErrors = o.Get(k).ToBoolean() + case "isMobile": + popts.IsMobile = o.Get(k).ToBoolean() + case "javaScriptEnabled": + popts.JavaScriptEnabled = o.Get(k).ToBoolean() + case "locale": + popts.Locale = o.Get(k).String() + case "offline": + popts.Offline = o.Get(k).ToBoolean() + case "permissions": + if ps, ok := o.Get(k).Export().([]any); ok { + for _, p := range ps { + popts.Permissions = append(popts.Permissions, fmt.Sprintf("%v", p)) + } + } + case "reducedMotion": + switch common.ReducedMotion(o.Get(k).String()) { //nolint:exhaustive + case "reduce": + popts.ReducedMotion = common.ReducedMotionReduce + default: + popts.ReducedMotion = common.ReducedMotionNoPreference + } + case "screen": + screen := &common.Screen{} + if err := screen.Parse(ctx, o.Get(k).ToObject(rt)); err != nil { + return nil, fmt.Errorf("parsing screen options: %w", err) + } + popts.Screen = screen + case "timezoneID": + popts.TimezoneID = o.Get(k).String() + case "userAgent": + popts.UserAgent = o.Get(k).String() + case "viewport": + viewport := &common.Viewport{} + if err := viewport.Parse(ctx, o.Get(k).ToObject(rt)); err != nil { + return nil, fmt.Errorf("parsing viewport options: %w", err) + } + popts.Viewport = viewport + } + } + + return popts, nil +} diff --git a/common/browser_context_options_test.go b/browser/browser_context_options_test.go similarity index 61% rename from common/browser_context_options_test.go rename to browser/browser_context_options_test.go index afc25aab8..03171374b 100644 --- a/common/browser_context_options_test.go +++ b/browser/browser_context_options_test.go @@ -1,22 +1,22 @@ -package common +package browser import ( "testing" - "github.com/grafana/xk6-browser/k6ext/k6test" - "github.com/stretchr/testify/assert" + + "github.com/grafana/xk6-browser/k6ext/k6test" ) func TestBrowserContextOptionsPermissions(t *testing.T) { vu := k6test.NewVU(t) - var opts BrowserContextOptions - err := opts.Parse(vu.Context(), vu.ToSobekValue((struct { - Permissions []any `js:"permissions"` - }{ - Permissions: []any{"camera", "microphone"}, - }))) + opts, err := ParseBrowserContextOptions(vu.Context(), + vu.ToSobekValue((struct { + Permissions []any `js:"permissions"` + }{ + Permissions: []any{"camera", "microphone"}, + }))) assert.NoError(t, err) assert.Len(t, opts.Permissions, 2) assert.Equal(t, opts.Permissions, []string{"camera", "microphone"}) diff --git a/browser/browser_mapping.go b/browser/browser_mapping.go index 19569c09c..f6e72bd2f 100644 --- a/browser/browser_mapping.go +++ b/browser/browser_mapping.go @@ -36,8 +36,8 @@ func mapBrowser(vu moduleVU) mapping { //nolint:funlen,cyclop,gocognit return b.IsConnected(), nil }, "newContext": func(opts sobek.Value) (*sobek.Promise, error) { - popts := common.NewBrowserContextOptions() - if err := popts.Parse(vu.Context(), opts); err != nil { + popts, err := ParseBrowserContextOptions(vu.Context(), opts) + if err != nil { return nil, fmt.Errorf("parsing browser.newContext options: %w", err) } return k6ext.Promise(vu.Context(), func() (any, error) { @@ -71,8 +71,8 @@ func mapBrowser(vu moduleVU) mapping { //nolint:funlen,cyclop,gocognit return b.Version(), nil }, "newPage": func(opts sobek.Value) (*sobek.Promise, error) { - popts := common.NewBrowserContextOptions() - if err := popts.Parse(vu.Context(), opts); err != nil { + popts, err := ParseBrowserContextOptions(vu.Context(), opts) + if err != nil { return nil, fmt.Errorf("parsing browser.newPage options: %w", err) } return k6ext.Promise(vu.Context(), func() (any, error) { diff --git a/browser/sync_browser_mapping.go b/browser/sync_browser_mapping.go index 689e610a1..b1ba9e898 100644 --- a/browser/sync_browser_mapping.go +++ b/browser/sync_browser_mapping.go @@ -4,8 +4,6 @@ import ( "fmt" "github.com/grafana/sobek" - - "github.com/grafana/xk6-browser/common" ) // syncMapBrowser is like mapBrowser but returns synchronous functions. @@ -34,11 +32,10 @@ func syncMapBrowser(vu moduleVU) mapping { //nolint:funlen,cyclop return b.IsConnected(), nil }, "newContext": func(opts sobek.Value) (*sobek.Object, error) { - popts := common.NewBrowserContextOptions() - if err := popts.Parse(vu.Context(), opts); err != nil { + popts, err := ParseBrowserContextOptions(vu.Context(), opts) + if err != nil { return nil, fmt.Errorf("parsing browser.newContext options: %w", err) } - b, err := vu.browser() if err != nil { return nil, err @@ -71,11 +68,10 @@ func syncMapBrowser(vu moduleVU) mapping { //nolint:funlen,cyclop return b.Version(), nil }, "newPage": func(opts sobek.Value) (mapping, error) { - popts := common.NewBrowserContextOptions() - if err := popts.Parse(vu.Context(), opts); err != nil { - return nil, fmt.Errorf("parsing browser.newContext options: %w", err) + popts, err := ParseBrowserContextOptions(vu.Context(), opts) + if err != nil { + return nil, fmt.Errorf("parsing browser.newPage options: %w", err) } - b, err := vu.browser() if err != nil { return nil, err diff --git a/common/browser_context_options.go b/common/browser_context_options.go index 507cde33d..17614c439 100644 --- a/common/browser_context_options.go +++ b/common/browser_context_options.go @@ -100,94 +100,6 @@ func NewBrowserContextOptions() *BrowserContextOptions { } } -// Parse parses the browser context options. -func (b *BrowserContextOptions) Parse(ctx context.Context, opts sobek.Value) error { //nolint:cyclop,funlen,gocognit - rt := k6ext.Runtime(ctx) - if !sobekValueExists(opts) { - return nil - } - o := opts.ToObject(rt) - for _, k := range o.Keys() { - switch k { - case "acceptDownloads": - b.AcceptDownloads = o.Get(k).ToBoolean() - case "bypassCSP": - b.BypassCSP = o.Get(k).ToBoolean() - case "colorScheme": - switch ColorScheme(o.Get(k).String()) { //nolint:exhaustive - case "light": - b.ColorScheme = ColorSchemeLight - case "dark": - b.ColorScheme = ColorSchemeDark - default: - b.ColorScheme = ColorSchemeNoPreference - } - case "deviceScaleFactor": - b.DeviceScaleFactor = o.Get(k).ToFloat() - case "extraHTTPHeaders": - headers := o.Get(k).ToObject(rt) - for _, k := range headers.Keys() { - b.ExtraHTTPHeaders[k] = headers.Get(k).String() - } - case "geolocation": - geolocation := NewGeolocation() - if err := geolocation.Parse(ctx, o.Get(k).ToObject(rt)); err != nil { - return err - } - b.Geolocation = geolocation - case "hasTouch": - b.HasTouch = o.Get(k).ToBoolean() - case "httpCredentials": - credentials := NewCredentials() - if err := credentials.Parse(ctx, o.Get(k).ToObject(rt)); err != nil { - return err - } - b.HttpCredentials = credentials - case "ignoreHTTPSErrors": - b.IgnoreHTTPSErrors = o.Get(k).ToBoolean() - case "isMobile": - b.IsMobile = o.Get(k).ToBoolean() - case "javaScriptEnabled": - b.JavaScriptEnabled = o.Get(k).ToBoolean() - case "locale": - b.Locale = o.Get(k).String() - case "offline": - b.Offline = o.Get(k).ToBoolean() - case "permissions": - if ps, ok := o.Get(k).Export().([]any); ok { - for _, p := range ps { - b.Permissions = append(b.Permissions, fmt.Sprintf("%v", p)) - } - } - case "reducedMotion": - switch ReducedMotion(o.Get(k).String()) { //nolint:exhaustive - case "reduce": - b.ReducedMotion = ReducedMotionReduce - default: - b.ReducedMotion = ReducedMotionNoPreference - } - case "screen": - screen := &Screen{} - if err := screen.Parse(ctx, o.Get(k).ToObject(rt)); err != nil { - return err - } - b.Screen = screen - case "timezoneID": - b.TimezoneID = o.Get(k).String() - case "userAgent": - b.UserAgent = o.Get(k).String() - case "viewport": - viewport := &Viewport{} - if err := viewport.Parse(ctx, o.Get(k).ToObject(rt)); err != nil { - return err - } - b.Viewport = viewport - } - } - - return nil -} - // WaitForEventOptions are the options used by the browserContext.waitForEvent API. type WaitForEventOptions struct { Timeout time.Duration From b7b8770ee0f416c5aaff468b284b2596ed00a0cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=B0nan=C3=A7=20G=C3=BCm=C3=BC=C5=9F?= Date: Fri, 21 Jun 2024 15:50:39 +0300 Subject: [PATCH 3/9] Move out geolocation parsing - Separates the parsing of options from their validation. The validation logic remains in the business logic while the parsing happens in the mapping layer. - Moves geolocation option parsing to the mapping layer. - Adds a validation function for geolocation. --- browser/browser_context_mapping.go | 13 +++-- browser/browser_context_options.go | 35 +++++++++--- browser/browser_mapping.go | 6 +++ browser/sync_browser_mapping.go | 6 +++ common/browser_context.go | 9 +--- common/browser_context_options.go | 85 ++++++++++++------------------ 6 files changed, 88 insertions(+), 66 deletions(-) diff --git a/browser/browser_context_mapping.go b/browser/browser_context_mapping.go index f4d14fc29..d740aec14 100644 --- a/browser/browser_context_mapping.go +++ b/browser/browser_context_mapping.go @@ -83,10 +83,17 @@ func mapBrowserContext(vu moduleVU, bc *common.BrowserContext) mapping { //nolin }, "setDefaultNavigationTimeout": bc.SetDefaultNavigationTimeout, "setDefaultTimeout": bc.SetDefaultTimeout, - "setGeolocation": func(geolocation sobek.Value) *sobek.Promise { + "setGeolocation": func(geolocation sobek.Value) (*sobek.Promise, error) { + geoloc, err := ParseGeolocation(vu.Context(), geolocation) + if err != nil { + return nil, fmt.Errorf("parsing geo location: %w", err) + } + if err := geoloc.Validate(); err != nil { + return nil, fmt.Errorf("validating geo location: %w", err) + } return k6ext.Promise(vu.Context(), func() (any, error) { - return nil, bc.SetGeolocation(geolocation) //nolint:wrapcheck - }) + return nil, bc.SetGeolocation(geoloc) //nolint:wrapcheck + }), nil }, "setHTTPCredentials": func(httpCredentials sobek.Value) *sobek.Promise { return k6ext.Promise(vu.Context(), func() (any, error) { diff --git a/browser/browser_context_options.go b/browser/browser_context_options.go index afaee0c52..ade40d47d 100644 --- a/browser/browser_context_options.go +++ b/browser/browser_context_options.go @@ -15,12 +15,12 @@ func ParseBrowserContextOptions( //nolint:funlen,gocognit,cyclop ctx context.Context, opts sobek.Value, ) (*common.BrowserContextOptions, error) { + popts := common.NewBrowserContextOptions() + if !sobekValueExists(opts) { - return nil, nil //nolint:nilnil + return popts, nil // return the default options } - popts := common.NewBrowserContextOptions() - rt := k6ext.Runtime(ctx) o := opts.ToObject(rt) for _, k := range o.Keys() { @@ -46,11 +46,11 @@ func ParseBrowserContextOptions( //nolint:funlen,gocognit,cyclop popts.ExtraHTTPHeaders[k] = headers.Get(k).String() } case "geolocation": - geolocation := common.NewGeolocation() - if err := geolocation.Parse(ctx, o.Get(k).ToObject(rt)); err != nil { + geoloc, err := ParseGeolocation(ctx, o.Get(k).ToObject(rt)) + if err != nil { return nil, fmt.Errorf("parsing geolocation options: %w", err) } - popts.Geolocation = geolocation + popts.Geolocation = geoloc case "hasTouch": popts.HasTouch = o.Get(k).ToBoolean() case "httpCredentials": @@ -103,3 +103,26 @@ func ParseBrowserContextOptions( //nolint:funlen,gocognit,cyclop return popts, nil } + +// ParseGeolocation parses the geolocation. +func ParseGeolocation(ctx context.Context, opts sobek.Value) (*common.Geolocation, error) { + var geoloc common.Geolocation + + if !sobekValueExists(opts) { + return &geoloc, nil // return the default options + } + + o := opts.ToObject(k6ext.Runtime(ctx)) + for _, k := range o.Keys() { + switch k { + case "accuracy": + geoloc.Accurracy = o.Get(k).ToFloat() + case "latitude": + geoloc.Latitude = o.Get(k).ToFloat() + case "longitude": + geoloc.Longitude = o.Get(k).ToFloat() + } + } + + return &geoloc, nil +} diff --git a/browser/browser_mapping.go b/browser/browser_mapping.go index f6e72bd2f..ad09b073a 100644 --- a/browser/browser_mapping.go +++ b/browser/browser_mapping.go @@ -40,6 +40,9 @@ func mapBrowser(vu moduleVU) mapping { //nolint:funlen,cyclop,gocognit if err != nil { return nil, fmt.Errorf("parsing browser.newContext options: %w", err) } + if err := popts.Validate(); err != nil { + return nil, fmt.Errorf("validating browser.newContext options: %w", err) + } return k6ext.Promise(vu.Context(), func() (any, error) { b, err := vu.browser() if err != nil { @@ -75,6 +78,9 @@ func mapBrowser(vu moduleVU) mapping { //nolint:funlen,cyclop,gocognit if err != nil { return nil, fmt.Errorf("parsing browser.newPage options: %w", err) } + if err := popts.Validate(); err != nil { + return nil, fmt.Errorf("validating browser.newPage options: %w", err) + } return k6ext.Promise(vu.Context(), func() (any, error) { b, err := vu.browser() if err != nil { diff --git a/browser/sync_browser_mapping.go b/browser/sync_browser_mapping.go index b1ba9e898..97ea57829 100644 --- a/browser/sync_browser_mapping.go +++ b/browser/sync_browser_mapping.go @@ -36,6 +36,9 @@ func syncMapBrowser(vu moduleVU) mapping { //nolint:funlen,cyclop if err != nil { return nil, fmt.Errorf("parsing browser.newContext options: %w", err) } + if err := popts.Validate(); err != nil { + return nil, fmt.Errorf("validating browser.newContext options: %w", err) + } b, err := vu.browser() if err != nil { return nil, err @@ -72,6 +75,9 @@ func syncMapBrowser(vu moduleVU) mapping { //nolint:funlen,cyclop if err != nil { return nil, fmt.Errorf("parsing browser.newPage options: %w", err) } + if err := popts.Validate(); err != nil { + return nil, fmt.Errorf("validating browser.newPage options: %w", err) + } b, err := vu.browser() if err != nil { return nil, err diff --git a/common/browser_context.go b/common/browser_context.go index b2a0ef8ab..71e7f81a0 100644 --- a/common/browser_context.go +++ b/common/browser_context.go @@ -265,15 +265,10 @@ func (b *BrowserContext) SetDefaultTimeout(timeout int64) { } // SetGeolocation overrides the geo location of the user. -func (b *BrowserContext) SetGeolocation(geolocation sobek.Value) error { +func (b *BrowserContext) SetGeolocation(geolocation *Geolocation) error { b.logger.Debugf("BrowserContext:SetGeolocation", "bctxid:%v", b.id) - g := NewGeolocation() - if err := g.Parse(b.ctx, geolocation); err != nil { - return fmt.Errorf("parsing geo location: %w", err) - } - - b.opts.Geolocation = g + b.opts.Geolocation = geolocation for _, p := range b.browser.getPages() { if err := p.updateGeolocation(); err != nil { return fmt.Errorf("updating geo location in target ID %s: %w", p.targetID, err) diff --git a/common/browser_context_options.go b/common/browser_context_options.go index 17614c439..5415cb226 100644 --- a/common/browser_context_options.go +++ b/common/browser_context_options.go @@ -11,56 +11,6 @@ import ( "github.com/grafana/xk6-browser/k6ext" ) -// Geolocation represents a geolocation. -type Geolocation struct { - Latitude float64 `js:"latitude"` - Longitude float64 `js:"longitude"` - Accurracy float64 `js:"accurracy"` -} - -// NewGeolocation creates a new instance of Geolocation. -func NewGeolocation() *Geolocation { - return &Geolocation{} -} - -// Parse parses the geolocation options. -func (g *Geolocation) Parse(ctx context.Context, opts sobek.Value) error { //nolint:cyclop - rt := k6ext.Runtime(ctx) - longitude := 0.0 - latitude := 0.0 - accuracy := 0.0 - - if opts != nil && !sobek.IsUndefined(opts) && !sobek.IsNull(opts) { - opts := opts.ToObject(rt) - for _, k := range opts.Keys() { - switch k { - case "accuracy": - accuracy = opts.Get(k).ToFloat() - case "latitude": - latitude = opts.Get(k).ToFloat() - case "longitude": - longitude = opts.Get(k).ToFloat() - } - } - } - - if longitude < -180 || longitude > 180 { - return fmt.Errorf(`invalid longitude "%.2f": precondition -180 <= LONGITUDE <= 180 failed`, longitude) - } - if latitude < -90 || latitude > 90 { - return fmt.Errorf(`invalid latitude "%.2f": precondition -90 <= LATITUDE <= 90 failed`, latitude) - } - if accuracy < 0 { - return fmt.Errorf(`invalid accuracy "%.2f": precondition 0 <= ACCURACY failed`, accuracy) - } - - g.Accurracy = accuracy - g.Latitude = latitude - g.Longitude = longitude - - return nil -} - // BrowserContextOptions stores browser context options. type BrowserContextOptions struct { AcceptDownloads bool `js:"acceptDownloads"` @@ -100,6 +50,41 @@ func NewBrowserContextOptions() *BrowserContextOptions { } } +// Validate validates the browser context options. +func (b *BrowserContextOptions) Validate() error { + if err := b.Geolocation.Validate(); err != nil { + return fmt.Errorf("validating geolocation option: %w", err) + } + + return nil +} + +// Geolocation represents a geolocation. +type Geolocation struct { + Latitude float64 `js:"latitude"` + Longitude float64 `js:"longitude"` + Accurracy float64 `js:"accurracy"` +} + +// Validate validates the geolocation. +func (g *Geolocation) Validate() error { + if g == nil { + return nil // nothing to validate + } + + if g.Accurracy < 0 { + return fmt.Errorf(`invalid accuracy "%.2f": precondition 0 <= ACCURACY failed`, g.Accurracy) + } + if g.Latitude < -90 || g.Latitude > 90 { + return fmt.Errorf(`invalid latitude "%.2f": precondition -90 <= LATITUDE <= 90 failed`, g.Latitude) + } + if g.Longitude < -180 || g.Longitude > 180 { + return fmt.Errorf(`invalid longitude "%.2f": precondition -180 <= LONGITUDE <= 180 failed`, g.Longitude) + } + + return nil +} + // WaitForEventOptions are the options used by the browserContext.waitForEvent API. type WaitForEventOptions struct { Timeout time.Duration From 3fc7c301f9c7e1fb36e51c7f243145c3f17f2459 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=B0nan=C3=A7=20G=C3=BCm=C3=BC=C5=9F?= Date: Fri, 21 Jun 2024 16:01:30 +0300 Subject: [PATCH 4/9] Add ParseCredentials --- browser/browser_context_options.go | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/browser/browser_context_options.go b/browser/browser_context_options.go index ade40d47d..6c792c3d3 100644 --- a/browser/browser_context_options.go +++ b/browser/browser_context_options.go @@ -54,11 +54,11 @@ func ParseBrowserContextOptions( //nolint:funlen,gocognit,cyclop case "hasTouch": popts.HasTouch = o.Get(k).ToBoolean() case "httpCredentials": - credentials := common.NewCredentials() - if err := credentials.Parse(ctx, o.Get(k).ToObject(rt)); err != nil { + creds, err := ParseCredentials(ctx, o.Get(k).ToObject(rt)) + if err != nil { return nil, fmt.Errorf("parsing httpCredentials options: %w", err) } - popts.HttpCredentials = credentials + popts.HttpCredentials = creds case "ignoreHTTPSErrors": popts.IgnoreHTTPSErrors = o.Get(k).ToBoolean() case "isMobile": @@ -126,3 +126,24 @@ func ParseGeolocation(ctx context.Context, opts sobek.Value) (*common.Geolocatio return &geoloc, nil } + +// ParseCredentials parses the credentials. +func ParseCredentials(ctx context.Context, opts sobek.Value) (*common.Credentials, error) { + var creds common.Credentials + + if !sobekValueExists(opts) { + return &creds, nil // return the default options + } + + o := opts.ToObject(k6ext.Runtime(ctx)) + for _, k := range o.Keys() { + switch k { + case "username": + creds.Username = o.Get(k).String() + case "password": + creds.Password = o.Get(k).String() + } + } + + return &creds, nil +} From 0883595b7e81b0c1683f3cab8b6a1296e2c43671 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=B0nan=C3=A7=20G=C3=BCm=C3=BC=C5=9F?= Date: Fri, 21 Jun 2024 16:05:28 +0300 Subject: [PATCH 5/9] Use Credentials in SetHTTPCredentials --- browser/browser_context_mapping.go | 10 +++++++--- browser/sync_browser_context_mapping.go | 10 ++++++++-- common/browser_context.go | 9 ++------- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/browser/browser_context_mapping.go b/browser/browser_context_mapping.go index d740aec14..e0d5cebc4 100644 --- a/browser/browser_context_mapping.go +++ b/browser/browser_context_mapping.go @@ -95,10 +95,14 @@ func mapBrowserContext(vu moduleVU, bc *common.BrowserContext) mapping { //nolin return nil, bc.SetGeolocation(geoloc) //nolint:wrapcheck }), nil }, - "setHTTPCredentials": func(httpCredentials sobek.Value) *sobek.Promise { + "setHTTPCredentials": func(httpCredentials sobek.Value) (*sobek.Promise, error) { + creds, err := ParseCredentials(vu.Context(), httpCredentials) + if err != nil { + return nil, fmt.Errorf("parsing httpCredentials options: %w", err) + } return k6ext.Promise(vu.Context(), func() (any, error) { - return nil, bc.SetHTTPCredentials(httpCredentials) //nolint:staticcheck,wrapcheck - }) + return nil, bc.SetHTTPCredentials(creds) //nolint:staticcheck,wrapcheck + }), nil }, "setOffline": func(offline bool) *sobek.Promise { return k6ext.Promise(vu.Context(), func() (any, error) { diff --git a/browser/sync_browser_context_mapping.go b/browser/sync_browser_context_mapping.go index 5e8de66d4..6661855ba 100644 --- a/browser/sync_browser_context_mapping.go +++ b/browser/sync_browser_context_mapping.go @@ -57,8 +57,14 @@ func syncMapBrowserContext(vu moduleVU, bc *common.BrowserContext) mapping { //n "setDefaultNavigationTimeout": bc.SetDefaultNavigationTimeout, "setDefaultTimeout": bc.SetDefaultTimeout, "setGeolocation": bc.SetGeolocation, - "setHTTPCredentials": bc.SetHTTPCredentials, //nolint:staticcheck - "setOffline": bc.SetOffline, + "setHTTPCredentials": func(httpCredentials sobek.Value) error { + creds, err := ParseCredentials(vu.Context(), httpCredentials) + if err != nil { + return fmt.Errorf("parsing httpCredentials options: %w", err) + } + return bc.SetHTTPCredentials(creds) //nolint:wrapcheck,staticcheck + }, + "setOffline": bc.SetOffline, "waitForEvent": func(event string, optsOrPredicate sobek.Value) (*sobek.Promise, error) { ctx := vu.Context() popts := common.NewWaitForEventOptions( diff --git a/common/browser_context.go b/common/browser_context.go index 71e7f81a0..90d828ce4 100644 --- a/common/browser_context.go +++ b/common/browser_context.go @@ -12,7 +12,6 @@ import ( "github.com/chromedp/cdproto/network" "github.com/chromedp/cdproto/storage" "github.com/chromedp/cdproto/target" - "github.com/grafana/sobek" "github.com/grafana/xk6-browser/common/js" "github.com/grafana/xk6-browser/k6error" @@ -284,17 +283,13 @@ func (b *BrowserContext) SetGeolocation(geolocation *Geolocation) error { // See for details: // - https://github.com/microsoft/playwright/issues/2196#issuecomment-627134837 // - https://github.com/microsoft/playwright/pull/2763 -func (b *BrowserContext) SetHTTPCredentials(httpCredentials sobek.Value) error { +func (b *BrowserContext) SetHTTPCredentials(httpCredentials *Credentials) error { b.logger.Warnf("setHTTPCredentials", "setHTTPCredentials is deprecated."+ " Create a new BrowserContext with httpCredentials instead.") b.logger.Debugf("BrowserContext:SetHTTPCredentials", "bctxid:%v", b.id) - c := NewCredentials() - if err := c.Parse(b.ctx, httpCredentials); err != nil { - return fmt.Errorf("parsing HTTP credentials: %w", err) - } + b.opts.HttpCredentials = httpCredentials - b.opts.HttpCredentials = c for _, p := range b.browser.getPages() { if err := p.updateHTTPCredentials(); err != nil { return fmt.Errorf("setting HTTP credentials in target ID %s: %w", p.targetID, err) From 0b543b730707f59fc2aa7ce7aa9168e3f47d9d23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=B0nan=C3=A7=20G=C3=BCm=C3=BC=C5=9F?= Date: Fri, 21 Jun 2024 16:20:45 +0300 Subject: [PATCH 6/9] Add ParseScreen --- browser/browser_context_options.go | 25 +++++++++++++++++++++++-- common/page.go | 23 ----------------------- 2 files changed, 23 insertions(+), 25 deletions(-) diff --git a/browser/browser_context_options.go b/browser/browser_context_options.go index 6c792c3d3..31a61c772 100644 --- a/browser/browser_context_options.go +++ b/browser/browser_context_options.go @@ -83,8 +83,8 @@ func ParseBrowserContextOptions( //nolint:funlen,gocognit,cyclop popts.ReducedMotion = common.ReducedMotionNoPreference } case "screen": - screen := &common.Screen{} - if err := screen.Parse(ctx, o.Get(k).ToObject(rt)); err != nil { + screen, err := ParseScreen(ctx, o.Get(k).ToObject(rt)) + if err != nil { return nil, fmt.Errorf("parsing screen options: %w", err) } popts.Screen = screen @@ -147,3 +147,24 @@ func ParseCredentials(ctx context.Context, opts sobek.Value) (*common.Credential return &creds, nil } + +// ParseScreen parses the screen options. +func ParseScreen(ctx context.Context, opts sobek.Value) (*common.Screen, error) { + var screen common.Screen + + if !sobekValueExists(opts) { + return &screen, nil // return the default options + } + + o := opts.ToObject(k6ext.Runtime(ctx)) + for _, k := range o.Keys() { + switch k { + case "width": + screen.Width = o.Get(k).ToInteger() + case "height": + screen.Height = o.Get(k).ToInteger() + } + } + + return &screen, nil +} diff --git a/common/page.go b/common/page.go index 35b2f296e..25053e727 100644 --- a/common/page.go +++ b/common/page.go @@ -98,29 +98,6 @@ type Screen struct { Height int64 `js:"height"` } -const ( - screenWidth = "width" - screenHeight = "height" -) - -// Parse parses the given screen options. -func (s *Screen) Parse(ctx context.Context, screen sobek.Value) error { - rt := k6ext.Runtime(ctx) - if screen != nil && !sobek.IsUndefined(screen) && !sobek.IsNull(screen) { - screen := screen.ToObject(rt) - for _, k := range screen.Keys() { - switch k { - case screenWidth: - s.Width = screen.Get(k).ToInteger() - case screenHeight: - s.Height = screen.Get(k).ToInteger() - } - } - } - - return nil -} - // ColorScheme represents a browser color scheme. type ColorScheme string From cefe21494cf1dd2e88fed1f374618f1a70a8d839 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=B0nan=C3=A7=20G=C3=BCm=C3=BC=C5=9F?= Date: Fri, 21 Jun 2024 16:24:58 +0300 Subject: [PATCH 7/9] Add ParseViewport --- browser/browser_context_options.go | 27 ++++++++++++++++++++++++--- common/layout.go | 18 ------------------ 2 files changed, 24 insertions(+), 21 deletions(-) diff --git a/browser/browser_context_options.go b/browser/browser_context_options.go index 31a61c772..e5d63ad35 100644 --- a/browser/browser_context_options.go +++ b/browser/browser_context_options.go @@ -93,11 +93,11 @@ func ParseBrowserContextOptions( //nolint:funlen,gocognit,cyclop case "userAgent": popts.UserAgent = o.Get(k).String() case "viewport": - viewport := &common.Viewport{} - if err := viewport.Parse(ctx, o.Get(k).ToObject(rt)); err != nil { + vp, err := ParseViewport(ctx, o.Get(k).ToObject(rt)) + if err != nil { return nil, fmt.Errorf("parsing viewport options: %w", err) } - popts.Viewport = viewport + popts.Viewport = vp } } @@ -168,3 +168,24 @@ func ParseScreen(ctx context.Context, opts sobek.Value) (*common.Screen, error) return &screen, nil } + +// ParseViewport parses the viewport options. +func ParseViewport(ctx context.Context, opts sobek.Value) (*common.Viewport, error) { + var viewport common.Viewport + + if !sobekValueExists(opts) { + return &viewport, nil // return the default options + } + + o := opts.ToObject(k6ext.Runtime(ctx)) + for _, k := range o.Keys() { + switch k { + case "width": + viewport.Width = o.Get(k).ToInteger() + case "height": + viewport.Height = o.Get(k).ToInteger() + } + } + + return &viewport, nil +} diff --git a/common/layout.go b/common/layout.go index afc8ad964..a6765e8a9 100644 --- a/common/layout.go +++ b/common/layout.go @@ -80,24 +80,6 @@ type Viewport struct { Height int64 `js:"height"` } -// Parse viewport details from a given sobek viewport value. -func (v *Viewport) Parse(ctx context.Context, viewport sobek.Value) error { - rt := k6ext.Runtime(ctx) - if viewport != nil && !sobek.IsUndefined(viewport) && !sobek.IsNull(viewport) { - viewport := viewport.ToObject(rt) - for _, k := range viewport.Keys() { - switch k { - case "width": - v.Width = viewport.Get(k).ToInteger() - case "height": - v.Height = viewport.Get(k).ToInteger() - } - } - } - - return nil -} - func (v Viewport) String() string { return fmt.Sprintf("%dx%d", v.Width, v.Height) } From 57b9aca81bb15e9088ba91151eed18c38935bc27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=B0nan=C3=A7=20G=C3=BCm=C3=BC=C5=9F?= Date: Fri, 21 Jun 2024 16:27:03 +0300 Subject: [PATCH 8/9] Add optionWidth and optionHeight --- browser/browser_context_options.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/browser/browser_context_options.go b/browser/browser_context_options.go index e5d63ad35..71149ae78 100644 --- a/browser/browser_context_options.go +++ b/browser/browser_context_options.go @@ -10,6 +10,11 @@ import ( "github.com/grafana/xk6-browser/k6ext" ) +const ( + optionWidth = "width" + optionHeight = "height" +) + // ParseBrowserContextOptions parses the browser context options. func ParseBrowserContextOptions( //nolint:funlen,gocognit,cyclop ctx context.Context, @@ -159,9 +164,9 @@ func ParseScreen(ctx context.Context, opts sobek.Value) (*common.Screen, error) o := opts.ToObject(k6ext.Runtime(ctx)) for _, k := range o.Keys() { switch k { - case "width": + case optionWidth: screen.Width = o.Get(k).ToInteger() - case "height": + case optionHeight: screen.Height = o.Get(k).ToInteger() } } @@ -180,9 +185,9 @@ func ParseViewport(ctx context.Context, opts sobek.Value) (*common.Viewport, err o := opts.ToObject(k6ext.Runtime(ctx)) for _, k := range o.Keys() { switch k { - case "width": + case optionWidth: viewport.Width = o.Get(k).ToInteger() - case "height": + case optionHeight: viewport.Height = o.Get(k).ToInteger() } } From 83ac07e7a16e1ee9963c04e92df826a6727338ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=B0nan=C3=A7=20G=C3=BCm=C3=BC=C5=9F?= Date: Fri, 21 Jun 2024 16:39:59 +0300 Subject: [PATCH 9/9] Refactor browser opt parsers to take rt --- browser/browser_context_mapping.go | 4 ++-- browser/browser_context_options.go | 29 +++++++++++-------------- browser/browser_context_options_test.go | 3 ++- browser/browser_mapping.go | 4 ++-- browser/sync_browser_context_mapping.go | 2 +- browser/sync_browser_mapping.go | 4 ++-- 6 files changed, 22 insertions(+), 24 deletions(-) diff --git a/browser/browser_context_mapping.go b/browser/browser_context_mapping.go index e0d5cebc4..fd9338e5d 100644 --- a/browser/browser_context_mapping.go +++ b/browser/browser_context_mapping.go @@ -84,7 +84,7 @@ func mapBrowserContext(vu moduleVU, bc *common.BrowserContext) mapping { //nolin "setDefaultNavigationTimeout": bc.SetDefaultNavigationTimeout, "setDefaultTimeout": bc.SetDefaultTimeout, "setGeolocation": func(geolocation sobek.Value) (*sobek.Promise, error) { - geoloc, err := ParseGeolocation(vu.Context(), geolocation) + geoloc, err := ParseGeolocation(rt, geolocation) if err != nil { return nil, fmt.Errorf("parsing geo location: %w", err) } @@ -96,7 +96,7 @@ func mapBrowserContext(vu moduleVU, bc *common.BrowserContext) mapping { //nolin }), nil }, "setHTTPCredentials": func(httpCredentials sobek.Value) (*sobek.Promise, error) { - creds, err := ParseCredentials(vu.Context(), httpCredentials) + creds, err := ParseCredentials(rt, httpCredentials) if err != nil { return nil, fmt.Errorf("parsing httpCredentials options: %w", err) } diff --git a/browser/browser_context_options.go b/browser/browser_context_options.go index 71149ae78..2192ebbb2 100644 --- a/browser/browser_context_options.go +++ b/browser/browser_context_options.go @@ -1,13 +1,11 @@ package browser import ( - "context" "fmt" "github.com/grafana/sobek" "github.com/grafana/xk6-browser/common" - "github.com/grafana/xk6-browser/k6ext" ) const ( @@ -17,7 +15,7 @@ const ( // ParseBrowserContextOptions parses the browser context options. func ParseBrowserContextOptions( //nolint:funlen,gocognit,cyclop - ctx context.Context, + rt *sobek.Runtime, opts sobek.Value, ) (*common.BrowserContextOptions, error) { popts := common.NewBrowserContextOptions() @@ -26,7 +24,6 @@ func ParseBrowserContextOptions( //nolint:funlen,gocognit,cyclop return popts, nil // return the default options } - rt := k6ext.Runtime(ctx) o := opts.ToObject(rt) for _, k := range o.Keys() { switch k { @@ -51,7 +48,7 @@ func ParseBrowserContextOptions( //nolint:funlen,gocognit,cyclop popts.ExtraHTTPHeaders[k] = headers.Get(k).String() } case "geolocation": - geoloc, err := ParseGeolocation(ctx, o.Get(k).ToObject(rt)) + geoloc, err := ParseGeolocation(rt, o.Get(k).ToObject(rt)) if err != nil { return nil, fmt.Errorf("parsing geolocation options: %w", err) } @@ -59,7 +56,7 @@ func ParseBrowserContextOptions( //nolint:funlen,gocognit,cyclop case "hasTouch": popts.HasTouch = o.Get(k).ToBoolean() case "httpCredentials": - creds, err := ParseCredentials(ctx, o.Get(k).ToObject(rt)) + creds, err := ParseCredentials(rt, o.Get(k).ToObject(rt)) if err != nil { return nil, fmt.Errorf("parsing httpCredentials options: %w", err) } @@ -88,7 +85,7 @@ func ParseBrowserContextOptions( //nolint:funlen,gocognit,cyclop popts.ReducedMotion = common.ReducedMotionNoPreference } case "screen": - screen, err := ParseScreen(ctx, o.Get(k).ToObject(rt)) + screen, err := ParseScreen(rt, o.Get(k).ToObject(rt)) if err != nil { return nil, fmt.Errorf("parsing screen options: %w", err) } @@ -98,7 +95,7 @@ func ParseBrowserContextOptions( //nolint:funlen,gocognit,cyclop case "userAgent": popts.UserAgent = o.Get(k).String() case "viewport": - vp, err := ParseViewport(ctx, o.Get(k).ToObject(rt)) + vp, err := ParseViewport(rt, o.Get(k).ToObject(rt)) if err != nil { return nil, fmt.Errorf("parsing viewport options: %w", err) } @@ -110,14 +107,14 @@ func ParseBrowserContextOptions( //nolint:funlen,gocognit,cyclop } // ParseGeolocation parses the geolocation. -func ParseGeolocation(ctx context.Context, opts sobek.Value) (*common.Geolocation, error) { +func ParseGeolocation(rt *sobek.Runtime, opts sobek.Value) (*common.Geolocation, error) { var geoloc common.Geolocation if !sobekValueExists(opts) { return &geoloc, nil // return the default options } - o := opts.ToObject(k6ext.Runtime(ctx)) + o := opts.ToObject(rt) for _, k := range o.Keys() { switch k { case "accuracy": @@ -133,14 +130,14 @@ func ParseGeolocation(ctx context.Context, opts sobek.Value) (*common.Geolocatio } // ParseCredentials parses the credentials. -func ParseCredentials(ctx context.Context, opts sobek.Value) (*common.Credentials, error) { +func ParseCredentials(rt *sobek.Runtime, opts sobek.Value) (*common.Credentials, error) { var creds common.Credentials if !sobekValueExists(opts) { return &creds, nil // return the default options } - o := opts.ToObject(k6ext.Runtime(ctx)) + o := opts.ToObject(rt) for _, k := range o.Keys() { switch k { case "username": @@ -154,14 +151,14 @@ func ParseCredentials(ctx context.Context, opts sobek.Value) (*common.Credential } // ParseScreen parses the screen options. -func ParseScreen(ctx context.Context, opts sobek.Value) (*common.Screen, error) { +func ParseScreen(rt *sobek.Runtime, opts sobek.Value) (*common.Screen, error) { var screen common.Screen if !sobekValueExists(opts) { return &screen, nil // return the default options } - o := opts.ToObject(k6ext.Runtime(ctx)) + o := opts.ToObject(rt) for _, k := range o.Keys() { switch k { case optionWidth: @@ -175,14 +172,14 @@ func ParseScreen(ctx context.Context, opts sobek.Value) (*common.Screen, error) } // ParseViewport parses the viewport options. -func ParseViewport(ctx context.Context, opts sobek.Value) (*common.Viewport, error) { +func ParseViewport(rt *sobek.Runtime, opts sobek.Value) (*common.Viewport, error) { var viewport common.Viewport if !sobekValueExists(opts) { return &viewport, nil // return the default options } - o := opts.ToObject(k6ext.Runtime(ctx)) + o := opts.ToObject(rt) for _, k := range o.Keys() { switch k { case optionWidth: diff --git a/browser/browser_context_options_test.go b/browser/browser_context_options_test.go index 03171374b..7eb0f77bd 100644 --- a/browser/browser_context_options_test.go +++ b/browser/browser_context_options_test.go @@ -11,7 +11,8 @@ import ( func TestBrowserContextOptionsPermissions(t *testing.T) { vu := k6test.NewVU(t) - opts, err := ParseBrowserContextOptions(vu.Context(), + opts, err := ParseBrowserContextOptions( + vu.Runtime(), vu.ToSobekValue((struct { Permissions []any `js:"permissions"` }{ diff --git a/browser/browser_mapping.go b/browser/browser_mapping.go index ad09b073a..a32837e51 100644 --- a/browser/browser_mapping.go +++ b/browser/browser_mapping.go @@ -36,7 +36,7 @@ func mapBrowser(vu moduleVU) mapping { //nolint:funlen,cyclop,gocognit return b.IsConnected(), nil }, "newContext": func(opts sobek.Value) (*sobek.Promise, error) { - popts, err := ParseBrowserContextOptions(vu.Context(), opts) + popts, err := ParseBrowserContextOptions(vu.Runtime(), opts) if err != nil { return nil, fmt.Errorf("parsing browser.newContext options: %w", err) } @@ -74,7 +74,7 @@ func mapBrowser(vu moduleVU) mapping { //nolint:funlen,cyclop,gocognit return b.Version(), nil }, "newPage": func(opts sobek.Value) (*sobek.Promise, error) { - popts, err := ParseBrowserContextOptions(vu.Context(), opts) + popts, err := ParseBrowserContextOptions(vu.Runtime(), opts) if err != nil { return nil, fmt.Errorf("parsing browser.newPage options: %w", err) } diff --git a/browser/sync_browser_context_mapping.go b/browser/sync_browser_context_mapping.go index 6661855ba..94413b823 100644 --- a/browser/sync_browser_context_mapping.go +++ b/browser/sync_browser_context_mapping.go @@ -58,7 +58,7 @@ func syncMapBrowserContext(vu moduleVU, bc *common.BrowserContext) mapping { //n "setDefaultTimeout": bc.SetDefaultTimeout, "setGeolocation": bc.SetGeolocation, "setHTTPCredentials": func(httpCredentials sobek.Value) error { - creds, err := ParseCredentials(vu.Context(), httpCredentials) + creds, err := ParseCredentials(rt, httpCredentials) if err != nil { return fmt.Errorf("parsing httpCredentials options: %w", err) } diff --git a/browser/sync_browser_mapping.go b/browser/sync_browser_mapping.go index 97ea57829..c3580cbed 100644 --- a/browser/sync_browser_mapping.go +++ b/browser/sync_browser_mapping.go @@ -32,7 +32,7 @@ func syncMapBrowser(vu moduleVU) mapping { //nolint:funlen,cyclop return b.IsConnected(), nil }, "newContext": func(opts sobek.Value) (*sobek.Object, error) { - popts, err := ParseBrowserContextOptions(vu.Context(), opts) + popts, err := ParseBrowserContextOptions(rt, opts) if err != nil { return nil, fmt.Errorf("parsing browser.newContext options: %w", err) } @@ -71,7 +71,7 @@ func syncMapBrowser(vu moduleVU) mapping { //nolint:funlen,cyclop return b.Version(), nil }, "newPage": func(opts sobek.Value) (mapping, error) { - popts, err := ParseBrowserContextOptions(vu.Context(), opts) + popts, err := ParseBrowserContextOptions(rt, opts) if err != nil { return nil, fmt.Errorf("parsing browser.newPage options: %w", err) }