Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove support for browser env option #870

Merged
merged 3 commits into from
May 3, 2023
Merged

Remove support for browser env option #870

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
558452e
Remove support for browser env option
ka3de May 3, 2023
64e6522
Remove unused ctxCancel function
ka3de May 3, 2023
f1676b4
Merge branch 'main' into remove/869-browser-env-opt
ka3de May 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 3 additions & 7 deletions chromium/browser_type.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
Expand Down Expand Up @@ -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)
Expand All @@ -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.
Expand Down
8 changes: 0 additions & 8 deletions common/browser_options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
const (
optArgs = "args"
optDebug = "debug"
optEnv = "env"
optExecutablePath = "executablePath"
optHeadless = "headless"
optIgnoreDefaultArgs = "ignoreDefaultArgs"
Expand All @@ -35,7 +34,6 @@ type ProxyOptions struct {
type LaunchOptions struct {
Args []string
Debug bool
Env map[string]string
ExecutablePath string
Headless bool
IgnoreDefaultArgs []string
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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:
Expand Down Expand Up @@ -141,7 +134,6 @@ func (l *LaunchOptions) shouldIgnoreIfBrowserIsRemote(opt string) bool {

shouldIgnoreIfBrowserIsRemote := map[string]struct{}{
optArgs: {},
optEnv: {},
optExecutablePath: {},
optHeadless: {},
optIgnoreDefaultArgs: {},
Expand Down
18 changes: 0 additions & 18 deletions common/browser_options_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ func TestBrowserLaunchOptionsParse(t *testing.T) {
t.Parallel()

defaultOptions := &LaunchOptions{
Env: make(map[string]string),
Headless: true,
LogCategoryFilter: ".*",
Timeout: DefaultTimeout,
Expand Down Expand Up @@ -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"},
Expand All @@ -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,
Expand All @@ -75,15 +72,13 @@ 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,
},
assert: func(tb testing.TB, lo *LaunchOptions) {
tb.Helper()
assert.Equal(tb, &LaunchOptions{
Env: make(map[string]string),
Headless: true,
LogCategoryFilter: ".*",
Timeout: DefaultTimeout,
Expand Down Expand Up @@ -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",
Expand Down
11 changes: 3 additions & 8 deletions common/browser_process.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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...)
Expand All @@ -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()
Expand Down