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 guarantees 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 3fee711
Showing 1 changed file with 11 additions and 3 deletions.
14 changes: 11 additions & 3 deletions src/plugin/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,21 @@ import (
)

func TestFailOnMissingEnvironment(t *testing.T) {
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)

expectedErr := "required key BUILDKITE_PLUGIN_EXAMPLE_GO_MESSAGE missing value"
assert.EqualError(t, err, expectedErr, "fetch should error on missing environment variable")
}

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

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

Expand All @@ -33,3 +35,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 3fee711

Please sign in to comment.