diff --git a/pkg/cli/audit_agent_output_test.go b/pkg/cli/audit_agent_output_test.go index 8ac51d330ee..8bd23a64fc9 100644 --- a/pkg/cli/audit_agent_output_test.go +++ b/pkg/cli/audit_agent_output_test.go @@ -10,8 +10,20 @@ import ( "time" "github.com/github/gh-aw/pkg/workflow" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) +func hasFindingByCategory(findings []Finding, category string) bool { + for _, finding := range findings { + if finding.Category == category { + return true + } + } + + return false +} + // TestKeyFindingsGeneration verifies key findings are generated correctly func TestKeyFindingsGeneration(t *testing.T) { tests := []struct { @@ -100,51 +112,32 @@ func TestKeyFindingsGeneration(t *testing.T) { findings := generateFindings(processedRun, tt.metrics, tt.errors) - if len(findings) < tt.expectedCount { - t.Errorf("Expected at least %d findings, got %d", tt.expectedCount, len(findings)) - } + assert.GreaterOrEqual(t, len(findings), tt.expectedCount, + "Expected at least %d findings for scenario %q, got %d", tt.expectedCount, tt.name, len(findings)) // Verify expected categories if tt.hasFailure { - found := false + var failureFinding *Finding for _, finding := range findings { if finding.Category == "error" && strings.Contains(finding.Title, "Failed") { - found = true - if finding.Severity != "critical" { - t.Errorf("Expected critical severity for failure, got %s", finding.Severity) - } + failureFinding = &finding break } } - if !found { - t.Error("Expected failure finding but didn't find one") - } + require.NotNil(t, failureFinding, + "Expected an error finding with 'Failed' in title for scenario %q", tt.name) + assert.Equal(t, "critical", failureFinding.Severity, + "Expected critical severity for failure finding in scenario %q", tt.name) } if tt.hasCost { - found := false - for _, finding := range findings { - if finding.Category == "cost" { - found = true - break - } - } - if !found { - t.Error("Expected cost finding but didn't find one") - } + assert.True(t, hasFindingByCategory(findings, "cost"), + "Expected at least one cost finding for scenario %q", tt.name) } if tt.hasTooling { - found := false - for _, finding := range findings { - if finding.Category == "tooling" { - found = true - break - } - } - if !found { - t.Error("Expected tooling finding but didn't find one") - } + assert.True(t, hasFindingByCategory(findings, "tooling"), + "Expected at least one tooling finding for scenario %q", tt.name) } }) } @@ -221,34 +214,29 @@ func TestRecommendationsGeneration(t *testing.T) { recommendations := generateRecommendations(processedRun, tt.metrics, tt.findings) - if len(recommendations) < tt.expectedMinCount { - t.Errorf("Expected at least %d recommendations, got %d", tt.expectedMinCount, len(recommendations)) - } + assert.GreaterOrEqual(t, len(recommendations), tt.expectedMinCount, + "Expected at least %d recommendations for scenario %q, got %d", + tt.expectedMinCount, tt.name, len(recommendations)) if tt.hasHighPriority { - found := false - for _, rec := range recommendations { - if rec.Priority == "high" { - found = true - break + assert.Condition(t, func() bool { + for _, rec := range recommendations { + if rec.Priority == "high" { + return true + } } - } - if !found { - t.Error("Expected high priority recommendation but didn't find one") - } + return false + }, "Expected at least one high priority recommendation for scenario %q", tt.name) } // Verify all recommendations have required fields for _, rec := range recommendations { - if rec.Action == "" { - t.Error("Recommendation missing action") - } - if rec.Reason == "" { - t.Error("Recommendation missing reason") - } - if rec.Priority == "" { - t.Error("Recommendation missing priority") - } + assert.NotEmpty(t, rec.Action, + "Recommendation action should be set for scenario %q", tt.name) + assert.NotEmpty(t, rec.Reason, + "Recommendation reason should be set for scenario %q", tt.name) + assert.NotEmpty(t, rec.Priority, + "Recommendation priority should be set for scenario %q", tt.name) } }) } @@ -323,32 +311,26 @@ func TestPerformanceMetricsGeneration(t *testing.T) { pm := generatePerformanceMetrics(processedRun, tt.metrics, tt.toolUsage) - if pm == nil { - t.Fatal("Expected performance metrics but got nil") - } + require.NotNil(t, pm, "Expected performance metrics to be generated for scenario %q", tt.name) if tt.expectedCostEfficiency != "" { - if pm.CostEfficiency != tt.expectedCostEfficiency { - t.Errorf("Expected cost efficiency '%s', got '%s'", tt.expectedCostEfficiency, pm.CostEfficiency) - } + assert.Equal(t, tt.expectedCostEfficiency, pm.CostEfficiency, + "Expected cost efficiency to match for scenario %q", tt.name) } if tt.expectTokensPerMin { - if pm.TokensPerMinute <= 0 { - t.Error("Expected positive tokens per minute") - } + assert.Positive(t, pm.TokensPerMinute, + "Expected positive tokens per minute for scenario %q", tt.name) } if tt.expectMostUsedTool { - if pm.MostUsedTool == "" { - t.Error("Expected most used tool to be set") - } + assert.NotEmpty(t, pm.MostUsedTool, + "Expected most used tool to be populated for scenario %q", tt.name) } if tt.expectNetworkRequests { - if pm.NetworkRequests <= 0 { - t.Error("Expected network requests count") - } + assert.Positive(t, pm.NetworkRequests, + "Expected network request count to be positive for scenario %q", tt.name) } }) } @@ -401,9 +383,7 @@ func TestAuditDataJSONStructure(t *testing.T) { // Marshal to JSON jsonBytes, err := json.MarshalIndent(auditData, "", " ") - if err != nil { - t.Fatalf("Failed to marshal audit data to JSON: %v", err) - } + require.NoError(t, err, "Failed to marshal audit data to JSON") jsonStr := string(jsonBytes) @@ -424,46 +404,27 @@ func TestAuditDataJSONStructure(t *testing.T) { } for _, field := range expectedFields { - if !strings.Contains(jsonStr, fmt.Sprintf(`"%s"`, field)) { - t.Errorf("JSON output missing expected field: %s", field) - } + assert.Contains(t, jsonStr, fmt.Sprintf(`"%s"`, field), + "JSON output should include expected field %q", field) } // Verify key findings structure - if !strings.Contains(jsonStr, `"category"`) { - t.Error("Key findings missing category field") - } - if !strings.Contains(jsonStr, `"severity"`) { - t.Error("Key findings missing severity field") - } + assert.Contains(t, jsonStr, `"category"`, "Key findings should include category field") + assert.Contains(t, jsonStr, `"severity"`, "Key findings should include severity field") // Verify recommendations structure - if !strings.Contains(jsonStr, `"priority"`) { - t.Error("Recommendations missing priority field") - } - if !strings.Contains(jsonStr, `"action"`) { - t.Error("Recommendations missing action field") - } + assert.Contains(t, jsonStr, `"priority"`, "Recommendations should include priority field") + assert.Contains(t, jsonStr, `"action"`, "Recommendations should include action field") // Verify performance metrics structure - if !strings.Contains(jsonStr, `"cost_efficiency"`) { - t.Error("Performance metrics missing cost_efficiency field") - } + assert.Contains(t, jsonStr, `"cost_efficiency"`, "Performance metrics should include cost_efficiency field") // Parse back to verify structure var parsed AuditData - if err := json.Unmarshal(jsonBytes, &parsed); err != nil { - t.Fatalf("Failed to parse JSON back to AuditData: %v", err) - } + require.NoError(t, json.Unmarshal(jsonBytes, &parsed), "Failed to parse JSON back to AuditData") // Verify parsed data has expected content - if len(parsed.KeyFindings) == 0 { - t.Error("Expected key findings but got none") - } - if len(parsed.Recommendations) == 0 { - t.Error("Expected recommendations but got none") - } - if parsed.PerformanceMetrics == nil { - t.Error("Expected performance metrics but got nil") - } + assert.NotEmpty(t, parsed.KeyFindings, "Expected key findings to be present after JSON round-trip") + assert.NotEmpty(t, parsed.Recommendations, "Expected recommendations to be present after JSON round-trip") + assert.NotNil(t, parsed.PerformanceMetrics, "Expected performance metrics to be present after JSON round-trip") }