Skip to content

Commit

Permalink
test: os.Unsetenv affects global state
Browse files Browse the repository at this point in the history
This change adds a helper function that gaurantees the cleanup of
environment variables.

For each applicable test function, we run this at the start, and at the
end through a `defer` keyword. Previously, we were unsetting the
environment variable within the test, though there is a risk that since
global state is being manipulated, we may run into a flakey test where
an environment variable is inadvertently lingering.
  • Loading branch information
therealvio committed Dec 5, 2024
1 parent 95f23d2 commit b826ce5
Showing 1 changed file with 12 additions and 1 deletion.
13 changes: 12 additions & 1 deletion src/plugin/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@ import (
)

func TestFailOnMissingEnvironment(t *testing.T) {
unsetEnvironmentVariables()
defer unsetEnvironmentVariables()

var config plugin.Config
fetcher := plugin.EnvironmentConfigFetcher{}

t.Setenv("BUILDKITE_PLUGIN_EXAMPLE_GO_MESSAGE", "")
os.Unsetenv("BUILDKITE_PLUGIN_EXAMPLE_GO_MESSAGE")

err := fetcher.Fetch(&config)

Expand All @@ -23,6 +25,9 @@ func TestFailOnMissingEnvironment(t *testing.T) {
}

func TestFetchConfigFromEnvironment(t *testing.T) {
unsetEnvironmentVariables()
defer unsetEnvironmentVariables()

var config plugin.Config
fetcher := plugin.EnvironmentConfigFetcher{}

Expand All @@ -33,3 +38,9 @@ func TestFetchConfigFromEnvironment(t *testing.T) {
require.NoError(t, err, "fetch should not error")
assert.Equal(t, "test-message", config.Message, "fetched message should match environment")
}

// Unsets environment variables through an all-in-one function. Extend this with additional environment variables as
// needed.
func unsetEnvironmentVariables() {
os.Unsetenv("BUILDKITE_PLUGIN_EXAMPLE_GO_MESSAGE")
}

0 comments on commit b826ce5

Please sign in to comment.