test: increase test coverage in pkg/flags, pkg/filesystem, pkg/http, and pkg/function

pkg/filesystem/glob_atomic_test.go

@@ -0,0 +1,216 @@
+//go:build !windows
+
+package filesystem
+
+import (
+	"os"
+	"path/filepath"
+	"sync"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestGetGlobMatches_CacheHit verifies that GetGlobMatches returns cached results
+// on a second call with the same pattern, without re-reading the filesystem.
+func TestGetGlobMatches_CacheHit(t *testing.T) {
+	// Use a fresh sync map state by clearing it.
+	getGlobMatchesSyncMap = sync.Map{}
+
+	tmpDir := t.TempDir()
+	require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "a.yaml"), []byte(""), 0o644))
+	require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "b.yaml"), []byte(""), 0o644))
+
+	pattern := filepath.Join(tmpDir, "*.yaml")
+
+	// First call - cache miss.
+	first, err := GetGlobMatches(pattern)
+	require.NoError(t, err)
+	assert.Len(t, first, 2)
+
+	// Second call with same pattern - should hit cache.
+	second, err := GetGlobMatches(pattern)
+	require.NoError(t, err)
+	assert.Len(t, second, 2)
+
+	// Results should be equal.
+	assert.ElementsMatch(t, first, second)
+}
+
+// TestGetGlobMatches_CacheIsolation verifies that cached results are cloned, so
+// mutating the returned slice does not corrupt subsequent calls.
+func TestGetGlobMatches_CacheIsolation(t *testing.T) {
+	getGlobMatchesSyncMap = sync.Map{}
+
+	tmpDir := t.TempDir()
+	require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "c.yaml"), []byte(""), 0o644))
+
+	pattern := filepath.Join(tmpDir, "*.yaml")
+
+	first, err := GetGlobMatches(pattern)
+	require.NoError(t, err)
+
+	// Mutate the returned slice.
+	if len(first) > 0 {
+		first[0] = "mutated"
+	}
+
+	// Second call should still return the original value.
+	second, err := GetGlobMatches(pattern)
+	require.NoError(t, err)
+	if len(second) > 0 {
+		assert.NotEqual(t, "mutated", second[0])
+	}
+}
+
+// TestGetGlobMatches_NonExistentBaseDir verifies that GetGlobMatches returns an
+// appropriate error when the base directory does not exist.
+func TestGetGlobMatches_NonExistentBaseDir(t *testing.T) {
+	getGlobMatchesSyncMap = sync.Map{}
+
+	// Use a path that is guaranteed not to exist (use a tmpdir + nonexistent subdir).
+	pattern := filepath.Join(os.TempDir(), "atmos-nonexistent-dir-xyzzy-12345", "*.yaml")
+	_, err := GetGlobMatches(pattern)
+	require.Error(t, err, "expected error for non-existent base directory")
+}
+
+// TestGetGlobMatches_EmptyResults verifies that a pattern matching no files returns
+// an empty slice (not an error).
+func TestGetGlobMatches_EmptyResults(t *testing.T) {
+	getGlobMatchesSyncMap = sync.Map{}
+
+	tmpDir := t.TempDir()
+	// No files created in tmpDir.
+
+	pattern := filepath.Join(tmpDir, "*.yaml")
+	matches, err := GetGlobMatches(pattern)
+	require.NoError(t, err)
+	assert.Empty(t, matches)
+}
+
+// TestGetGlobMatches_EmptyResultsCache verifies that empty results are cached and
+// retrieved without hitting the filesystem again.
+func TestGetGlobMatches_EmptyResultsCache(t *testing.T) {
+	getGlobMatchesSyncMap = sync.Map{}
+
+	tmpDir := t.TempDir()
+
+	pattern := filepath.Join(tmpDir, "*.nonexistent")
+
+	// First call - should return empty (not nil) slice and cache it.
+	first, err := GetGlobMatches(pattern)
+	require.NoError(t, err)
+
+	// Second call should use cache.
+	second, err := GetGlobMatches(pattern)
+	require.NoError(t, err)
+
+	// Both should have the same empty result.
+	assert.Equal(t, len(first), len(second))
+}
+
+// TestPathMatch_CacheHit verifies that the PathMatch cache is used on repeated calls.
+func TestPathMatch_CacheHit(t *testing.T) {
+	// Clear the path match cache.
+	pathMatchCacheMu.Lock()
+	pathMatchCache = make(map[pathMatchKey]bool)
+	pathMatchCacheMu.Unlock()
+
+	pattern := "stacks/**/*.yaml"
+	name := "stacks/dev/vpc.yaml"
+
+	// First call - cache miss.
+	first, err := PathMatch(pattern, name)
+	require.NoError(t, err)
+
+	// Second call - should hit cache.
+	second, err := PathMatch(pattern, name)
+	require.NoError(t, err)
+
+	assert.Equal(t, first, second)
+	assert.True(t, first, "pattern should match the name")
+}
+
+// TestPathMatch_CacheHit_NoMatch verifies that cache entries for non-matching patterns
+// are also cached and returned correctly.
+func TestPathMatch_CacheHit_NoMatch(t *testing.T) {
+	pathMatchCacheMu.Lock()
+	pathMatchCache = make(map[pathMatchKey]bool)
+	pathMatchCacheMu.Unlock()
+
+	pattern := "*.go"
+	name := "file.yaml"
+
+	// First call - cache miss.
+	first, err := PathMatch(pattern, name)
+	require.NoError(t, err)
+	assert.False(t, first)
+
+	// Second call - should hit cache.
+	second, err := PathMatch(pattern, name)
+	require.NoError(t, err)
+
+	assert.Equal(t, first, second)
+}
+
+// TestPathMatch_InvalidPattern verifies that an invalid glob pattern returns an error.
+func TestPathMatch_InvalidPattern(t *testing.T) {
+	pathMatchCacheMu.Lock()
+	pathMatchCache = make(map[pathMatchKey]bool)
+	pathMatchCacheMu.Unlock()
+
+	// An invalid pattern with unclosed bracket.
+	_, err := PathMatch("[invalid", "file.yaml")
+	// doublestar.Match returns an error for invalid patterns.
+	assert.Error(t, err)
+}
+
+// TestWriteFileAtomic verifies that WriteFileAtomic writes file contents correctly.
+func TestWriteFileAtomic(t *testing.T) {
+	tmpDir := t.TempDir()
+	filePath := filepath.Join(tmpDir, "atomic-test.txt")
+	content := []byte("hello atomic world")
+
+	err := WriteFileAtomicUnix(filePath, content, 0o644)
+	require.NoError(t, err)
+
+	// Verify file was written.
+	got, err := os.ReadFile(filePath)
+	require.NoError(t, err)
+	assert.Equal(t, content, got)
+}
+
+// TestWriteFileAtomic_Overwrite verifies that WriteFileAtomic correctly overwrites
+// an existing file atomically.
+func TestWriteFileAtomic_Overwrite(t *testing.T) {
+	tmpDir := t.TempDir()
+	filePath := filepath.Join(tmpDir, "atomic-overwrite.txt")
+
+	// Write initial content.
+	require.NoError(t, os.WriteFile(filePath, []byte("initial content"), 0o644))
+
+	// Overwrite with atomic write.
+	newContent := []byte("new content")
+	err := WriteFileAtomicUnix(filePath, newContent, 0o644)
+	require.NoError(t, err)
+
+	got, err := os.ReadFile(filePath)
+	require.NoError(t, err)
+	assert.Equal(t, newContent, got)
+}
+
+// TestOSFileSystem_WriteFileAtomic verifies that OSFileSystem.WriteFileAtomic works.
+func TestOSFileSystem_WriteFileAtomic(t *testing.T) {
+	fs := NewOSFileSystem()
+	tmpDir := t.TempDir()
+	filePath := filepath.Join(tmpDir, "os-atomic.txt")
+	content := []byte("atomic content via OSFileSystem")
+
+	err := fs.WriteFileAtomic(filePath, content, 0o644)
+	require.NoError(t, err)
+
+	got, err := os.ReadFile(filePath)
+	require.NoError(t, err)
+	assert.Equal(t, content, got)
+}

