From b826ce59954b5fc9fcdcfa36289154eb1f6ef8dd Mon Sep 17 00:00:00 2001 From: Sam Craske Date: Thu, 5 Dec 2024 15:53:33 +1100 Subject: [PATCH] test: os.Unsetenv affects global state 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. --- src/plugin/config_test.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/plugin/config_test.go b/src/plugin/config_test.go index 7ea1f27..6925c63 100644 --- a/src/plugin/config_test.go +++ b/src/plugin/config_test.go @@ -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) @@ -23,6 +25,9 @@ func TestFailOnMissingEnvironment(t *testing.T) { } func TestFetchConfigFromEnvironment(t *testing.T) { + unsetEnvironmentVariables() + defer unsetEnvironmentVariables() + var config plugin.Config fetcher := plugin.EnvironmentConfigFetcher{} @@ -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") +}