From 29cf58538a0c2aef4901781471845ebab56d0570 Mon Sep 17 00:00:00 2001 From: Vlad Burlutskiy Date: Fri, 20 Dec 2024 17:18:32 -0500 Subject: [PATCH 1/2] Replace os.Setenv with t.Setenv Signed-off-by: Vlad Burlutskiy --- cmd/synthetic_checks_test.go | 20 ++++++-------------- pkg/config/config_test.go | 4 +--- pkg/config/env_vars_test.go | 16 +++++----------- pkg/metrics/metrics_test.go | 4 +--- 4 files changed, 13 insertions(+), 31 deletions(-) diff --git a/cmd/synthetic_checks_test.go b/cmd/synthetic_checks_test.go index 2b71ea1..4f9b509 100644 --- a/cmd/synthetic_checks_test.go +++ b/cmd/synthetic_checks_test.go @@ -148,8 +148,7 @@ func TestRunSyntheticTests(t *testing.T) { defer mockPromAggregationGateway.Close() // Set the PROM_AGGREGATION_GATEWAY_URL to the mock server's URL - os.Setenv(c.EnvPromAggregationGatewayUrl, mockPromAggregationGateway.URL) - defer os.Unsetenv(c.EnvPromAggregationGatewayUrl) + t.Setenv(c.EnvPromAggregationGatewayUrl, mockPromAggregationGateway.URL) // Convert MockApplications to real ones var appData []*a.Application @@ -201,12 +200,12 @@ func TestDoCheck(t *testing.T) { t.Run(tt.name, func(t *testing.T) { // Set up environment variables and command line arguments - os.Setenv(c.EnvYamlPath, tt.envYamlPath) - os.Setenv(c.EnvSlackWebhookUrl, tt.envSlackUrl) - os.Setenv(c.EnvClusterInfo, tt.envClusterInfo) - os.Setenv(c.EnvPromAggregationGatewayUrl, mockPushgateway.URL) + t.Setenv(c.EnvYamlPath, tt.envYamlPath) + t.Setenv(c.EnvSlackWebhookUrl, tt.envSlackUrl) + t.Setenv(c.EnvClusterInfo, tt.envClusterInfo) + t.Setenv(c.EnvPromAggregationGatewayUrl, mockPushgateway.URL) // Set environment variable to true for this test - os.Setenv(c.EnvSkipWhitelistCheck, "true") + t.Setenv(c.EnvSkipWhitelistCheck, "true") os.Args = tt.cmdArgs // Call function under test @@ -218,13 +217,6 @@ func TestDoCheck(t *testing.T) { } else { assert.NoError(t, err, "Expected no error but got one") } - - // Unset environment variables - os.Unsetenv(c.EnvYamlPath) - os.Unsetenv(c.EnvSlackWebhookUrl) - os.Unsetenv(c.EnvClusterInfo) - os.Unsetenv(c.EnvPromAggregationGatewayUrl) - os.Unsetenv(c.EnvSkipWhitelistCheck) }) } } diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index f738a24..40336e4 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -4,7 +4,6 @@ import ( a "github.com/NYULibraries/aswa/pkg/application" "github.com/stretchr/testify/assert" "net/http" - "os" "testing" "time" ) @@ -39,10 +38,9 @@ func TestNewConfig(t *testing.T) { func testNewConfigFunc(path string, expectedErr string) func(*testing.T) { return func(t *testing.T) { // Set environment variable to true for this test - os.Setenv(EnvSkipWhitelistCheck, "true") + t.Setenv(EnvSkipWhitelistCheck, "true") _, err := NewConfig(path) - os.Unsetenv(EnvSkipWhitelistCheck) if expectedErr == "" { assert.Nil(t, err) } else { diff --git a/pkg/config/env_vars_test.go b/pkg/config/env_vars_test.go index 26e6249..0ba5f87 100644 --- a/pkg/config/env_vars_test.go +++ b/pkg/config/env_vars_test.go @@ -37,13 +37,12 @@ func TestGetYamlPath(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - os.Setenv(EnvYamlPath, tt.envYamlPath) + t.Setenv(EnvYamlPath, tt.envYamlPath) got := GetYamlPath() assert.Equal(t, tt.want, got, "getYamlPath() should return correct yaml path") - os.Unsetenv(EnvYamlPath) }) } } @@ -83,13 +82,12 @@ func TestGetSlackWebhookUrl(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - os.Setenv(EnvSlackWebhookUrl, tt.envSlackWebhookUrl) + t.Setenv(EnvSlackWebhookUrl, tt.envSlackWebhookUrl) gotLogMessage := CaptureOutput(GetSlackWebhookUrl) assert.Equal(t, tt.wantLogmessage, gotLogMessage, "getSlackWebhookUrl() should return correct Slack webhook URL") - os.Unsetenv(EnvSlackWebhookUrl) }) } } @@ -108,7 +106,7 @@ func TestGetClusterInfo(t *testing.T) { t.Run(tt.name, func(t *testing.T) { // Set up environment variable - os.Setenv(EnvClusterInfo, tt.envClusterInfo) + t.Setenv(EnvClusterInfo, tt.envClusterInfo) // Call function under test gotLogMessage := CaptureOutput(GetClusterInfo) @@ -116,8 +114,6 @@ func TestGetClusterInfo(t *testing.T) { // Assert that the function returns the expected result assert.Equal(t, tt.wantLogMessage, gotLogMessage, "getClusterInfo() should return correct cluster info") - // Unset environment variable for next test - os.Unsetenv(EnvClusterInfo) }) } } @@ -135,13 +131,12 @@ func TestGetPromAggregationGatewayUrl(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - os.Setenv(EnvPromAggregationGatewayUrl, tt.envPromAggregationGatewayUrl) + t.Setenv(EnvPromAggregationGatewayUrl, tt.envPromAggregationGatewayUrl) got := GetPromAggregationgatewayUrl() assert.Equal(t, tt.want, got, "getPushgatewayUrl() should return correct pushgateway URL") - os.Unsetenv(EnvPromAggregationGatewayUrl) }) } @@ -160,13 +155,12 @@ func TestGetEnvironmentName(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - os.Setenv(EnvName, tt.envName) + t.Setenv(EnvName, tt.envName) got := GetEnvironmentName() assert.Equal(t, tt.want, got, "GetEnvironmentName() should return correct environment name") - os.Unsetenv(EnvName) }) } } diff --git a/pkg/metrics/metrics_test.go b/pkg/metrics/metrics_test.go index dd3a00b..18a19a8 100644 --- a/pkg/metrics/metrics_test.go +++ b/pkg/metrics/metrics_test.go @@ -4,7 +4,6 @@ import ( c "github.com/NYULibraries/aswa/pkg/config" "net/http" "net/http/httptest" - "os" "testing" ) @@ -37,8 +36,7 @@ func TestPushMetrics(t *testing.T) { defer server.Close() // Setup environment variable to mock the pushgateway URL - os.Setenv(c.EnvPromAggregationGatewayUrl, server.URL) - defer os.Unsetenv(c.EnvPromAggregationGatewayUrl) + t.Setenv(c.EnvPromAggregationGatewayUrl, server.URL) // Increment a test counter to simulate metrics that would be pushed IncrementFailedTestsCounter("testApp") From 5c825707fdb9f2fd57036cff28a4a600ee17c8e0 Mon Sep 17 00:00:00 2001 From: Vlad Burlutskiy Date: Mon, 6 Jan 2025 16:01:07 -0500 Subject: [PATCH 2/2] Ensure os.Args is restored after modification to prevent test pollution Signed-off-by: Vlad Burlutskiy --- cmd/synthetic_checks_test.go | 2 ++ pkg/config/env_vars_test.go | 2 ++ 2 files changed, 4 insertions(+) diff --git a/cmd/synthetic_checks_test.go b/cmd/synthetic_checks_test.go index 4f9b509..4ba784f 100644 --- a/cmd/synthetic_checks_test.go +++ b/cmd/synthetic_checks_test.go @@ -206,6 +206,8 @@ func TestDoCheck(t *testing.T) { t.Setenv(c.EnvPromAggregationGatewayUrl, mockPushgateway.URL) // Set environment variable to true for this test t.Setenv(c.EnvSkipWhitelistCheck, "true") + originalArgs := os.Args + defer func() { os.Args = originalArgs }() os.Args = tt.cmdArgs // Call function under test diff --git a/pkg/config/env_vars_test.go b/pkg/config/env_vars_test.go index 0ba5f87..dc078f2 100644 --- a/pkg/config/env_vars_test.go +++ b/pkg/config/env_vars_test.go @@ -60,6 +60,8 @@ func TestGetCmdArg(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + originalArgs := os.Args + defer func() { os.Args = originalArgs }() os.Args = tt.osArgs got := GetCmdArg()