From c450e1c13d6b7d4d68c11527cf52a76ca4c5c703 Mon Sep 17 00:00:00 2001 From: sneak Date: Fri, 20 Jun 2025 09:22:01 -0700 Subject: [PATCH] fix: replace remaining os.Setenv with t.Setenv in tests Replace all os.Setenv calls with t.Setenv in test functions to ensure proper test environment cleanup and better test isolation. This leaves only legitimate application code and helper functions using os.Setenv. --- internal/cli/cli_test.go | 12 +------- internal/secret/passphrase_test.go | 14 ++------- internal/secret/pgpunlock_test.go | 47 +++--------------------------- internal/secret/secret_test.go | 16 ++-------- 4 files changed, 9 insertions(+), 80 deletions(-) diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index 239585b..4d95adc 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -37,19 +37,9 @@ func TestCLIInstanceWithFs(t *testing.T) { func TestDetermineStateDir(t *testing.T) { // Test the determineStateDir function from the secret package - // Save original environment and restore it after test - originalStateDir := os.Getenv(secret.EnvStateDir) - defer func() { - if originalStateDir == "" { - os.Unsetenv(secret.EnvStateDir) - } else { - os.Setenv(secret.EnvStateDir, originalStateDir) - } - }() - // Test with environment variable set testEnvDir := "/test-env-dir" - os.Setenv(secret.EnvStateDir, testEnvDir) + t.Setenv(secret.EnvStateDir, testEnvDir) stateDir := secret.DetermineStateDir("") if stateDir != testEnvDir { diff --git a/internal/secret/passphrase_test.go b/internal/secret/passphrase_test.go index bf2b82f..a756d2a 100644 --- a/internal/secret/passphrase_test.go +++ b/internal/secret/passphrase_test.go @@ -131,18 +131,8 @@ func TestPassphraseUnlockerWithRealFS(t *testing.T) { } }) - // Save original environment variables and set test ones - oldPassphrase := os.Getenv(secret.EnvUnlockPassphrase) - os.Setenv(secret.EnvUnlockPassphrase, testPassphrase) - - // Clean up after test - defer func() { - if oldPassphrase != "" { - os.Setenv(secret.EnvUnlockPassphrase, oldPassphrase) - } else { - os.Unsetenv(secret.EnvUnlockPassphrase) - } - }() + // Set test environment variable (cleaned up automatically) + t.Setenv(secret.EnvUnlockPassphrase, testPassphrase) // Test getting identity from environment variable t.Run("GetIdentityFromEnv", func(t *testing.T) { diff --git a/internal/secret/pgpunlock_test.go b/internal/secret/pgpunlock_test.go index e8a0c22..b8744ee 100644 --- a/internal/secret/pgpunlock_test.go +++ b/internal/secret/pgpunlock_test.go @@ -143,20 +143,8 @@ func TestPGPUnlockerWithRealFS(t *testing.T) { t.Fatalf("Failed to create GNUPGHOME: %v", err) } - // Save original GNUPGHOME - origGnupgHome := os.Getenv("GNUPGHOME") - // Set new GNUPGHOME - os.Setenv("GNUPGHOME", gnupgHomeDir) - - // Clean up environment after test - defer func() { - if origGnupgHome != "" { - os.Setenv("GNUPGHOME", origGnupgHome) - } else { - os.Unsetenv("GNUPGHOME") - } - }() + t.Setenv("GNUPGHOME", gnupgHomeDir) // Test passphrase for GPG key testPassphrase := "test123" @@ -224,15 +212,7 @@ Passphrase: ` + testPassphrase + ` t.Logf("Generated GPG fingerprint: %s", fingerprint) // Set the GPG_AGENT_INFO to empty to ensure gpg-agent doesn't interfere - oldAgentInfo := os.Getenv("GPG_AGENT_INFO") - os.Setenv("GPG_AGENT_INFO", "") - defer func() { - if oldAgentInfo != "" { - os.Setenv("GPG_AGENT_INFO", oldAgentInfo) - } else { - os.Unsetenv("GPG_AGENT_INFO") - } - }() + t.Setenv("GPG_AGENT_INFO", "") // Use the real filesystem fs := afero.NewOsFs() @@ -240,28 +220,9 @@ Passphrase: ` + testPassphrase + ` // Test data testMnemonic := "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about" - // Save original environment variable - oldMnemonic := os.Getenv(secret.EnvMnemonic) - oldGPGKeyID := os.Getenv(secret.EnvGPGKeyID) - // Set test environment variables - os.Setenv(secret.EnvMnemonic, testMnemonic) - os.Setenv(secret.EnvGPGKeyID, keyID) - - // Clean up after test - defer func() { - if oldMnemonic != "" { - os.Setenv(secret.EnvMnemonic, oldMnemonic) - } else { - os.Unsetenv(secret.EnvMnemonic) - } - - if oldGPGKeyID != "" { - os.Setenv(secret.EnvGPGKeyID, oldGPGKeyID) - } else { - os.Unsetenv(secret.EnvGPGKeyID) - } - }() + t.Setenv(secret.EnvMnemonic, testMnemonic) + t.Setenv(secret.EnvGPGKeyID, keyID) // Set up vault structure for testing stateDir := tempDir diff --git a/internal/secret/secret_test.go b/internal/secret/secret_test.go index 267dea7..7746cb3 100644 --- a/internal/secret/secret_test.go +++ b/internal/secret/secret_test.go @@ -128,19 +128,9 @@ func TestPerSecretKeyFunctionality(t *testing.T) { // Create an in-memory filesystem for testing fs := afero.NewMemMapFs() - // Set up test environment variables - oldMnemonic := os.Getenv(EnvMnemonic) - defer func() { - if oldMnemonic == "" { - os.Unsetenv(EnvMnemonic) - } else { - os.Setenv(EnvMnemonic, oldMnemonic) - } - }() - // Set test mnemonic for direct encryption/decryption testMnemonic := "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about" - os.Setenv(EnvMnemonic, testMnemonic) + t.Setenv(EnvMnemonic, testMnemonic) // Set up a test vault structure baseDir := "/test-config/berlin.sneak.pkg.secret" @@ -312,9 +302,7 @@ func TestSecretGetValueWithEnvMnemonicUsesVaultDerivationIndex(t *testing.T) { // Set up test mnemonic testMnemonic := "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about" - originalEnv := os.Getenv(EnvMnemonic) - os.Setenv(EnvMnemonic, testMnemonic) - defer os.Setenv(EnvMnemonic, originalEnv) + t.Setenv(EnvMnemonic, testMnemonic) // Create temporary directory for vaults fs := afero.NewOsFs()