pkg/flags/compat/compatibility_flags_test.go

@@ -998,3 +998,51 @@ func TestCompatibilityFlagTranslator_ShorthandNormalization(t *testing.T) {
 		})
 	}
 }
+
+// TestCompatibilityFlagTranslator_UnknownBehavior verifies that unknown/custom CompatibilityBehavior
+// values fall through to the default case (passed as-is to Atmos args).
+// This tests the defensive default branches in applyFlagBehaviorWithEquals and
+// applyFlagBehaviorWithoutEquals.
+func TestCompatibilityFlagTranslator_UnknownBehavior(t *testing.T) {
+	// Use a custom behavior value that is not MapToAtmosFlag or AppendToSeparated.
+	unknownBehavior := CompatibilityBehavior(999)
+
+	tests := []struct {
+		name              string
+		args              []string
+		flagMap           map[string]CompatibilityFlag
+		expectedAtmosArgs []string
+		expectedSeparated []string
+	}{
+		{
+			name: "unknown behavior with equals syntax",
+			args: []string{"-custom=value"},
+			flagMap: map[string]CompatibilityFlag{
+				"-custom": {Behavior: unknownBehavior, Target: "--custom"},
+			},
+			// default branch: pass original arg to atmos args.
+			expectedAtmosArgs: []string{"-custom=value"},
+			expectedSeparated: []string{},
+		},
+		{
+			name: "unknown behavior without equals syntax",
+			args: []string{"-custom", "value"},
+			flagMap: map[string]CompatibilityFlag{
+				"-custom": {Behavior: unknownBehavior, Target: "--custom"},
+			},
+			// default branch: pass the flag arg to atmos, ignore the value.
+			expectedAtmosArgs: []string{"-custom", "value"},
+			expectedSeparated: []string{},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			translator := NewCompatibilityFlagTranslator(tt.flagMap)
+			atmosArgs, separatedArgs := translator.Translate(tt.args)
+
+			assert.Equal(t, tt.expectedAtmosArgs, atmosArgs)
+			assert.Equal(t, tt.expectedSeparated, separatedArgs)
+		})
+	}
+}

