Skip to content

Commit

Permalink
Refactor cloud and remote browser flags
Browse files Browse the repository at this point in the history
Define constants for the different modes for both flags so it improves
readability.
Resolves:
#800 (comment).
Resolves:
#800 (comment).
  • Loading branch information
ka3de committed Mar 1, 2023
1 parent c5c2aeb commit c421358
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 22 deletions.
13 changes: 9 additions & 4 deletions chromium/browser_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func NewBrowserType(vu k6modules.VU) api.BrowserType {
}

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

Expand All @@ -68,7 +68,12 @@ func (b *BrowserType) init(
return nil, nil, nil, fmt.Errorf("error setting up logger: %w", err)
}

launchOpts := common.NewLaunchOptions(k6ext.OnCloud(), isRemoteBrowser)
cloudMode := common.CloudModeOff
if k6ext.OnCloud() {
cloudMode = common.CloudModeOn
}

launchOpts := common.NewLaunchOptions(cloudMode, browserMode)
if err = launchOpts.Parse(ctx, logger, opts); err != nil {
return nil, nil, nil, fmt.Errorf("error parsing launch options: %w", err)
}
Expand All @@ -94,7 +99,7 @@ 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)
ctx, launchOpts, logger, err := b.init(opts, common.BrowserModeRemote)
if err != nil {
k6ext.Panic(ctx, "initializing browser type: %w", err)
}
Expand Down Expand Up @@ -150,7 +155,7 @@ 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)
ctx, launchOpts, logger, err := b.init(opts, common.BrowserModeLocal)
if err != nil {
k6ext.Panic(ctx, "initializing browser type: %w", err)
}
Expand Down
34 changes: 27 additions & 7 deletions common/browser_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,26 @@ const (
optTimeout = "timeout"
)

const (
// CloudModeOff indicates that execution is NOT running in the cloud.
CloudModeOff CloudMode = iota
// CloudModeOn indicates that execution is running in the cloud.
CloudModeOn
)

// CloudMode indicates wether execution is running in the cloud.
type CloudMode int

const (
// BrowserModeLocal indicates that the browser is running in the local machine.
BrowserModeLocal BrowserMode = iota
// BrowserModeRemote indicates that the browser is running in a remote machine.
BrowserModeRemote
)

// BrowserMode indicates wether browser is local or remote.
type BrowserMode int

// ProxyOptions allows configuring a proxy server.
type ProxyOptions struct {
Server string
Expand All @@ -46,8 +66,8 @@ type LaunchOptions struct {
SlowMo time.Duration
Timeout time.Duration

onCloud bool // some options will be ignored when running in the cloud
isRemoteBrowser bool // some options will be ignored if browser is in a remote machine
cloudMode CloudMode // some options will be ignored when running in the cloud
browserMode BrowserMode // some options will be ignored if browser is in a remote machine
}

// LaunchPersistentContextOptions stores browser launch options for persistent context.
Expand All @@ -57,14 +77,14 @@ type LaunchPersistentContextOptions struct {
}

// NewLaunchOptions returns a new LaunchOptions.
func NewLaunchOptions(onCloud, isRemoteBrowser bool) *LaunchOptions {
func NewLaunchOptions(cloudMode CloudMode, browserMode BrowserMode) *LaunchOptions {
return &LaunchOptions{
Env: make(map[string]string),
Headless: true,
LogCategoryFilter: ".*",
Timeout: DefaultTimeout,
onCloud: onCloud,
isRemoteBrowser: isRemoteBrowser,
cloudMode: cloudMode,
browserMode: browserMode,
}
}

Expand Down Expand Up @@ -134,9 +154,9 @@ func (l *LaunchOptions) shouldIgnoreOpt(opt string) bool {
// TODO: Depending on future browser setup in the cloud it
// might be necessary to deprecate the current 'onCloud' ignored
// options and only use the ones for 'remote browser'.
if l.onCloud {
if l.cloudMode == CloudModeOn {
ignore = l.shouldIgnoreOnCloud(opt)
} else if l.isRemoteBrowser {
} else if l.browserMode == BrowserModeRemote {
ignore = l.shouldIgnoreIfBrowserIsRemote(opt)
}

Expand Down
20 changes: 10 additions & 10 deletions common/browser_options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ func TestBrowserLaunchOptionsParse(t *testing.T) {
}

for name, tt := range map[string]struct {
opts map[string]any
assert func(testing.TB, *LaunchOptions)
err string
onCloud bool
isRemoteBrowser bool
opts map[string]any
assert func(testing.TB, *LaunchOptions)
err string
cloudMode CloudMode
browserMode BrowserMode
}{
"defaults": {
opts: map[string]any{},
Expand All @@ -43,7 +43,7 @@ func TestBrowserLaunchOptionsParse(t *testing.T) {
},
},
"defaults_on_cloud": {
onCloud: true,
cloudMode: CloudModeOn,
opts: map[string]any{
// disallow changing the following opts
"headless": false,
Expand Down Expand Up @@ -76,12 +76,12 @@ func TestBrowserLaunchOptionsParse(t *testing.T) {
SlowMo: time.Second,
Timeout: time.Second,

onCloud: true,
cloudMode: CloudModeOn,
}, lo)
},
},
"defaults_remote_browser": {
isRemoteBrowser: true,
browserMode: BrowserModeRemote,
opts: map[string]any{
// disallow changing the following opts
"args": []string{"any"},
Expand Down Expand Up @@ -109,7 +109,7 @@ func TestBrowserLaunchOptionsParse(t *testing.T) {
SlowMo: time.Second,
Timeout: time.Second,

isRemoteBrowser: true,
browserMode: BrowserModeRemote,
}, lo)
},
},
Expand Down Expand Up @@ -294,7 +294,7 @@ func TestBrowserLaunchOptionsParse(t *testing.T) {
t.Parallel()
var (
vu = k6test.NewVU(t)
lo = NewLaunchOptions(tt.onCloud, tt.isRemoteBrowser)
lo = NewLaunchOptions(tt.cloudMode, tt.browserMode)
)
err := lo.Parse(vu.Context(), log.NewNullLogger(), vu.ToGojaValue(tt.opts))
if tt.err != "" {
Expand Down
2 changes: 1 addition & 1 deletion common/browser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func TestBrowserNewPageInContext(t *testing.T) {
newTestCase := func(id cdp.BrowserContextID) *testCase {
ctx, cancel := context.WithCancel(context.Background())
logger := log.NewNullLogger()
b := newBrowser(ctx, cancel, nil, NewLaunchOptions(false, false), logger)
b := newBrowser(ctx, cancel, nil, NewLaunchOptions(CloudModeOff, BrowserModeLocal), logger)
// set a new browser context in the browser with `id`, so that newPageInContext can find it.
b.contexts[id] = NewBrowserContext(ctx, b, id, nil, nil)
return &testCase{
Expand Down

0 comments on commit c421358

Please sign in to comment.