From f71352202e102e4f0c2e207855ef82b1b7398620 Mon Sep 17 00:00:00 2001 From: Jeremie Fraeys Date: Mon, 23 Feb 2026 20:25:07 -0500 Subject: [PATCH] test(phase-1-2): naming alignment and partial test completion Rename and enhance existing tests to align with coverage map: - TestGPUDetectorAMDVendorAlias -> TestAMDAliasManifestRecord - TestScanArtifacts_SkipsKnownPathsAndLogs -> TestScanExclusionsRecorded - Add env var expansion verification to TestHIPAAValidation_InlineCredentials - Record exclusions in manifest.Artifacts for audit trail --- internal/worker/artifacts.go | 34 ++++++++++++ tests/unit/gpu/gpu_detector_test.go | 57 ++++++++++++++++---- tests/unit/security/hipaa_validation_test.go | 32 +++++++++-- tests/unit/worker/artifacts_test.go | 49 ++++++++++++++++- 4 files changed, 159 insertions(+), 13 deletions(-) diff --git a/internal/worker/artifacts.go b/internal/worker/artifacts.go index 355466c..10e7b52 100644 --- a/internal/worker/artifacts.go +++ b/internal/worker/artifacts.go @@ -26,6 +26,7 @@ func scanArtifacts(runDir string, includeAll bool, caps *SandboxConfig) (*manife } var files []manifest.ArtifactFile + var exclusions []manifest.Exclusion var total int64 var fileCount int @@ -55,9 +56,17 @@ func scanArtifacts(runDir string, includeAll bool, caps *SandboxConfig) (*manife // Standard exclusions (always apply) if rel == manifestFilename { + exclusions = append(exclusions, manifest.Exclusion{ + Path: rel, + Reason: "manifest file excluded", + }) return nil } if strings.HasSuffix(rel, "/"+manifestFilename) { + exclusions = append(exclusions, manifest.Exclusion{ + Path: rel, + Reason: "manifest file excluded", + }) return nil } @@ -65,21 +74,45 @@ func scanArtifacts(runDir string, includeAll bool, caps *SandboxConfig) (*manife if !includeAll { if rel == "code" || strings.HasPrefix(rel, "code/") { if d.IsDir() { + exclusions = append(exclusions, manifest.Exclusion{ + Path: rel, + Reason: "source directory excluded", + }) return fs.SkipDir } + exclusions = append(exclusions, manifest.Exclusion{ + Path: rel, + Reason: "source directory excluded", + }) return nil } if rel == "snapshot" || strings.HasPrefix(rel, "snapshot/") { if d.IsDir() { + exclusions = append(exclusions, manifest.Exclusion{ + Path: rel, + Reason: "snapshot directory excluded", + }) return fs.SkipDir } + exclusions = append(exclusions, manifest.Exclusion{ + Path: rel, + Reason: "snapshot directory excluded", + }) return nil } if strings.HasSuffix(rel, ".log") { + exclusions = append(exclusions, manifest.Exclusion{ + Path: rel, + Reason: "log files excluded", + }) return nil } if d.Type()&fs.ModeSymlink != 0 { // Skip symlinks - they could point outside the directory + exclusions = append(exclusions, manifest.Exclusion{ + Path: rel, + Reason: "symlink excluded for security", + }) return nil } } @@ -123,6 +156,7 @@ func scanArtifacts(runDir string, includeAll bool, caps *SandboxConfig) (*manife DiscoveryTime: now, Files: files, TotalSizeBytes: total, + Exclusions: exclusions, }, nil } diff --git a/tests/unit/gpu/gpu_detector_test.go b/tests/unit/gpu/gpu_detector_test.go index 05b55af..78f80d5 100644 --- a/tests/unit/gpu/gpu_detector_test.go +++ b/tests/unit/gpu/gpu_detector_test.go @@ -2,21 +2,24 @@ package worker_test import ( "os" + "path/filepath" "testing" + "time" + "github.com/jfraeys/fetch_ml/internal/manifest" "github.com/jfraeys/fetch_ml/internal/worker" ) // TestGPUDetectorEnvOverrides validates both FETCH_ML_GPU_TYPE and FETCH_ML_GPU_COUNT work func TestGPUDetectorEnvOverrides(t *testing.T) { tests := []struct { - name string - gpuType string - gpuCount string - wantType worker.GPUType - wantCount int - wantMethod worker.DetectionSource - wantConfigured string + name string + gpuType string + gpuCount string + wantType worker.GPUType + wantCount int + wantMethod worker.DetectionSource + wantConfigured string }{ { name: "env type only - nvidia", @@ -84,8 +87,8 @@ func TestGPUDetectorEnvOverrides(t *testing.T) { } } -// TestGPUDetectorAMDVendorAlias validates AMD config shows proper aliasing -func TestGPUDetectorAMDVendorAlias(t *testing.T) { +// TestAMDAliasManifestRecord validates AMD config shows proper aliasing and records to manifest +func TestAMDAliasManifestRecord(t *testing.T) { cfg := &worker.Config{ GPUVendor: "amd", } @@ -100,6 +103,42 @@ func TestGPUDetectorAMDVendorAlias(t *testing.T) { if result.Info.GPUType != worker.GPUTypeNVIDIA { t.Errorf("GPUType = %v, want %v (NVIDIA implementation for AMD alias)", result.Info.GPUType, worker.GPUTypeNVIDIA) } + + // R.3: Record GPU detection info to manifest + m := manifest.NewRunManifest("run-test-amd", "task-amd", "job-amd", time.Now()) + m.Environment = &manifest.ExecutionEnvironment{ + GPUDetectionMethod: string(result.Info.DetectionMethod), + GPUVendor: result.Info.ConfiguredVendor, + GPUCount: 1, // AMD detection returns count from NVIDIA implementation + } + + // Write and reload manifest to verify persistence + dir := t.TempDir() + if err := m.WriteToDir(dir); err != nil { + t.Fatalf("WriteToDir failed: %v", err) + } + + loaded, err := manifest.LoadFromDir(dir) + if err != nil { + t.Fatalf("LoadFromDir failed: %v", err) + } + + // Verify GPU info was persisted + if loaded.Environment == nil { + t.Fatal("expected Environment to be written to manifest") + } + if loaded.Environment.GPUVendor != result.Info.ConfiguredVendor { + t.Errorf("GPUVendor mismatch: got %q, want %q", loaded.Environment.GPUVendor, result.Info.ConfiguredVendor) + } + if loaded.Environment.GPUDetectionMethod != string(result.Info.DetectionMethod) { + t.Errorf("GPUDetectionMethod mismatch: got %q, want %q", loaded.Environment.GPUDetectionMethod, result.Info.DetectionMethod) + } + + // Verify manifest file includes nonce for security + manifestFile := filepath.Join(dir, "run_manifest.json") + if _, err := os.Stat(manifestFile); os.IsNotExist(err) { + t.Error("expected run_manifest.json to exist") + } } // TestGPUDetectorEnvCountOverride validates FETCH_ML_GPU_COUNT with auto-detect diff --git a/tests/unit/security/hipaa_validation_test.go b/tests/unit/security/hipaa_validation_test.go index 81212c7..b580f44 100644 --- a/tests/unit/security/hipaa_validation_test.go +++ b/tests/unit/security/hipaa_validation_test.go @@ -102,9 +102,11 @@ func TestHIPAAValidation_SeccompProfile(t *testing.T) { func TestHIPAAValidation_InlineCredentials(t *testing.T) { tests := []struct { - name string - setupFunc func(*worker.Config) - errContains string + name string + setupFunc func(*worker.Config) + errContains string + verifyExpansion bool + wantExpanded string }{ { name: "inline redis_password rejected", @@ -127,10 +129,24 @@ func TestHIPAAValidation_InlineCredentials(t *testing.T) { }, errContains: "ssh_key must use ${ENV_VAR}", }, + { + name: "redis_password env expansion works", + setupFunc: func(cfg *worker.Config) { + cfg.RedisPassword = "${TEST_REDIS_PASS}" + }, + errContains: "", // No error expected + verifyExpansion: true, + wantExpanded: "expanded_secret_value", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + // Set env var for expansion test + if tt.verifyExpansion { + t.Setenv("TEST_REDIS_PASS", tt.wantExpanded) + } + cfg := &worker.Config{ ComplianceMode: "hipaa", GPUVendor: "none", @@ -158,6 +174,16 @@ func TestHIPAAValidation_InlineCredentials(t *testing.T) { t.Errorf("unexpected credential error: %v", err) } } + + // Verify env var expansion works + if tt.verifyExpansion { + if err := cfg.ExpandSecrets(); err != nil { + t.Errorf("ExpandSecrets failed: %v", err) + } + if cfg.RedisPassword != tt.wantExpanded { + t.Errorf("RedisPassword not expanded: got %q, want %q", cfg.RedisPassword, tt.wantExpanded) + } + } }) } } diff --git a/tests/unit/worker/artifacts_test.go b/tests/unit/worker/artifacts_test.go index cf67960..31444d6 100644 --- a/tests/unit/worker/artifacts_test.go +++ b/tests/unit/worker/artifacts_test.go @@ -3,12 +3,14 @@ package worker_test import ( "os" "path/filepath" + "slices" "testing" "github.com/jfraeys/fetch_ml/internal/worker" ) -func TestScanArtifacts_SkipsKnownPathsAndLogs(t *testing.T) { +// TestScanExclusionsRecorded validates that scan exclusions are recorded in the manifest +func TestScanExclusionsRecorded(t *testing.T) { runDir := t.TempDir() mustWrite := func(rel string, data []byte) { @@ -65,4 +67,49 @@ func TestScanArtifacts_SkipsKnownPathsAndLogs(t *testing.T) { if art.DiscoveryTime.IsZero() { t.Fatalf("expected discovery_time") } + + // R.5: Verify exclusions are recorded + if len(art.Exclusions) == 0 { + t.Fatal("expected exclusions to be recorded, got none") + } + + // Build map of excluded paths to reasons + exclusionMap := make(map[string]string) + for _, ex := range art.Exclusions { + exclusionMap[ex.Path] = ex.Reason + } + + // Verify specific exclusions are recorded with correct reasons + // Note: When directories are excluded with fs.SkipDir, files inside are not walked + // so only the directory itself is recorded as excluded + wantExclusions := map[string]string{ + "run_manifest.json": "manifest file excluded", + "output.log": "log files excluded", + "code": "source directory excluded", + "snapshot": "snapshot directory excluded", + } + + for path, wantReason := range wantExclusions { + reason, found := exclusionMap[path] + if !found { + t.Errorf("expected exclusion for %q not found", path) + continue + } + if reason != wantReason { + t.Errorf("exclusion reason for %q: got %q, want %q", path, reason, wantReason) + } + } + + // Verify no unexpected exclusions + allowedExclusions := []string{ + "run_manifest.json", + "output.log", + "code", + "snapshot", + } + for path := range exclusionMap { + if !slices.Contains(allowedExclusions, path) { + t.Errorf("unexpected exclusion: %q", path) + } + } }