pkg/flags/compat/separated_test.go

@@ -0,0 +1,96 @@
+package compat
+
+import (
+	"sync"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSetSeparated(t *testing.T) {
+	t.Cleanup(func() { ResetSeparated() })
+
+	args := []string{"-var", "region=us-east-1", "-var-file", "prod.tfvars"}
+	SetSeparated(args)
+
+	got := GetSeparated()
+	assert.Equal(t, args, got)
+}
+
+func TestSetSeparated_DefensiveCopy(t *testing.T) {
+	t.Cleanup(func() { ResetSeparated() })
+
+	original := []string{"-var", "foo=bar"}
+	SetSeparated(original)
+
+	// Mutate the original slice.
+	original[0] = "mutated"
+
+	// GetSeparated should return the original values, not the mutated ones.
+	got := GetSeparated()
+	assert.Equal(t, "-var", got[0], "SetSeparated should make a defensive copy")
+}
+
+func TestGetSeparated_ReturnsNilWhenNotSet(t *testing.T) {
+	t.Cleanup(func() { ResetSeparated() })
+	ResetSeparated()
+
+	got := GetSeparated()
+	assert.Nil(t, got)
+}
+
+func TestGetSeparated_DefensiveCopy(t *testing.T) {
+	t.Cleanup(func() { ResetSeparated() })
+
+	SetSeparated([]string{"-var", "x=1"})
+
+	// Mutate the returned slice.
+	got1 := GetSeparated()
+	got1[0] = "mutated"
+
+	// Second call should return original value.
+	got2 := GetSeparated()
+	assert.Equal(t, "-var", got2[0], "GetSeparated should return a defensive copy")
+}
+
+func TestGetSeparated_ReturnsEmptySliceForEmpty(t *testing.T) {
+	t.Cleanup(func() { ResetSeparated() })
+
+	// SetSeparated with empty slice: append([]string(nil), []string{}...) returns nil.
+	// This is expected Go behavior - appending zero elements to nil yields nil.
+	SetSeparated([]string{})
+
+	got := GetSeparated()
+	// An empty input slice results in nil globalSeparatedArgs (nil == nil is true).
+	assert.Nil(t, got)
+}
+
+func TestResetSeparated(t *testing.T) {
+	t.Cleanup(func() { ResetSeparated() })
+
+	SetSeparated([]string{"-var", "x=1"})
+	assert.NotNil(t, GetSeparated())
+
+	ResetSeparated()
+	assert.Nil(t, GetSeparated())
+}
+
+func TestSeparated_Concurrent(t *testing.T) {
+	t.Cleanup(func() { ResetSeparated() })
+
+	const goroutines = 50
+	var wg sync.WaitGroup
+	wg.Add(goroutines)
+
+	for i := 0; i < goroutines; i++ {
+		go func() {
+			defer wg.Done()
+			SetSeparated([]string{"-var", "key=value"})
+			_ = GetSeparated()
+			ResetSeparated()
+		}()
+	}
+
+	wg.Wait()
+	// No race conditions or panics.
+}