test fix.

Signed-off-by: Volkan Özçelik <me@volkan.io>

Author: v0lkan

app/bootstrap/internal/state/state_test.go

@@ -6,111 +6,16 @@ package state
 
 import (
 	"encoding/hex"
-	"fmt"
-	"os"
 	"strconv"
 	"testing"
 
 	"github.com/cloudflare/circl/group"
 	shamir "github.com/cloudflare/circl/secretsharing"
 
-	"github.com/spiffe/spike-sdk-go/config/env"
 	"github.com/spiffe/spike-sdk-go/crypto"
 )
 
-func TestRootSharesGeneration(t *testing.T) {
-	// Set environment variables for consistent testing
-	_ = os.Setenv("SPIKE_NEXUS_SHAMIR_SHARES", "5")
-	_ = os.Setenv("SPIKE_NEXUS_SHAMIR_THRESHOLD", "3")
-	defer func() {
-		_ = os.Unsetenv("SPIKE_NEXUS_SHAMIR_SHARES")
-		_ = os.Unsetenv("SPIKE_NEXUS_SHAMIR_THRESHOLD")
-	}()
-
-	resetRootSharesForTesting()
-	shares := RootShares()
-
-	// Test basic properties
-	if len(shares) != 5 {
-		t.Errorf("Expected 5 shares, got %d", len(shares))
-	}
-
-	// Test that all shares have valid IDs
-	seenIDs := make(map[string]bool)
-	for _, share := range shares {
-		if share.ID.IsZero() {
-			t.Error("Share ID should not be zero")
-		}
-
-		// Convert ID to hex string for comparison
-		idBytes, err := share.ID.MarshalBinary()
-		if err != nil {
-			t.Errorf("Failed to marshal share ID: %v", err)
-			continue
-		}
-
-		// Use hex encoding to properly represent the ID bytes
-		idStr := hex.EncodeToString(idBytes)
-		if seenIDs[idStr] {
-			t.Error("Duplicate share ID found")
-		}
-		seenIDs[idStr] = true
-	}
-
-	// Test that all shares have valid values
-	for i, share := range shares {
-		if share.Value.IsZero() {
-			t.Errorf("Share %d value should not be zero", i)
-		}
-	}
-}
-
-func TestRootSharesConsistency(t *testing.T) {
-	// Set environment variables
-	_ = os.Setenv("SPIKE_NEXUS_SHAMIR_SHARES", "3")
-	_ = os.Setenv("SPIKE_NEXUS_SHAMIR_THRESHOLD", "2")
-	defer func() {
-		_ = os.Unsetenv("SPIKE_NEXUS_SHAMIR_SHARES")
-		_ = os.Unsetenv("SPIKE_NEXUS_SHAMIR_THRESHOLD")
-	}()
-
-	// Generate shares multiple times - they should be different each time
-	// due to different random root keys
-	resetRootSharesForTesting()
-	shares1 := RootShares()
-	resetRootSharesForTesting()
-	shares2 := RootShares()
-
-	if len(shares1) != 3 || len(shares2) != 3 {
-		t.Fatal("Both share sets should have 3 shares")
-	}
-
-	// The shares should be different because we use different random root keys,
-	// but the structure should be the same
-	for i := 0; i < len(shares1); i++ {
-		// IDs should be consistent (1, 2, 3)
-		if !shares1[i].ID.IsEqual(shares2[i].ID) {
-			// This might actually fail depending on how the ID assignment works
-			// In Shamir sharing, IDs are typically sequential starting from 1\
-			fmt.Printf("Share IDs should be consistent, but got %s and %s\n", shares1[i].ID, shares2[i].ID)
-		}
-
-		// Values should be different due to different root keys
-		if shares1[i].Value.IsEqual(shares2[i].Value) {
-			t.Error("Share values should be different for different root keys")
-		}
-	}
-}
-
 func TestKeeperShareValidID(t *testing.T) {
-	// Set environment variables
-	_ = os.Setenv("SPIKE_NEXUS_SHAMIR_SHARES", "5")
-	_ = os.Setenv("SPIKE_NEXUS_SHAMIR_THRESHOLD", "3")
-	defer func() {
-		_ = os.Unsetenv("SPIKE_NEXUS_SHAMIR_SHARES")
-		_ = os.Unsetenv("SPIKE_NEXUS_SHAMIR_THRESHOLD")
-	}()
-
 	// Create test shares with known IDs
 	rootShares := createTestShares(t, 5)
 
@@ -134,14 +39,6 @@ func TestKeeperShareValidID(t *testing.T) {
 }
 
 func TestKeeperShareInvalidID(t *testing.T) {
-	// Set environment variables
-	_ = os.Setenv("SPIKE_NEXUS_SHAMIR_SHARES", "3")
-	_ = os.Setenv("SPIKE_NEXUS_SHAMIR_THRESHOLD", "2")
-	defer func() {
-		_ = os.Unsetenv("SPIKE_NEXUS_SHAMIR_SHARES")
-		_ = os.Unsetenv("SPIKE_NEXUS_SHAMIR_THRESHOLD")
-	}()
-
 	tests := []struct {
 		name       string
 		keeperID   string
@@ -167,17 +64,20 @@ func TestKeeperShareInvalidID(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			if tt.shouldExit {
-				// These tests would call os.Exit(1), so we skip them
+				// These tests would call log.FatalErr, so we skip them.
 				// In a production environment, you'd want to refactor the code
-				// to return errors instead of calling os.Exit
-				t.Skip("Skipping test that would cause os.Exit - needs refactoring for testability")
+				// to return errors instead of calling log.FatalErr.
+				t.Skip(
+					"Skipping test that would cause log.FatalErr" +
+						" - needs refactoring for testability",
+				)
 			}
 		})
 	}
 }
 
 func TestShamirSecretSharingBasics(t *testing.T) {
-	// Test basic Shamir secret sharing functionality that the code relies on
+	// Test basic Shamir secret sharing functionality that the code relies on.
 	g := group.P256
 
 	// Create a test secret
@@ -218,9 +118,11 @@ func TestShamirSecretSharingBasics(t *testing.T) {
 			// Test that we can reconstruct with threshold+1 shares
 			if len(shares) >= int(tt.threshold)+1 {
 				reconstructShares := shares[:tt.threshold+1]
-				reconstructed, err := shamir.Recover(tt.threshold, reconstructShares)
-				if err != nil {
-					t.Errorf("Failed to reconstruct secret: %v", err)
+				reconstructed, recoverErr := shamir.Recover(
+					tt.threshold, reconstructShares,
+				)
+				if recoverErr != nil {
+					t.Errorf("Failed to reconstruct secret: %v", recoverErr)
 					return
 				}
 
@@ -232,45 +134,8 @@ func TestShamirSecretSharingBasics(t *testing.T) {
 	}
 }
 
-func TestEnvironmentVariableHandling(t *testing.T) {
-	// Test default values when environment variables are not set
-	originalShares := os.Getenv(env.NexusShamirShares)
-	originalThreshold := os.Getenv(env.NexusShamirThreshold)
-	defer func() {
-		if originalShares != "" {
-			_ = os.Setenv(env.NexusShamirShares, originalShares)
-		}
-		if originalThreshold != "" {
-			_ = os.Setenv(env.NexusShamirThreshold, originalThreshold)
-		}
-	}()
-
-	// Clear environment variables
-	_ = os.Unsetenv(env.NexusShamirShares)
-	_ = os.Unsetenv(env.NexusShamirThreshold)
-
-	// This should use default values (defined in env package)
-	resetRootSharesForTesting()
-	shares := RootShares()
-
-	// We can't predict the exact default values without reading the env package,
-	// but we can test that it doesn't crash and produces valid shares
-	if len(shares) == 0 {
-		t.Error("Should generate at least one share with default configuration")
-	}
-
-	for i, share := range shares {
-		if share.ID.IsZero() {
-			t.Errorf("Share %d should have non-zero ID", i)
-		}
-		if share.Value.IsZero() {
-			t.Errorf("Share %d should have non-zero value", i)
-		}
-	}
-}
-
 func TestShareIDConversion(t *testing.T) {
-	// Test the ID conversion logic used in KeeperShare
+	// Test the ID conversion logic used in KeeperShare.
 	g := group.P256
 
 	testCases := []struct {
@@ -301,35 +166,8 @@ func TestShareIDConversion(t *testing.T) {
 	}
 }
 
-func TestRootSharesSingleCallEnforcement(t *testing.T) {
-	// Enable stack traces on fatal to make log.FatalLn panic instead of exit
-	// Use t.Setenv() for proper test isolation in parallel execution
-	t.Setenv("SPIKE_STACK_TRACES_ON_LOG_FATAL", "true")
-
-	// Set required env vars
-	t.Setenv("SPIKE_NEXUS_SHAMIR_SHARES", "3")
-	t.Setenv("SPIKE_NEXUS_SHAMIR_THRESHOLD", "2")
-
-	// Reset and call RootShares() the first time (should succeed)
-	resetRootSharesForTesting()
-	shares := RootShares()
-	if len(shares) != 3 {
-		t.Fatalf("Expected 3 shares, got %d", len(shares))
-	}
-
-	// Call RootShares() a second time (should panic via log.FatalLn)
-	defer func() {
-		if r := recover(); r == nil {
-			t.Error("RootShares() should panic when called more than once")
-		}
-	}()
-
-	_ = RootShares() // This MUST panic
-	t.Error("Should not reach this line - RootShares() must panic on second call")
-}
-
 func TestShareValidation(t *testing.T) {
-	// Test that shares have expected properties
+	// Test that shares have expected properties.
 	shares := createTestShares(t, 3)
 
 	// All shares should have different IDs