Skip to content

Commit

Permalink
Merge pull request #876 from grafana/update/858-validate-browser-opts
Browse files Browse the repository at this point in the history 
Refactor browser options
  • Loading branch information
@ka3de
ka3de committed May 18, 2023
2 parents be6980d + a6b5b60 commit 4193ba7
Show file tree
Hide file tree
Showing 51 changed files with 763 additions and 453 deletions.
4 changes: 2 additions & 2 deletions api/browser_type.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ import (

// BrowserType is the public interface of a CDP browser client.
type BrowserType interface {
Connect(wsEndpoint string, opts goja.Value) Browser
Connect(wsEndpoint string) Browser
ExecutablePath() string
Launch(opts goja.Value) (_ Browser, browserProcessID int)
Launch() (_ Browser, browserProcessID int)
LaunchPersistentContext(userDataDir string, opts goja.Value) Browser
Name() string
}
9 changes: 5 additions & 4 deletions browser/mapping.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

"github.com/grafana/xk6-browser/api"
"github.com/grafana/xk6-browser/chromium"
"github.com/grafana/xk6-browser/env"
"github.com/grafana/xk6-browser/k6error"
"github.com/grafana/xk6-browser/k6ext"

Expand All @@ -36,7 +37,7 @@ func mapBrowserToGoja(vu moduleVU) *goja.Object {
obj = rt.NewObject()
// TODO: Use k6 LookupEnv instead of OS package methods.
// See https://github.com/grafana/xk6-browser/issues/822.
wsURL, isRemoteBrowser = k6ext.IsRemoteBrowser(os.LookupEnv)
wsURL, isRemoteBrowser = env.IsRemoteBrowser(os.LookupEnv)
browserType = chromium.NewBrowserType(vu)
)
for k, v := range mapBrowserType(vu, browserType, wsURL, isRemoteBrowser) {
Expand Down Expand Up @@ -714,7 +715,7 @@ func mapBrowserType(vu moduleVU, bt api.BrowserType, wsURL string, isRemoteBrows
rt := vu.Runtime()
return mapping{
"connect": func(wsEndpoint string, opts goja.Value) *goja.Object {
b := bt.Connect(wsEndpoint, opts)
b := bt.Connect(wsEndpoint)
m := mapBrowser(vu, b)
return rt.ToValue(m).ToObject(rt)
},
Expand All @@ -726,11 +727,11 @@ func mapBrowserType(vu moduleVU, bt api.BrowserType, wsURL string, isRemoteBrows
// to connect and avoid storing the browser pid
// as we have no access to it.
if isRemoteBrowser {
m := mapBrowser(vu, bt.Connect(wsURL, opts))
m := mapBrowser(vu, bt.Connect(wsURL))
return rt.ToValue(m).ToObject(rt)
}

b, pid := bt.Launch(opts)
b, pid := bt.Launch()
// store the pid so we can kill it later on panic.
vu.registerPid(pid)
m := mapBrowser(vu, b)
Expand Down
75 changes: 42 additions & 33 deletions chromium/browser_type.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (

"github.com/grafana/xk6-browser/api"
"github.com/grafana/xk6-browser/common"
"github.com/grafana/xk6-browser/env"
"github.com/grafana/xk6-browser/k6ext"
"github.com/grafana/xk6-browser/log"
"github.com/grafana/xk6-browser/storage"
Expand All @@ -34,12 +35,13 @@ var _ api.BrowserType = &BrowserType{}
type BrowserType struct {
// FIXME: This is only exported because testBrowser needs it. Contexts
// shouldn't be stored on structs if we can avoid it.
Ctx context.Context
vu k6modules.VU
hooks *common.Hooks
k6Metrics *k6ext.CustomMetrics
execPath string // path to the Chromium executable
randSrc *rand.Rand
Ctx context.Context
vu k6modules.VU
hooks *common.Hooks
k6Metrics *k6ext.CustomMetrics
execPath string // path to the Chromium executable
randSrc *rand.Rand
envLookupper env.LookupFunc
}

// NewBrowserType registers our custom k6 metrics, creates method mappings on
Expand All @@ -49,45 +51,47 @@ func NewBrowserType(vu k6modules.VU) api.BrowserType {
// otherwise it will return nil.
k6m := k6ext.RegisterCustomMetrics(vu.InitEnv().Registry)
b := BrowserType{
vu: vu,
hooks: common.NewHooks(),
k6Metrics: k6m,
randSrc: rand.New(rand.NewSource(time.Now().UnixNano())), //nolint: gosec
vu: vu,
hooks: common.NewHooks(),
k6Metrics: k6m,
randSrc: rand.New(rand.NewSource(time.Now().UnixNano())), //nolint: gosec
envLookupper: os.LookupEnv,
}

return &b
}

func (b *BrowserType) init(
opts goja.Value, isRemoteBrowser bool,
) (context.Context, *common.LaunchOptions, *log.Logger, error) {
isRemoteBrowser bool,
) (context.Context, *common.BrowserOptions, *log.Logger, error) {
ctx := b.initContext()

logger, err := makeLogger(ctx)
if err != nil {
return nil, nil, nil, fmt.Errorf("error setting up logger: %w", err)
}

var launchOpts *common.LaunchOptions
var browserOpts *common.BrowserOptions
if isRemoteBrowser {
launchOpts = common.NewRemoteBrowserLaunchOptions()
browserOpts = common.NewRemoteBrowserOptions()
} else {
launchOpts = common.NewLaunchOptions()
browserOpts = common.NewLocalBrowserOptions()
}

if err = launchOpts.Parse(ctx, logger, opts); err != nil {
return nil, nil, nil, fmt.Errorf("error parsing launch options: %w", err)
opts := k6ext.GetScenarioOpts(b.vu.Context(), b.vu)
if err = browserOpts.Parse(ctx, logger, opts, b.envLookupper); err != nil {
return nil, nil, nil, fmt.Errorf("error parsing browser options: %w", err)
}
ctx = common.WithLaunchOptions(ctx, launchOpts)
ctx = common.WithBrowserOptions(ctx, browserOpts)

if err := logger.SetCategoryFilter(launchOpts.LogCategoryFilter); err != nil {
if err := logger.SetCategoryFilter(browserOpts.LogCategoryFilter); err != nil {
return nil, nil, nil, fmt.Errorf("error setting category filter: %w", err)
}
if launchOpts.Debug {
if browserOpts.Debug {
_ = logger.SetLevel("debug")
}

return ctx, launchOpts, logger, nil
return ctx, browserOpts, logger, nil
}

func (b *BrowserType) initContext() context.Context {
Expand All @@ -99,17 +103,17 @@ func (b *BrowserType) initContext() context.Context {
}

// Connect attaches k6 browser to an existing browser instance.
func (b *BrowserType) Connect(wsEndpoint string, opts goja.Value) api.Browser {
ctx, launchOpts, logger, err := b.init(opts, true)
func (b *BrowserType) Connect(wsEndpoint string) api.Browser {
ctx, browserOpts, logger, err := b.init(true)
if err != nil {
k6ext.Panic(ctx, "initializing browser type: %w", err)
}

bp, err := b.connect(ctx, wsEndpoint, launchOpts, logger)
bp, err := b.connect(ctx, wsEndpoint, browserOpts, logger)
if err != nil {
err = &k6ext.UserFriendlyError{
Err: err,
Timeout: launchOpts.Timeout,
Timeout: browserOpts.Timeout,
}
k6ext.Panic(ctx, "%w", err)
}
Expand All @@ -118,7 +122,7 @@ func (b *BrowserType) Connect(wsEndpoint string, opts goja.Value) api.Browser {
}

func (b *BrowserType) connect(
ctx context.Context, wsURL string, opts *common.LaunchOptions, logger *log.Logger,
ctx context.Context, wsURL string, opts *common.BrowserOptions, logger *log.Logger,
) (*common.Browser, error) {
browserProc, err := b.link(ctx, wsURL, logger)
if browserProc == nil {
Expand Down Expand Up @@ -154,17 +158,17 @@ func (b *BrowserType) link(

// Launch allocates a new Chrome browser process and returns a new api.Browser value,
// which can be used for controlling the Chrome browser.
func (b *BrowserType) Launch(opts goja.Value) (_ api.Browser, browserProcessID int) {
ctx, launchOpts, logger, err := b.init(opts, false)
func (b *BrowserType) Launch() (_ api.Browser, browserProcessID int) {
ctx, browserOpts, logger, err := b.init(false)
if err != nil {
k6ext.Panic(ctx, "initializing browser type: %w", err)
}

bp, pid, err := b.launch(ctx, launchOpts, logger)
bp, pid, err := b.launch(ctx, browserOpts, logger)
if err != nil {
err = &k6ext.UserFriendlyError{
Err: err,
Timeout: launchOpts.Timeout,
Timeout: browserOpts.Timeout,
}
k6ext.Panic(ctx, "%w", err)
}
Expand All @@ -173,7 +177,7 @@ func (b *BrowserType) Launch(opts goja.Value) (_ api.Browser, browserProcessID i
}

func (b *BrowserType) launch(
ctx context.Context, opts *common.LaunchOptions, logger *log.Logger,
ctx context.Context, opts *common.BrowserOptions, logger *log.Logger,
) (_ *common.Browser, pid int, _ error) {
flags, err := prepareFlags(opts, &(b.vu.State()).Options)
if err != nil {
Expand Down Expand Up @@ -231,7 +235,7 @@ func (b *BrowserType) Name() string {

// allocate starts a new Chromium browser process and returns it.
func (b *BrowserType) allocate(
ctx context.Context, opts *common.LaunchOptions,
ctx context.Context, opts *common.BrowserOptions,
flags map[string]any, dataDir *storage.Dir,
logger *log.Logger,
) (_ *common.BrowserProcess, rerr error) {
Expand Down Expand Up @@ -295,6 +299,11 @@ func (b *BrowserType) ExecutablePath() (execPath string) {
return ""
}

// SetEnvLookupper sets the environment variable lookupper function.
func (b *BrowserType) SetEnvLookupper(envLookupper env.LookupFunc) {
b.envLookupper = envLookupper
}

// parseArgs parses command-line arguments and returns them.
func parseArgs(flags map[string]any) ([]string, error) {
// Build command line args list
Expand Down Expand Up @@ -322,7 +331,7 @@ func parseArgs(flags map[string]any) ([]string, error) {
return args, nil
}

func prepareFlags(lopts *common.LaunchOptions, k6opts *k6lib.Options) (map[string]any, error) {
func prepareFlags(lopts *common.BrowserOptions, k6opts *k6lib.Options) (map[string]any, error) {
// After Puppeteer's and Playwright's default behavior.
f := map[string]any{
"disable-background-networking": true,
Expand Down
24 changes: 12 additions & 12 deletions chromium/browser_type_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,52 +28,52 @@ func TestBrowserTypePrepareFlags(t *testing.T) {

testCases := []struct {
flag string
changeOpts *common.LaunchOptions
changeOpts *common.BrowserOptions
changeK6Opts *k6lib.Options
expInitVal, expChangedVal any
post func(t *testing.T, flags map[string]any)
}{
{
flag: "hide-scrollbars",
changeOpts: &common.LaunchOptions{IgnoreDefaultArgs: []string{"hide-scrollbars"}, Headless: true},
changeOpts: &common.BrowserOptions{IgnoreDefaultArgs: []string{"hide-scrollbars"}, Headless: true},
},
{
flag: "hide-scrollbars",
changeOpts: &common.LaunchOptions{Headless: true},
changeOpts: &common.BrowserOptions{Headless: true},
expChangedVal: true,
},
{
flag: "browser-arg",
expInitVal: nil,
changeOpts: &common.LaunchOptions{Args: []string{"browser-arg=value"}},
changeOpts: &common.BrowserOptions{Args: []string{"browser-arg=value"}},
expChangedVal: "value",
},
{
flag: "browser-arg-flag",
expInitVal: nil,
changeOpts: &common.LaunchOptions{Args: []string{"browser-arg-flag"}},
changeOpts: &common.BrowserOptions{Args: []string{"browser-arg-flag"}},
expChangedVal: "",
},
{
flag: "browser-arg-trim-double-quote",
expInitVal: nil,
changeOpts: &common.LaunchOptions{Args: []string{
changeOpts: &common.BrowserOptions{Args: []string{
` browser-arg-trim-double-quote = "value " `,
}},
expChangedVal: "value ",
},
{
flag: "browser-arg-trim-single-quote",
expInitVal: nil,
changeOpts: &common.LaunchOptions{Args: []string{
changeOpts: &common.BrowserOptions{Args: []string{
` browser-arg-trim-single-quote=' value '`,
}},
expChangedVal: " value ",
},
{
flag: "browser-args",
expInitVal: nil,
changeOpts: &common.LaunchOptions{Args: []string{
changeOpts: &common.BrowserOptions{Args: []string{
"browser-arg1='value1", "browser-arg2=''value2''", "browser-flag",
}},
post: func(t *testing.T, flags map[string]any) {
Expand All @@ -87,7 +87,7 @@ func TestBrowserTypePrepareFlags(t *testing.T) {
{
flag: "host-resolver-rules",
expInitVal: nil,
changeOpts: &common.LaunchOptions{Args: []string{
changeOpts: &common.BrowserOptions{Args: []string{
`host-resolver-rules="MAP * www.example.com, EXCLUDE *.youtube.*"`,
}},
changeK6Opts: &k6lib.Options{
Expand All @@ -99,14 +99,14 @@ func TestBrowserTypePrepareFlags(t *testing.T) {
{
flag: "host-resolver-rules",
expInitVal: nil,
changeOpts: &common.LaunchOptions{},
changeOpts: &common.BrowserOptions{},
changeK6Opts: &k6lib.Options{},
expChangedVal: nil,
},
{
flag: "headless",
expInitVal: false,
changeOpts: &common.LaunchOptions{Headless: true},
changeOpts: &common.BrowserOptions{Headless: true},
expChangedVal: true,
post: func(t *testing.T, flags map[string]any) {
t.Helper()
Expand All @@ -124,7 +124,7 @@ func TestBrowserTypePrepareFlags(t *testing.T) {
t.Run(tc.flag, func(t *testing.T) {
t.Parallel()

flags, err := prepareFlags(&common.LaunchOptions{}, nil)
flags, err := prepareFlags(&common.BrowserOptions{}, nil)
require.NoError(t, err, "failed to prepare flags")

if tc.expInitVal != nil {
Expand Down
16 changes: 8 additions & 8 deletions common/browser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ type Browser struct {
state int64

browserProc *BrowserProcess
launchOpts *LaunchOptions
browserOpts *BrowserOptions

// Connection to the browser to talk CDP protocol.
// A *Connection is saved to this field, see: connect().
Expand Down Expand Up @@ -78,10 +78,10 @@ func NewBrowser(
ctx context.Context,
cancel context.CancelFunc,
browserProc *BrowserProcess,
launchOpts *LaunchOptions,
browserOpts *BrowserOptions,
logger *log.Logger,
) (*Browser, error) {
b := newBrowser(ctx, cancel, browserProc, launchOpts, logger)
b := newBrowser(ctx, cancel, browserProc, browserOpts, logger)
if err := b.connect(); err != nil {
return nil, err
}
Expand All @@ -93,7 +93,7 @@ func newBrowser(
ctx context.Context,
cancelFn context.CancelFunc,
browserProc *BrowserProcess,
launchOpts *LaunchOptions,
browserOpts *BrowserOptions,
logger *log.Logger,
) *Browser {
return &Browser{
Expand All @@ -102,7 +102,7 @@ func newBrowser(
cancelFn: cancelFn,
state: int64(BrowserStateOpen),
browserProc: browserProc,
launchOpts: launchOpts,
browserOpts: browserOpts,
contexts: make(map[cdp.BrowserContextID]*BrowserContext),
pages: make(map[target.ID]*Page),
sessionIDtoTargetID: make(map[target.SessionID]target.ID),
Expand Down Expand Up @@ -384,7 +384,7 @@ func (b *Browser) newPageInContext(id cdp.BrowserContextID) (*Page, error) {
return nil, fmt.Errorf("missing browser context: %s", id)
}

ctx, cancel := context.WithTimeout(b.ctx, b.launchOpts.Timeout)
ctx, cancel := context.WithTimeout(b.ctx, b.browserOpts.Timeout)
defer cancel()

// buffer of one is for sending the target ID whether an event handler
Expand Down Expand Up @@ -425,7 +425,7 @@ func (b *Browser) newPageInContext(id cdp.BrowserContextID) (*Page, error) {
case <-ctx.Done():
err = &k6ext.UserFriendlyError{
Err: ctx.Err(),
Timeout: b.launchOpts.Timeout,
Timeout: b.browserOpts.Timeout,
}
b.logger.Debugf("Browser:newPageInContext:<-ctx.Done", "tid:%v bctxid:%v err:%v", tid, id, err)
}
Expand Down Expand Up @@ -462,7 +462,7 @@ func (b *Browser) Close() {

// If the browser is not being executed remotely, send the Browser.close CDP
// command, which triggers the browser process to exit.
if !b.launchOpts.isRemoteBrowser {
if !b.browserOpts.isRemoteBrowser {
var closeErr *websocket.CloseError
err := cdpbrowser.Close().Do(cdp.WithExecutor(b.ctx, b.conn))
if err != nil && !errors.As(err, &closeErr) {
Expand Down
Loading

0 comments on commit 4193ba7

Please sign in to comment.