Refactor to k6 lookupEnv and abstract env. var. lookup

browser/module_test.go

@@ -7,6 +7,8 @@ import (
 	"github.com/dop251/goja"
 	"github.com/stretchr/testify/require"
 
+	"github.com/grafana/xk6-browser/env"
+
 	k6common "go.k6.io/k6/js/common"
 	k6modulestest "go.k6.io/k6/js/modulestest"
 	k6lib "go.k6.io/k6/lib"
@@ -23,10 +25,8 @@ func TestModuleNew(t *testing.T) {
 		RuntimeField: goja.New(),
 		InitEnvField: &k6common.InitEnvironment{
 			TestPreInitState: &k6lib.TestPreInitState{
-				Registry: k6metrics.NewRegistry(),
-				LookupEnv: func(key string) (string, bool) {
-					return "", false
-				},
+				Registry:  k6metrics.NewRegistry(),
+				LookupEnv: env.EmptyLookup,
 			},
 		},
 		CtxField: context.Background(),

common/browser_options_test.go

@@ -21,10 +21,6 @@ func TestBrowserOptionsParse(t *testing.T) { //nolint:gocognit
 		Timeout:           DefaultTimeout,
 	}
 
-	noopEnvLookuper := func(string) (string, bool) {
-		return "", false
-	}
-
 	for name, tt := range map[string]struct {
 		opts            map[string]any
 		envLookupper    env.LookupFunc
@@ -36,7 +32,7 @@ func TestBrowserOptionsParse(t *testing.T) { //nolint:gocognit
 			opts: map[string]any{
 				"type": "chromium",
 			},
-			envLookupper: noopEnvLookuper,
+			envLookupper: env.EmptyLookup,
 			assert: func(tb testing.TB, lo *BrowserOptions) {
 				tb.Helper()
 				assert.Equal(t, defaultOptions, lo)
@@ -46,7 +42,7 @@ func TestBrowserOptionsParse(t *testing.T) { //nolint:gocognit
 			opts: map[string]any{
 				"type": "chromium",
 			},
-			envLookupper: noopEnvLookuper,
+			envLookupper: env.EmptyLookup,
 			assert: func(tb testing.TB, lo *BrowserOptions) {
 				tb.Helper()
 				assert.Equal(t, defaultOptions, lo)
@@ -113,12 +109,7 @@ func TestBrowserOptionsParse(t *testing.T) { //nolint:gocognit
 			opts: map[string]any{
 				"type": "chromium",
 			},
-			envLookupper: func(k string) (string, bool) {
-				if k == env.BrowserArguments {
-					return "browser-arg1='value1,browser-arg2=value2,browser-flag", true
-				}
-				return "", false
-			},
+			envLookupper: env.ConstLookup(env.BrowserArguments, "browser-arg1='value1,browser-arg2=value2,browser-flag"),
 			assert: func(tb testing.TB, lo *BrowserOptions) {
 				tb.Helper()
 				require.Len(tb, lo.Args, 3)
@@ -131,12 +122,7 @@ func TestBrowserOptionsParse(t *testing.T) { //nolint:gocognit
 			opts: map[string]any{
 				"type": "chromium",
 			},
-			envLookupper: func(k string) (string, bool) {
-				if k == env.BrowserEnableDebugging {
-					return "true", true
-				}
-				return "", false
-			},
+			envLookupper: env.ConstLookup(env.BrowserEnableDebugging, "true"),
 			assert: func(tb testing.TB, lo *BrowserOptions) {
 				tb.Helper()
 				assert.True(t, lo.Debug)
@@ -146,24 +132,14 @@ func TestBrowserOptionsParse(t *testing.T) { //nolint:gocognit
 			opts: map[string]any{
 				"type": "chromium",
 			},
-			envLookupper: func(k string) (string, bool) {
-				if k == env.BrowserEnableDebugging {
-					return "non-boolean", true
-				}
-				return "", false
-			},
-			err: "K6_BROWSER_DEBUG should be a boolean",
+			envLookupper: env.ConstLookup(env.BrowserEnableDebugging, "non-boolean"),
+			err:          "K6_BROWSER_DEBUG should be a boolean",
 		},
 		"executablePath": {
 			opts: map[string]any{
 				"type": "chromium",
 			},
-			envLookupper: func(k string) (string, bool) {
-				if k == env.BrowserExecutablePath {
-					return "cmd/somewhere", true
-				}
-				return "", false
-			},
+			envLookupper: env.ConstLookup(env.BrowserExecutablePath, "cmd/somewhere"),
 			assert: func(tb testing.TB, lo *BrowserOptions) {
 				tb.Helper()
 				assert.Equal(t, "cmd/somewhere", lo.ExecutablePath)
@@ -173,12 +149,7 @@ func TestBrowserOptionsParse(t *testing.T) { //nolint:gocognit
 			opts: map[string]any{
 				"type": "chromium",
 			},
-			envLookupper: func(k string) (string, bool) {
-				if k == env.BrowserHeadless {
-					return "false", true
-				}
-				return "", false
-			},
+			envLookupper: env.ConstLookup(env.BrowserHeadless, "false"),
 			assert: func(tb testing.TB, lo *BrowserOptions) {
 				tb.Helper()
 				assert.False(t, lo.Headless)
@@ -188,24 +159,14 @@ func TestBrowserOptionsParse(t *testing.T) { //nolint:gocognit
 			opts: map[string]any{
 				"type": "chromium",
 			},
-			envLookupper: func(k string) (string, bool) {
-				if k == env.BrowserHeadless {
-					return "non-boolean", true
-				}
-				return "", false
-			},
-			err: "K6_BROWSER_HEADLESS should be a boolean",
+			envLookupper: env.ConstLookup(env.BrowserHeadless, "non-boolean"),
+			err:          "K6_BROWSER_HEADLESS should be a boolean",
 		},
 		"ignoreDefaultArgs": {
 			opts: map[string]any{
 				"type": "chromium",
 			},
-			envLookupper: func(k string) (string, bool) {
-				if k == env.BrowserIgnoreDefaultArgs {
-					return "--hide-scrollbars,--hide-something", true
-				}
-				return "", false
-			},
+			envLookupper: env.ConstLookup(env.BrowserIgnoreDefaultArgs, "--hide-scrollbars,--hide-something"),
 			assert: func(tb testing.TB, lo *BrowserOptions) {
 				tb.Helper()
 				assert.Len(t, lo.IgnoreDefaultArgs, 2)
@@ -217,12 +178,7 @@ func TestBrowserOptionsParse(t *testing.T) { //nolint:gocognit
 			opts: map[string]any{
 				"type": "chromium",
 			},
-			envLookupper: func(k string) (string, bool) {
-				if k == env.LogCategoryFilter {
-					return "**", true
-				}
-				return "", false
-			},
+			envLookupper: env.ConstLookup(env.LogCategoryFilter, "**"),
 			assert: func(tb testing.TB, lo *BrowserOptions) {
 				tb.Helper()
 				assert.Equal(t, "**", lo.LogCategoryFilter)
@@ -232,12 +188,7 @@ func TestBrowserOptionsParse(t *testing.T) { //nolint:gocognit
 			opts: map[string]any{
 				"type": "chromium",
 			},
-			envLookupper: func(k string) (string, bool) {
-				if k == env.BrowserGlobalTimeout {
-					return "10s", true
-				}
-				return "", false
-			},
+			envLookupper: env.ConstLookup(env.BrowserGlobalTimeout, "10s"),
 			assert: func(tb testing.TB, lo *BrowserOptions) {
 				tb.Helper()
 				assert.Equal(t, 10*time.Second, lo.Timeout)
@@ -247,19 +198,14 @@ func TestBrowserOptionsParse(t *testing.T) { //nolint:gocognit
 			opts: map[string]any{
 				"type": "chromium",
 			},
-			envLookupper: func(k string) (string, bool) {
-				if k == env.BrowserGlobalTimeout {
-					return "ABC", true
-				}
-				return "", false
-			},
-			err: "K6_BROWSER_TIMEOUT should be a time duration value",
+			envLookupper: env.ConstLookup(env.BrowserGlobalTimeout, "ABC"),
+			err:          "K6_BROWSER_TIMEOUT should be a time duration value",
 		},
 		"browser_type": {
 			opts: map[string]any{
 				"type": "chromium",
 			},
-			envLookupper: noopEnvLookuper,
+			envLookupper: env.EmptyLookup,
 			assert: func(tb testing.TB, lo *BrowserOptions) {
 				tb.Helper()
 				// Noop, just expect no error
@@ -269,11 +215,11 @@ func TestBrowserOptionsParse(t *testing.T) { //nolint:gocognit
 			opts: map[string]any{
 				"type": "mybrowsertype",
 			},
-			envLookupper: noopEnvLookuper,
+			envLookupper: env.EmptyLookup,
 			err:          "unsupported browser type: mybrowsertype",
 		},
 		"browser_type_unset_err": {
-			envLookupper: noopEnvLookuper,
+			envLookupper: env.EmptyLookup,
 			err:          "browser type option must be set",
 		},
 	} {

env/env.go

@@ -1,6 +1,8 @@
 // Package env provides types to interact with environment setup.
 package env
 
+import "os"
+
 // Execution specific.
 const (
 	// InstanceScenarios is an environment variable that can be used to
@@ -62,3 +64,21 @@ const (
 
 // LookupFunc defines a function to look up a key from the environment.
 type LookupFunc func(key string) (string, bool)
+
+// EmptyLookup is a LookupFunc that always returns "" and false.
+func EmptyLookup(key string) (string, bool) { return "", false }
+
+// Lookup is a LookupFunc that uses os.LookupEnv.
+func Lookup(key string) (string, bool) { return os.LookupEnv(key) }
+
+// ConstLookup is a LookupFunc that always returns the given value and true
+// if the key matches the given key. Otherwise it returns EmptyLookup
+// behaviour. Useful for testing.
+func ConstLookup(k, v string) LookupFunc {
+	return func(key string) (string, bool) {
+		if key == k {
+			return v, true
+		}
+		return EmptyLookup(key)
+	}
+}

k6ext/k6test/vu.go

@@ -66,9 +66,7 @@ func NewVU(tb testing.TB, opts ...any) *VU {
 
 	var (
 		samples    = make(chan k6metrics.SampleContainer, 1000)
-		lookupFunc = func(_ string) (string, bool) {
-			return "", false
-		}
+		lookupFunc = env.EmptyLookup
 	)
 	for _, opt := range opts {
 		switch opt := opt.(type) {

tests/browser_test.go

@@ -43,15 +43,9 @@ func TestBrowserNewPage(t *testing.T) {
 func TestTmpDirCleanup(t *testing.T) {
 	t.Parallel()
 
-	tmpDirPath := "./"
-
-	b := newTestBrowser(t, withSkipClose(), withLookupFunc(func(k string) (string, bool) {
-		if k == "TMPDIR" {
-			return tmpDirPath, true
-		}
-		return "", false
-	}))
+	const tmpDirPath = "./"
 
+	b := newTestBrowser(t, withSkipClose(), withLookupFunc(env.ConstLookup("TMPDIR", tmpDirPath)))
 	p := b.NewPage(nil)
 	err := p.Close(nil)
 	require.NoError(t, err)
@@ -146,12 +140,9 @@ func TestBrowserUserAgent(t *testing.T) {
 
 func TestBrowserCrashErr(t *testing.T) {
 	// create a new VU in an environment that requires a bad remote-debugging-port.
-	vu := k6test.NewVU(t, k6test.WithLookupFunc(func(key string) (string, bool) {
-		if key == env.BrowserArguments {
-			return "remote-debugging-port=99999", true
-		}
-		return "", false
-	}))
+	vu := k6test.NewVU(t, k6test.WithLookupFunc(
+		env.ConstLookup(env.BrowserArguments, "remote-debugging-port=99999"),
+	))
 
 	mod := browser.New().NewModuleInstance(vu)
 	jsMod, ok := mod.Exports().Default.(*browser.JSModule)

tests/browser_type_test.go

@@ -32,12 +32,9 @@ func TestBrowserTypeLaunchToConnect(t *testing.T) {
 
 	// Export WS URL env var
 	// pointing to test browser proxy
-	vu := k6test.NewVU(t, k6test.WithLookupFunc(func(key string) (string, bool) {
-		if key == env.WebSocketURLs {
-			return bp.wsURL(), true
-		}
-		return "", false
-	}))
+	vu := k6test.NewVU(t, k6test.WithLookupFunc(
+		env.ConstLookup(env.WebSocketURLs, bp.wsURL())),
+	)
 
 	// We have to call launch method through JS API in Goja
 	// to take mapping layer into account, instead of calling

tests/frame_test.go

@@ -1,7 +1,6 @@
 package tests
 
 import (
-	"os"
 	"strconv"
 	"testing"
 
@@ -70,7 +69,7 @@ func TestFrameDismissDialogBox(t *testing.T) {
 func TestFrameNoPanicWithEmbeddedIFrame(t *testing.T) {
 	t.Parallel()
 
-	if s, ok := os.LookupEnv(env.BrowserHeadless); ok {
+	if s, ok := env.Lookup(env.BrowserHeadless); ok {
 		if v, err := strconv.ParseBool(s); err == nil && v {
 			// We're skipping this when running in headless
 			// environments since the bug that the test fixes
@@ -82,12 +81,9 @@ func TestFrameNoPanicWithEmbeddedIFrame(t *testing.T) {
 	}
 
 	// run the browser in headfull mode.
-	tb := newTestBrowser(t, withFileServer(), withLookupFunc(func(key string) (string, bool) {
-		if key == env.BrowserHeadless {
-			return "0", true
-		}
-		return "", false
-	}))
+	tb := newTestBrowser(t, withFileServer(), withLookupFunc(
+		env.ConstLookup(env.BrowserHeadless, "0"),
+	))
 
 	p := tb.NewPage(nil)
 	_, err := p.Goto(

tests/test_browser.go

@@ -11,6 +11,7 @@ import (
 	"github.com/grafana/xk6-browser/api"
 	"github.com/grafana/xk6-browser/chromium"
 	"github.com/grafana/xk6-browser/common"
+	"github.com/grafana/xk6-browser/env"
 	"github.com/grafana/xk6-browser/k6ext"
 	"github.com/grafana/xk6-browser/k6ext/k6test"
 
@@ -57,9 +58,9 @@ func newTestBrowser(tb testing.TB, opts ...any) *testBrowser {
 		enableLogCache     = false
 		skipClose          = false
 		samples            = make(chan k6metrics.SampleContainer, 1000)
-		// default lookup function is os.LookupEnv so that we can
+		// default lookup function is env.Lookup so that we can
 		// pass the environment variables while testing, i.e.: K6_BROWSER_LOG.
-		lookupFunc = os.LookupEnv
+		lookupFunc = env.Lookup
 	)
 	for _, opt := range opts {
 		switch opt := opt.(type) {
@@ -82,7 +83,7 @@ func newTestBrowser(tb testing.TB, opts ...any) *testBrowser {
 				}
 				// return from the real environment lookup function
 				// so that we can debug (or other things) when we want it.
-				return os.LookupEnv(key)
+				return env.Lookup(key)
 			}
 		}
 	}

tests/test_browser_test.go

@@ -37,15 +37,9 @@ func (t *testingT) Fatalf(format string, args ...any) {
 
 func TestTestBrowserWithLookupFunc(t *testing.T) {
 	tt := &testingT{TB: t}
-
 	// this lookup is expected to fail because the remote debugging port is
 	// invalid, practically testing that the InitEnv.LookupEnv is used.
-	lookup := func(key string) (string, bool) {
-		if key == env.BrowserArguments {
-			return "remote-debugging-port=99999", true
-		}
-		return "", false
-	}
+	lookup := env.ConstLookup(env.BrowserArguments, "remote-debugging-port=99999")
 	_ = newTestBrowser(tt, withLookupFunc(lookup))
 	require.True(t, tt.fatalfCalled)
 }