Refactor cloud and remote browser flags

Removes the cloud flag as it's overlapped by the remote flag.
Resolves: #800 (comment)

chromium/browser_type.go

@@ -68,7 +68,13 @@ func (b *BrowserType) init(
 		return nil, nil, nil, fmt.Errorf("error setting up logger: %w", err)
 	}
 
-	launchOpts := common.NewLaunchOptions(k6ext.OnCloud(), isRemoteBrowser)
+	var launchOpts *common.LaunchOptions
+	if isRemoteBrowser {
+		launchOpts = common.NewRemoteBrowserLaunchOptions()
+	} else {
+		launchOpts = common.NewLaunchOptions()
+	}
+
 	if err = launchOpts.Parse(ctx, logger, opts); err != nil {
 		return nil, nil, nil, fmt.Errorf("error parsing launch options: %w", err)
 	}

common/browser_options.go

@@ -46,7 +46,6 @@ 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
 }
 
@@ -57,14 +56,24 @@ type LaunchPersistentContextOptions struct {
 }
 
 // NewLaunchOptions returns a new LaunchOptions.
-func NewLaunchOptions(onCloud, isRemoteBrowser bool) *LaunchOptions {
+func NewLaunchOptions() *LaunchOptions {
 	return &LaunchOptions{
 		Env:               make(map[string]string),
 		Headless:          true,
 		LogCategoryFilter: ".*",
 		Timeout:           DefaultTimeout,
-		onCloud:           onCloud,
-		isRemoteBrowser:   isRemoteBrowser,
+	}
+}
+
+// NewRemoteBrowserLaunchOptions returns a new LaunchOptions
+// for a browser running in a remote machine.
+func NewRemoteBrowserLaunchOptions() *LaunchOptions {
+	return &LaunchOptions{
+		Env:               make(map[string]string),
+		Headless:          true,
+		LogCategoryFilter: ".*",
+		Timeout:           DefaultTimeout,
+		isRemoteBrowser:   true,
 	}
 }
 
@@ -85,8 +94,8 @@ func (l *LaunchOptions) Parse(ctx context.Context, logger *log.Logger, opts goja
 		}
 	)
 	for _, k := range o.Keys() {
-		if l.shouldIgnoreOpt(k) {
-			logger.Warnf("LaunchOptions", "setting %s option is disallowed.", k)
+		if l.shouldIgnoreIfBrowserIsRemote(k) {
+			logger.Warnf("LaunchOptions", "setting %s option is disallowed when browser is remote", k)
 			continue
 		}
 		v := o.Get(k)
@@ -129,32 +138,11 @@ func (l *LaunchOptions) Parse(ctx context.Context, logger *log.Logger, opts goja
 	return nil
 }
 
-func (l *LaunchOptions) shouldIgnoreOpt(opt string) bool {
-	var ignore 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 {
-		ignore = l.shouldIgnoreOnCloud(opt)
-	} else if l.isRemoteBrowser {
-		ignore = l.shouldIgnoreIfBrowserIsRemote(opt)
-	}
-
-	return ignore
-}
-
-func (l *LaunchOptions) shouldIgnoreOnCloud(opt string) bool {
-	shouldIgnoreOnCloud := map[string]struct{}{
-		optDevTools:       {},
-		optExecutablePath: {},
-		optHeadless:       {},
+func (l *LaunchOptions) shouldIgnoreIfBrowserIsRemote(opt string) bool {
+	if !l.isRemoteBrowser {
+		return false
 	}
-	_, ignore := shouldIgnoreOnCloud[opt]
 
-	return ignore
-}
-
-func (l *LaunchOptions) shouldIgnoreIfBrowserIsRemote(opt string) bool {
 	shouldIgnoreIfBrowserIsRemote := map[string]struct{}{
 		optArgs:              {},
 		optDevTools:          {},

common/browser_options_test.go

@@ -25,7 +25,6 @@ func TestBrowserLaunchOptionsParse(t *testing.T) {
 		opts            map[string]any
 		assert          func(testing.TB, *LaunchOptions)
 		err             string
-		onCloud         bool
 		isRemoteBrowser bool
 	}{
 		"defaults": {
@@ -42,44 +41,6 @@ func TestBrowserLaunchOptionsParse(t *testing.T) {
 				assert.Equal(t, defaultOptions, lo)
 			},
 		},
-		"defaults_on_cloud": {
-			onCloud: true,
-			opts: map[string]any{
-				// disallow changing the following opts
-				"headless":       false,
-				"devtools":       true,
-				"executablePath": "something else",
-				// allow changing the following opts
-				"args":              []string{"any"},
-				"debug":             true,
-				"env":               map[string]string{"some": "thing"},
-				"ignoreDefaultArgs": []string{"any"},
-				"logCategoryFilter": "...",
-				"proxy":             ProxyOptions{Server: "srv"},
-				"slowMo":            time.Second,
-				"timeout":           time.Second,
-			},
-			assert: func(tb testing.TB, lo *LaunchOptions) {
-				tb.Helper()
-				assert.Equal(t, &LaunchOptions{
-					// disallowed:
-					Headless:       true,
-					Devtools:       false,
-					ExecutablePath: "",
-					// allowed:
-					Args:              []string{"any"},
-					Debug:             true,
-					Env:               map[string]string{"some": "thing"},
-					IgnoreDefaultArgs: []string{"any"},
-					LogCategoryFilter: "...",
-					Proxy:             ProxyOptions{Server: "srv"},
-					SlowMo:            time.Second,
-					Timeout:           time.Second,
-
-					onCloud: true,
-				}, lo)
-			},
-		},
 		"defaults_remote_browser": {
 			isRemoteBrowser: true,
 			opts: map[string]any{
@@ -294,8 +255,15 @@ func TestBrowserLaunchOptionsParse(t *testing.T) {
 			t.Parallel()
 			var (
 				vu = k6test.NewVU(t)
-				lo = NewLaunchOptions(tt.onCloud, tt.isRemoteBrowser)
+				lo *LaunchOptions
 			)
+
+			if tt.isRemoteBrowser {
+				lo = NewRemoteBrowserLaunchOptions()
+			} else {
+				lo = NewLaunchOptions()
+			}
+
 			err := lo.Parse(vu.Context(), log.NewNullLogger(), vu.ToGojaValue(tt.opts))
 			if tt.err != "" {
 				require.ErrorContains(t, err, tt.err)

common/browser_test.go

@@ -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(), 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{