From af0f9f0dc183618605ca56ecab4a176f12303535 Mon Sep 17 00:00:00 2001 From: ka3de Date: Wed, 3 May 2023 15:17:37 +0200 Subject: [PATCH] Remove support for browser env option (#870) A decision was made to no longer support the env browser option as this was not providing much value. --- chromium/browser_type.go | 10 +++------- common/browser_options.go | 8 -------- common/browser_options_test.go | 18 ------------------ common/browser_process.go | 11 +++-------- 4 files changed, 6 insertions(+), 41 deletions(-) diff --git a/chromium/browser_type.go b/chromium/browser_type.go index 89a980915..bad9d7ea5 100644 --- a/chromium/browser_type.go +++ b/chromium/browser_type.go @@ -176,10 +176,6 @@ func (b *BrowserType) Launch(opts goja.Value) (_ api.Browser, browserProcessID i func (b *BrowserType) launch( ctx context.Context, opts *common.LaunchOptions, logger *log.Logger, ) (_ *common.Browser, pid int, _ error) { - envs := make([]string, 0, len(opts.Env)) - for k, v := range opts.Env { - envs = append(envs, fmt.Sprintf("%s=%s", k, v)) - } flags, err := prepareFlags(opts, &(b.vu.State()).Options) if err != nil { return nil, 0, fmt.Errorf("%w", err) @@ -204,7 +200,7 @@ func (b *BrowserType) launch( <-c.Done() }(ctx) - browserProc, err := b.allocate(ctx, opts, flags, envs, dataDir, logger) + browserProc, err := b.allocate(ctx, opts, flags, dataDir, logger) if browserProc == nil { return nil, 0, fmt.Errorf("launching browser: %w", err) } @@ -237,7 +233,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, - flags map[string]any, env []string, dataDir *storage.Dir, + flags map[string]any, dataDir *storage.Dir, logger *log.Logger, ) (_ *common.BrowserProcess, rerr error) { bProcCtx, bProcCtxCancel := context.WithTimeout(ctx, opts.Timeout) @@ -257,7 +253,7 @@ func (b *BrowserType) allocate( path = b.ExecutablePath() } - return common.NewLocalBrowserProcess(bProcCtx, path, args, env, dataDir, bProcCtxCancel, logger) //nolint: wrapcheck + return common.NewLocalBrowserProcess(bProcCtx, path, args, dataDir, bProcCtxCancel, logger) //nolint: wrapcheck } // ExecutablePath returns the path where the extension expects to find the browser executable. diff --git a/common/browser_options.go b/common/browser_options.go index 997c5c769..fe07fcb8a 100644 --- a/common/browser_options.go +++ b/common/browser_options.go @@ -13,7 +13,6 @@ import ( const ( optArgs = "args" optDebug = "debug" - optEnv = "env" optExecutablePath = "executablePath" optHeadless = "headless" optIgnoreDefaultArgs = "ignoreDefaultArgs" @@ -35,7 +34,6 @@ type ProxyOptions struct { type LaunchOptions struct { Args []string Debug bool - Env map[string]string ExecutablePath string Headless bool IgnoreDefaultArgs []string @@ -56,7 +54,6 @@ type LaunchPersistentContextOptions struct { // NewLaunchOptions returns a new LaunchOptions. func NewLaunchOptions() *LaunchOptions { return &LaunchOptions{ - Env: make(map[string]string), Headless: true, LogCategoryFilter: ".*", Timeout: DefaultTimeout, @@ -67,7 +64,6 @@ func NewLaunchOptions() *LaunchOptions { // for a browser running in a remote machine. func NewRemoteBrowserLaunchOptions() *LaunchOptions { return &LaunchOptions{ - Env: make(map[string]string), Headless: true, LogCategoryFilter: ".*", Timeout: DefaultTimeout, @@ -85,7 +81,6 @@ func (l *LaunchOptions) Parse(ctx context.Context, logger *log.Logger, opts goja rt = k6ext.Runtime(ctx) o = opts.ToObject(rt) defaults = map[string]any{ - optEnv: l.Env, optHeadless: l.Headless, optLogCategoryFilter: l.LogCategoryFilter, optTimeout: l.Timeout, @@ -109,8 +104,6 @@ func (l *LaunchOptions) Parse(ctx context.Context, logger *log.Logger, opts goja err = exportOpt(rt, k, v, &l.Args) case optDebug: l.Debug, err = parseBoolOpt(k, v) - case optEnv: - err = exportOpt(rt, k, v, &l.Env) case optExecutablePath: l.ExecutablePath, err = parseStrOpt(k, v) case optHeadless: @@ -141,7 +134,6 @@ func (l *LaunchOptions) shouldIgnoreIfBrowserIsRemote(opt string) bool { shouldIgnoreIfBrowserIsRemote := map[string]struct{}{ optArgs: {}, - optEnv: {}, optExecutablePath: {}, optHeadless: {}, optIgnoreDefaultArgs: {}, diff --git a/common/browser_options_test.go b/common/browser_options_test.go index 9eb1f922e..480b23e06 100644 --- a/common/browser_options_test.go +++ b/common/browser_options_test.go @@ -15,7 +15,6 @@ func TestBrowserLaunchOptionsParse(t *testing.T) { t.Parallel() defaultOptions := &LaunchOptions{ - Env: make(map[string]string), Headless: true, LogCategoryFilter: ".*", Timeout: DefaultTimeout, @@ -46,7 +45,6 @@ func TestBrowserLaunchOptionsParse(t *testing.T) { opts: map[string]any{ // disallow changing the following opts "args": []string{"any"}, - "env": map[string]string{"some": "thing"}, "executablePath": "something else", "headless": false, "ignoreDefaultArgs": []string{"any"}, @@ -61,7 +59,6 @@ func TestBrowserLaunchOptionsParse(t *testing.T) { tb.Helper() assert.Equal(t, &LaunchOptions{ // disallowed: - Env: make(map[string]string), Headless: true, // allowed: Debug: true, @@ -75,7 +72,6 @@ func TestBrowserLaunchOptionsParse(t *testing.T) { }, "nulls": { // don't override the defaults on `null` opts: map[string]any{ - "env": nil, "headless": nil, "logCategoryFilter": nil, "timeout": nil, @@ -83,7 +79,6 @@ func TestBrowserLaunchOptionsParse(t *testing.T) { assert: func(tb testing.TB, lo *LaunchOptions) { tb.Helper() assert.Equal(tb, &LaunchOptions{ - Env: make(map[string]string), Headless: true, LogCategoryFilter: ".*", Timeout: DefaultTimeout, @@ -119,19 +114,6 @@ func TestBrowserLaunchOptionsParse(t *testing.T) { opts: map[string]any{"debug": "true"}, err: "debug should be a boolean", }, - "env": { - opts: map[string]any{ - "env": map[string]string{"key": "value"}, - }, - assert: func(tb testing.TB, lo *LaunchOptions) { - tb.Helper() - assert.Equal(t, map[string]string{"key": "value"}, lo.Env) - }, - }, - "env_err": { - opts: map[string]any{"env": 1}, - err: "env should be a map", - }, "executablePath": { opts: map[string]any{ "executablePath": "cmd/somewhere", diff --git a/common/browser_process.go b/common/browser_process.go index 47e0862ab..0a82bb566 100644 --- a/common/browser_process.go +++ b/common/browser_process.go @@ -35,10 +35,10 @@ type BrowserProcess struct { // NewLocalBrowserProcess starts a local browser process and // returns a new BrowserProcess instance to interact with it. func NewLocalBrowserProcess( - ctx context.Context, path string, args, env []string, dataDir *storage.Dir, + ctx context.Context, path string, args []string, dataDir *storage.Dir, ctxCancel context.CancelFunc, logger *log.Logger, ) (*BrowserProcess, error) { - cmd, err := execute(ctx, ctxCancel, path, args, env, dataDir, logger) + cmd, err := execute(ctx, path, args, dataDir, logger) if err != nil { return nil, err } @@ -151,7 +151,7 @@ type command struct { } func execute( - ctx context.Context, ctxCancel func(), path string, args, env []string, + ctx context.Context, path string, args []string, dataDir *storage.Dir, logger *log.Logger, ) (command, error) { cmd := exec.CommandContext(ctx, path, args...) @@ -166,11 +166,6 @@ func execute( return command{}, fmt.Errorf("%w", err) } - // Set up environment variable for process - if len(env) > 0 { - cmd.Env = append(os.Environ(), env...) - } - // We must start the cmd before calling cmd.Wait, as otherwise the two // can run into a data race. err = cmd.Start()