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.

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

Refactor to k6 lookupEnv and abstract env. var. lookup #918

Merged
merged 28 commits into from Jun 6, 2023

Conversation

inancgumus
Copy link
Member

@inancgumus inancgumus commented Jun 1, 2023

This PR refactors the direct usage of os.LookupEnv to k6 LookupEnv. Please have a look at the commit descriptions for the changes needed.

These new changes provide enhanced testability and consistency throughout the module.

The following are all the environment variables and options that the k6-browser module supports. The outdated XK6_* are removed and aligned with the new browser options we have.

// Execution specific.
const (
	// InstanceScenarios is an environment variable that can be used to
	// define the extra scenarios details to use when running remotely.
	InstanceScenarios = "K6_INSTANCE_SCENARIOS"

	// WebSocketURLs is an environment variable that can be used to
	// define the WS URLs to connect to when running remotely.
	WebSocketURLs = "K6_BROWSER_WS_URL"

	// BrowserArguments is an environment variable that can be used to
	// pass extra arguments to the browser process.
	BrowserArguments = "K6_BROWSER_ARGS"

	// BrowserExecutablePath is an environment variable that can be used
	// to define the path to the browser to execute.
	BrowserExecutablePath = "K6_BROWSER_EXECUTABLE_PATH"

	// BrowserEnableDebugging is an environment variable that can be used to
	// define if the browser should be launched with debugging enabled.
	BrowserEnableDebugging = "K6_BROWSER_DEBUG"

	// BrowserHeadless is an environment variable that can be used to
	// define if the browser should be launched in headless mode.
	BrowserHeadless = "K6_BROWSER_HEADLESS"

	// BrowserIgnoreDefaultArgs is an environment variable that can be
	// used to define if the browser should ignore default arguments.
	BrowserIgnoreDefaultArgs = "K6_BROWSER_IGNORE_DEFAULT_ARGS"

	// BrowserGlobalTimeout is an environment variable that can be used
	// to set the global timeout for the browser.
	BrowserGlobalTimeout = "K6_BROWSER_TIMEOUT"
)

// Logging and debugging.
const (
	// EnableProfiling is an environment variable that can be used to
	// enable profiling for the browser. It will start up a debugging
	// server on ProfilingServerAddr.
	EnableProfiling = "K6_BROWSER_ENABLE_PPROF"

	// ProfilingServerAddr is the address of the profiling server.
	ProfilingServerAddr = "localhost:6060"

	// LogCaller is an environment variable that can be used to enable
	// the caller function information in the browser logs.
	LogCaller = "K6_BROWSER_LOG_CALLER"

	// LogLevel is an environment variable that can be used to set the
	// log level for the browser logs.
	LogLevel = "K6_BROWSER_LOG"

	// LogCategoryFilter is an environment variable that can be used to
	// filter the browser logs based on their category. It supports
	// regular expressions.
	LogCategoryFilter = "K6_BROWSER_LOG_CATEGORY_FILTER"
)

@inancgumus inancgumus force-pushed the refactor/822-k6-lookupenv branch 4 times, most recently from 0c67592 to 36943a9 Compare June 1, 2023 14:55
@inancgumus inancgumus changed the title Refactor/822 k6 lookupenv Refactor to k6 lookupEnv and abstract env. var. lookup Jun 1, 2023
@inancgumus inancgumus linked an issue Jun 1, 2023 that may be closed by this pull request
@inancgumus inancgumus self-assigned this Jun 1, 2023
@inancgumus inancgumus force-pushed the refactor/822-k6-lookupenv branch 3 times, most recently from 3fd6c8d to 59f6bef Compare June 2, 2023 11:28
This will be refactored in later commits to use k6.LookupEnv.
This will stop the whole test run and it's important because with a
panic only a single test will panic. However, this is a globally needed
functionality and the whole test run should abort.
This helps to get default behavior in tests and prevent nil pointer
errors.
Without this tests that depend on LookupEnv will fail with a nil pointer
error. Tests that want to set this can do so.
@inancgumus inancgumus marked this pull request as ready for review June 2, 2023 11:51
@inancgumus inancgumus requested review from ka3de and ankur22 June 2, 2023 11:51
The test needed a fix too as we're getting env. vars. from VU now.
Since we will use InitEnv().LookupEnv, add support for that in the test
browser. Any test want to test with an environment variable can use
withLookupFunc to set an environment variable.
The previous commit unlocked using this now.
Switch to withLookupFunc in testBrowser. We're now setting environment
variables via InitEnv(). So we no longer need this method.
Tests can provide explicit values if necessary by providing a lookupfunc
to the testBrowser.
This test wasn't running since we switched to environment variable
browser options. These changes need to be made along with other browser
options as you can see in the change because the commit would fail
otherwise.
Copy link
Collaborator

@ankur22 ankur22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🙂

tests/test_browser.go Outdated Show resolved Hide resolved
chromium/browser_type.go Show resolved Hide resolved
Copy link
Collaborator

@ka3de ka3de left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @inancgumus ! 👏 Made a couple of comments, but overall LGTM.

tests/test_browser.go Outdated Show resolved Hide resolved
env/env.go Show resolved Hide resolved
k6ext/k6test/vu.go Outdated Show resolved Hide resolved
We can revert this commit if this is needed.

Resolves: #918 (comment)
@inancgumus inancgumus merged commit 2d4ba2f into main Jun 6, 2023
13 checks passed
@inancgumus inancgumus deleted the refactor/822-k6-lookupenv branch June 6, 2023 13:13
@inancgumus inancgumus added the productivity Issues and PRs that improve our productivity label Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
productivity Issues and PRs that improve our productivity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use k6 LookupEnv instead of os.GetEnv and os.LookupEnv
3 participants