diff --git a/pkg/actionpins/actionpins_internal_test.go b/pkg/actionpins/actionpins_internal_test.go index c92cad4616e..de731509e7c 100644 --- a/pkg/actionpins/actionpins_internal_test.go +++ b/pkg/actionpins/actionpins_internal_test.go @@ -53,13 +53,19 @@ func TestInitWarnings_InitializesAndPreservesMap(t *testing.T) { }) t.Run("preserves existing warnings map", func(t *testing.T) { - existing := map[string]bool{"actions/checkout@v5": true} - ctx := &PinContext{Warnings: existing} + existing := map[string]struct{}{"actions/checkout@v5": {}} + ctx := &PinContext{Warnings: make(map[string]bool, len(existing))} + for warning := range existing { + ctx.Warnings[warning] = true + } initWarnings(ctx) require.NotNil(t, ctx.Warnings, "Expected warnings map to remain initialized") - assert.Equal(t, existing, ctx.Warnings, "Expected existing warnings entries to be preserved") + assert.Len(t, ctx.Warnings, len(existing), "Expected existing warnings entries to be preserved") + for warning := range existing { + assert.True(t, ctx.Warnings[warning], "Expected warning %q to be preserved", warning) + } }) } diff --git a/pkg/cli/add_interactive_schedule_test.go b/pkg/cli/add_interactive_schedule_test.go index 189c4610090..9c5f4b24f00 100644 --- a/pkg/cli/add_interactive_schedule_test.go +++ b/pkg/cli/add_interactive_schedule_test.go @@ -407,15 +407,15 @@ func TestBuildScheduleOptions(t *testing.T) { t.Run("includes all standard frequencies", func(t *testing.T) { opts := buildScheduleOptions("daily", "daily") require.NotEmpty(t, opts, "options should not be empty") - values := make(map[string]bool) + values := make(map[string]struct{}) for _, o := range opts { - values[o.Value] = true + values[o.Value] = struct{}{} } - assert.True(t, values["hourly"], "hourly should be included") - assert.True(t, values["3-hourly"], "3-hourly should be included") - assert.True(t, values["daily"], "daily should be included") - assert.True(t, values["weekly"], "weekly should be included") - assert.True(t, values["monthly"], "monthly should be included") + assert.Contains(t, values, "hourly", "hourly should be included") + assert.Contains(t, values, "3-hourly", "3-hourly should be included") + assert.Contains(t, values, "daily", "daily should be included") + assert.Contains(t, values, "weekly", "weekly should be included") + assert.Contains(t, values, "monthly", "monthly should be included") }) t.Run("custom schedule has no duplicate custom option", func(t *testing.T) { diff --git a/pkg/cli/copilot_events_jsonl_test.go b/pkg/cli/copilot_events_jsonl_test.go index 5c011fa1265..f136f6a4ca7 100644 --- a/pkg/cli/copilot_events_jsonl_test.go +++ b/pkg/cli/copilot_events_jsonl_test.go @@ -172,12 +172,12 @@ func TestParseEventsJSONLFile(t *testing.T) { } // Verify each expected tool name appears in ToolCalls - toolNames := make(map[string]bool) + toolNames := make(map[string]struct{}) for _, tc := range metrics.ToolCalls { - toolNames[tc.Name] = true + toolNames[tc.Name] = struct{}{} } for _, expectedTool := range tt.wantToolCalls { - assert.True(t, toolNames[expectedTool], "tool %q should be in ToolCalls", expectedTool) + assert.Contains(t, toolNames, expectedTool, "tool %q should be in ToolCalls", expectedTool) } }) } diff --git a/pkg/cli/dependency_graph_test.go b/pkg/cli/dependency_graph_test.go index fa57aeaaef3..1101d20fe27 100644 --- a/pkg/cli/dependency_graph_test.go +++ b/pkg/cli/dependency_graph_test.go @@ -123,15 +123,15 @@ description: Standalone workflow } // Check that both importers are in the list - affectedMap := make(map[string]bool) + affectedMap := make(map[string]struct{}) for _, w := range affected { - affectedMap[w] = true + affectedMap[w] = struct{}{} } - if !affectedMap[topWorkflow1] { + if _, ok := affectedMap[topWorkflow1]; !ok { t.Errorf("GetAffectedWorkflows() should include %s", topWorkflow1) } - if !affectedMap[topWorkflow2] { + if _, ok := affectedMap[topWorkflow2]; !ok { t.Errorf("GetAffectedWorkflows() should include %s", topWorkflow2) } }) @@ -353,13 +353,13 @@ imports: } // Verify all three workflows are in the affected list - affectedMap := make(map[string]bool) + affectedMap := make(map[string]struct{}) for _, w := range affected { - affectedMap[w] = true + affectedMap[w] = struct{}{} } for i, wf := range workflows { - if !affectedMap[wf] { + if _, ok := affectedMap[wf]; !ok { t.Errorf("GetAffectedWorkflows() should include workflow%d.md", i) } } @@ -662,12 +662,18 @@ imports: t.Errorf("GetAffectedWorkflows(C) returned %d workflows, want 3", len(affected)) } - affectedMap := make(map[string]bool) + affectedMap := make(map[string]struct{}) for _, w := range affected { - affectedMap[w] = true + affectedMap[w] = struct{}{} } - if !affectedMap[top1] || !affectedMap[top2] || !affectedMap[top3] { + if _, ok := affectedMap[top1]; !ok { + t.Errorf("GetAffectedWorkflows(C) should include top1, top2, and top3") + } + if _, ok := affectedMap[top2]; !ok { + t.Errorf("GetAffectedWorkflows(C) should include top1, top2, and top3") + } + if _, ok := affectedMap[top3]; !ok { t.Errorf("GetAffectedWorkflows(C) should include top1, top2, and top3") } }) @@ -680,12 +686,18 @@ imports: t.Errorf("GetAffectedWorkflows(B) returned %d workflows, want 3", len(affected)) } - affectedMap := make(map[string]bool) + affectedMap := make(map[string]struct{}) for _, w := range affected { - affectedMap[w] = true + affectedMap[w] = struct{}{} } - if !affectedMap[top1] || !affectedMap[top2] || !affectedMap[top3] { + if _, ok := affectedMap[top1]; !ok { + t.Errorf("GetAffectedWorkflows(B) should include top1, top2, and top3") + } + if _, ok := affectedMap[top2]; !ok { + t.Errorf("GetAffectedWorkflows(B) should include top1, top2, and top3") + } + if _, ok := affectedMap[top3]; !ok { t.Errorf("GetAffectedWorkflows(B) should include top1, top2, and top3") } }) @@ -698,16 +710,19 @@ imports: t.Errorf("GetAffectedWorkflows(A) returned %d workflows, want 2", len(affected)) } - affectedMap := make(map[string]bool) + affectedMap := make(map[string]struct{}) for _, w := range affected { - affectedMap[w] = true + affectedMap[w] = struct{}{} } - if !affectedMap[top1] || !affectedMap[top2] { + if _, ok := affectedMap[top1]; !ok { + t.Errorf("GetAffectedWorkflows(A) should include top1 and top2") + } + if _, ok := affectedMap[top2]; !ok { t.Errorf("GetAffectedWorkflows(A) should include top1 and top2") } - if affectedMap[top3] { + if _, ok := affectedMap[top3]; ok { t.Errorf("GetAffectedWorkflows(A) should NOT include top3") } }) diff --git a/pkg/cli/engine_secrets.go b/pkg/cli/engine_secrets.go index a447a557f88..fd9e2522d2c 100644 --- a/pkg/cli/engine_secrets.go +++ b/pkg/cli/engine_secrets.go @@ -174,6 +174,18 @@ func getMissingRequiredSecrets(requirements []SecretRequirement, existingSecrets return missing } +func stringSetToBoolMap(set map[string]struct{}) map[string]bool { + if len(set) == 0 { + return map[string]bool{} + } + + result := make(map[string]bool, len(set)) + for item := range set { + result[item] = true + } + return result +} + // ctx returns the context from the config, defaulting to background if nil func (c EngineSecretConfig) ctx() context.Context { if c.Ctx != nil { diff --git a/pkg/cli/engine_secrets_test.go b/pkg/cli/engine_secrets_test.go index adb9df6f73b..6f08e051271 100644 --- a/pkg/cli/engine_secrets_test.go +++ b/pkg/cli/engine_secrets_test.go @@ -96,12 +96,12 @@ func TestGetRequiredSecretsForEngine(t *testing.T) { "Should have at most %d requirements", tt.wantMaxCount) // Check that expected secrets are present - secretNames := make(map[string]bool) + secretNames := make(map[string]struct{}) for _, req := range requirements { - secretNames[req.Name] = true + secretNames[req.Name] = struct{}{} } for _, wantName := range tt.wantSecretNames { - assert.True(t, secretNames[wantName], + assert.Contains(t, secretNames, wantName, "Should include secret %s", wantName) } }) @@ -331,11 +331,11 @@ func TestGetEngineSecretNameAndValue(t *testing.T) { }() t.Run("secret exists in repository", func(t *testing.T) { - existingSecrets := map[string]bool{ - "COPILOT_GITHUB_TOKEN": true, + existingSecrets := map[string]struct{}{ + "COPILOT_GITHUB_TOKEN": {}, } - name, value, existsInRepo, err := GetEngineSecretNameAndValue("copilot", existingSecrets) + name, value, existsInRepo, err := GetEngineSecretNameAndValue("copilot", stringSetToBoolMap(existingSecrets)) require.NoError(t, err, "Should not error when secret exists in repo") assert.Equal(t, "COPILOT_GITHUB_TOKEN", name) @@ -347,9 +347,9 @@ func TestGetEngineSecretNameAndValue(t *testing.T) { os.Setenv("ANTHROPIC_API_KEY", "test-api-key-12345") defer os.Unsetenv("ANTHROPIC_API_KEY") - existingSecrets := map[string]bool{} + existingSecrets := map[string]struct{}{} - name, value, existsInRepo, err := GetEngineSecretNameAndValue("claude", existingSecrets) + name, value, existsInRepo, err := GetEngineSecretNameAndValue("claude", stringSetToBoolMap(existingSecrets)) require.NoError(t, err, "Should not error when secret in environment") assert.Equal(t, "ANTHROPIC_API_KEY", name) @@ -361,9 +361,9 @@ func TestGetEngineSecretNameAndValue(t *testing.T) { os.Unsetenv("OPENAI_API_KEY") os.Unsetenv("CODEX_API_KEY") - existingSecrets := map[string]bool{} + existingSecrets := map[string]struct{}{} - name, value, existsInRepo, err := GetEngineSecretNameAndValue("codex", existingSecrets) + name, value, existsInRepo, err := GetEngineSecretNameAndValue("codex", stringSetToBoolMap(existingSecrets)) require.NoError(t, err, "Should not error even when secret not found") assert.Equal(t, "OPENAI_API_KEY", name) @@ -372,18 +372,18 @@ func TestGetEngineSecretNameAndValue(t *testing.T) { }) t.Run("unknown engine returns error", func(t *testing.T) { - existingSecrets := map[string]bool{} + existingSecrets := map[string]struct{}{} - _, _, _, err := GetEngineSecretNameAndValue("unknown-engine", existingSecrets) + _, _, _, err := GetEngineSecretNameAndValue("unknown-engine", stringSetToBoolMap(existingSecrets)) require.Error(t, err, "Should error for unknown engine") assert.Contains(t, err.Error(), "unknown engine", "Error should mention unknown engine") }) t.Run("no alternative secret in repo", func(t *testing.T) { - existingSecrets := map[string]bool{} + existingSecrets := map[string]struct{}{} - name, value, existsInRepo, err := GetEngineSecretNameAndValue("claude", existingSecrets) + name, value, existsInRepo, err := GetEngineSecretNameAndValue("claude", stringSetToBoolMap(existingSecrets)) require.NoError(t, err, "Should not error") assert.Equal(t, "ANTHROPIC_API_KEY", name) @@ -395,11 +395,11 @@ func TestGetEngineSecretNameAndValue(t *testing.T) { os.Setenv("COPILOT_GITHUB_TOKEN", "test-token-from-env") defer os.Unsetenv("COPILOT_GITHUB_TOKEN") - existingSecrets := map[string]bool{ - "COPILOT_GITHUB_TOKEN": true, + existingSecrets := map[string]struct{}{ + "COPILOT_GITHUB_TOKEN": {}, } - name, value, existsInRepo, err := GetEngineSecretNameAndValue("copilot", existingSecrets) + name, value, existsInRepo, err := GetEngineSecretNameAndValue("copilot", stringSetToBoolMap(existingSecrets)) require.NoError(t, err, "Should not error") assert.Equal(t, "COPILOT_GITHUB_TOKEN", name) @@ -415,9 +415,9 @@ func TestGetMissingRequiredSecrets(t *testing.T) { {Name: "SECRET2", Optional: false}, {Name: "SECRET3", Optional: false}, } - existingSecrets := map[string]bool{} + existingSecrets := map[string]struct{}{} - missing := getMissingRequiredSecrets(requirements, existingSecrets) + missing := getMissingRequiredSecrets(requirements, stringSetToBoolMap(existingSecrets)) assert.Len(t, missing, 3, "Should have 3 missing secrets") assert.Equal(t, "SECRET1", missing[0].Name) @@ -430,12 +430,12 @@ func TestGetMissingRequiredSecrets(t *testing.T) { {Name: "SECRET1", Optional: false}, {Name: "SECRET2", Optional: false}, } - existingSecrets := map[string]bool{ - "SECRET1": true, - "SECRET2": true, + existingSecrets := map[string]struct{}{ + "SECRET1": {}, + "SECRET2": {}, } - missing := getMissingRequiredSecrets(requirements, existingSecrets) + missing := getMissingRequiredSecrets(requirements, stringSetToBoolMap(existingSecrets)) assert.Empty(t, missing, "Should have no missing secrets") }) @@ -446,11 +446,11 @@ func TestGetMissingRequiredSecrets(t *testing.T) { {Name: "SECRET2", Optional: false}, {Name: "SECRET3", Optional: false}, } - existingSecrets := map[string]bool{ - "SECRET1": true, + existingSecrets := map[string]struct{}{ + "SECRET1": {}, } - missing := getMissingRequiredSecrets(requirements, existingSecrets) + missing := getMissingRequiredSecrets(requirements, stringSetToBoolMap(existingSecrets)) assert.Len(t, missing, 2, "Should have 2 missing secrets") assert.Equal(t, "SECRET2", missing[0].Name) @@ -464,9 +464,9 @@ func TestGetMissingRequiredSecrets(t *testing.T) { {Name: "REQUIRED2", Optional: false}, {Name: "OPTIONAL2", Optional: true}, } - existingSecrets := map[string]bool{} + existingSecrets := map[string]struct{}{} - missing := getMissingRequiredSecrets(requirements, existingSecrets) + missing := getMissingRequiredSecrets(requirements, stringSetToBoolMap(existingSecrets)) assert.Len(t, missing, 2, "Should only include required secrets") assert.Equal(t, "REQUIRED1", missing[0].Name) @@ -482,11 +482,11 @@ func TestGetMissingRequiredSecrets(t *testing.T) { }, {Name: "OTHER_SECRET", Optional: false}, } - existingSecrets := map[string]bool{ - "ALT_SECRET1": true, // Alternative exists + existingSecrets := map[string]struct{}{ + "ALT_SECRET1": {}, // Alternative exists } - missing := getMissingRequiredSecrets(requirements, existingSecrets) + missing := getMissingRequiredSecrets(requirements, stringSetToBoolMap(existingSecrets)) assert.Len(t, missing, 1, "Should have 1 missing secret") assert.Equal(t, "OTHER_SECRET", missing[0].Name, "Should not include PRIMARY_SECRET since alternative exists") @@ -500,11 +500,11 @@ func TestGetMissingRequiredSecrets(t *testing.T) { AlternativeEnvVars: []string{"ALT_SECRET1", "ALT_SECRET2"}, }, } - existingSecrets := map[string]bool{ - "ALT_SECRET2": true, // Second alternative exists + existingSecrets := map[string]struct{}{ + "ALT_SECRET2": {}, // Second alternative exists } - missing := getMissingRequiredSecrets(requirements, existingSecrets) + missing := getMissingRequiredSecrets(requirements, stringSetToBoolMap(existingSecrets)) assert.Empty(t, missing, "Should find second alternative") }) @@ -517,23 +517,23 @@ func TestGetMissingRequiredSecrets(t *testing.T) { AlternativeEnvVars: []string{"ALT_SECRET"}, }, } - existingSecrets := map[string]bool{ - "PRIMARY_SECRET": true, - "ALT_SECRET": true, + existingSecrets := map[string]struct{}{ + "PRIMARY_SECRET": {}, + "ALT_SECRET": {}, } - missing := getMissingRequiredSecrets(requirements, existingSecrets) + missing := getMissingRequiredSecrets(requirements, stringSetToBoolMap(existingSecrets)) assert.Empty(t, missing, "Should not include secret when primary exists") }) t.Run("empty requirements list", func(t *testing.T) { requirements := []SecretRequirement{} - existingSecrets := map[string]bool{ - "SECRET1": true, + existingSecrets := map[string]struct{}{ + "SECRET1": {}, } - missing := getMissingRequiredSecrets(requirements, existingSecrets) + missing := getMissingRequiredSecrets(requirements, stringSetToBoolMap(existingSecrets)) assert.Empty(t, missing, "Should return empty list for empty requirements") }) @@ -543,9 +543,9 @@ func TestGetMissingRequiredSecrets(t *testing.T) { {Name: "SECRET1", Optional: false}, {Name: "SECRET2", Optional: false}, } - existingSecrets := map[string]bool{} + existingSecrets := map[string]struct{}{} - missing := getMissingRequiredSecrets(requirements, existingSecrets) + missing := getMissingRequiredSecrets(requirements, stringSetToBoolMap(existingSecrets)) assert.Len(t, missing, 2, "Should return all required secrets as missing") }) @@ -554,9 +554,9 @@ func TestGetMissingRequiredSecrets(t *testing.T) { requirements := []SecretRequirement{ {Name: "SECRET1", Optional: false}, } - var existingSecrets map[string]bool // nil map + var existingSecrets map[string]struct{} // nil map - missing := getMissingRequiredSecrets(requirements, existingSecrets) + missing := getMissingRequiredSecrets(requirements, stringSetToBoolMap(existingSecrets)) assert.Len(t, missing, 1, "Should handle nil map and return all as missing") }) @@ -580,11 +580,11 @@ func TestGetMissingRequiredSecrets(t *testing.T) { AlternativeEnvVars: []string{"CLAUDE_API_KEY"}, }, } - existingSecrets := map[string]bool{ - "GITHUB_TOKEN": true, // Alternative for COPILOT_GITHUB_TOKEN + existingSecrets := map[string]struct{}{ + "GITHUB_TOKEN": {}, // Alternative for COPILOT_GITHUB_TOKEN } - missing := getMissingRequiredSecrets(requirements, existingSecrets) + missing := getMissingRequiredSecrets(requirements, stringSetToBoolMap(existingSecrets)) assert.Len(t, missing, 1, "Should have 1 missing required secret") assert.Equal(t, "ANTHROPIC_API_KEY", missing[0].Name, "Should only include ANTHROPIC_API_KEY") diff --git a/pkg/cli/logs_report_test.go b/pkg/cli/logs_report_test.go index b4cb4b3949a..aec275dfafd 100644 --- a/pkg/cli/logs_report_test.go +++ b/pkg/cli/logs_report_test.go @@ -597,17 +597,17 @@ func TestAggregateDomainStats(t *testing.T) { // TestConvertDomainsToSortedSlices tests the domain conversion helper func TestConvertDomainsToSortedSlices(t *testing.T) { t.Run("converts and sorts domains", func(t *testing.T) { - allowedMap := map[string]bool{ - "z.com": true, - "a.com": true, - "m.com": true, + allowedMap := map[string]struct{}{ + "z.com": {}, + "a.com": {}, + "m.com": {}, } - deniedMap := map[string]bool{ - "y.com": true, - "b.com": true, + deniedMap := map[string]struct{}{ + "y.com": {}, + "b.com": {}, } - allowed, denied := convertDomainsToSortedSlices(allowedMap, deniedMap) + allowed, denied := convertDomainsToSortedSlices(stringSetToBoolMap(allowedMap), stringSetToBoolMap(deniedMap)) // Check sorted order expectedAllowed := []string{"a.com", "m.com", "z.com"} @@ -632,10 +632,10 @@ func TestConvertDomainsToSortedSlices(t *testing.T) { }) t.Run("handles empty maps", func(t *testing.T) { - allowedMap := map[string]bool{} - deniedMap := map[string]bool{} + allowedMap := map[string]struct{}{} + deniedMap := map[string]struct{}{} - allowed, denied := convertDomainsToSortedSlices(allowedMap, deniedMap) + allowed, denied := convertDomainsToSortedSlices(stringSetToBoolMap(allowedMap), stringSetToBoolMap(deniedMap)) if len(allowed) != 0 { t.Errorf("Expected 0 allowed domains, got %d", len(allowed)) diff --git a/pkg/cli/packages_test.go b/pkg/cli/packages_test.go index f65607ae0b5..386a1391c17 100644 --- a/pkg/cli/packages_test.go +++ b/pkg/cli/packages_test.go @@ -118,8 +118,8 @@ on: push // Collect includes var dependencies []IncludeDependency - seen := make(map[string]bool) - err := collectLocalIncludeDependenciesRecursive(tt.content, tmpDir, &dependencies, seen, false) + seen := make(map[string]struct{}) + err := collectLocalIncludeDependenciesRecursive(tt.content, tmpDir, &dependencies, stringSetToBoolMap(seen), false) // Check error expectation if tt.expectedError && err == nil { @@ -135,12 +135,12 @@ on: push } // Check paths - foundPaths := make(map[string]bool) + foundPaths := make(map[string]struct{}) for _, dep := range dependencies { - foundPaths[dep.TargetPath] = true + foundPaths[dep.TargetPath] = struct{}{} } for _, expectedPath := range tt.expectedPaths { - if !foundPaths[expectedPath] { + if _, ok := foundPaths[expectedPath]; !ok { t.Errorf("Expected to find path %s in dependencies", expectedPath) } } diff --git a/pkg/cli/secrets_test.go b/pkg/cli/secrets_test.go index 1295a3564dc..209d4fd8f33 100644 --- a/pkg/cli/secrets_test.go +++ b/pkg/cli/secrets_test.go @@ -127,25 +127,25 @@ func TestExtractSecretsFromConfig(t *testing.T) { } // Create a map of expected secrets for easier lookup - expectedMap := make(map[string]bool) + expectedMap := make(map[string]struct{}) for _, name := range tt.expectedSecrets { - expectedMap[name] = true + expectedMap[name] = struct{}{} } // Check that all extracted secrets are expected for _, secret := range secrets { - if !expectedMap[secret.Name] { + if _, ok := expectedMap[secret.Name]; !ok { t.Errorf("Unexpected secret: %s", secret.Name) } } // Check that all expected secrets were extracted - actualMap := make(map[string]bool) + actualMap := make(map[string]struct{}) for _, secret := range secrets { - actualMap[secret.Name] = true + actualMap[secret.Name] = struct{}{} } for _, expected := range tt.expectedSecrets { - if !actualMap[expected] { + if _, ok := actualMap[expected]; !ok { t.Errorf("Missing expected secret: %s", expected) } } diff --git a/pkg/cli/tokens_bootstrap_test.go b/pkg/cli/tokens_bootstrap_test.go index 2411e75662f..3dcfd105aa9 100644 --- a/pkg/cli/tokens_bootstrap_test.go +++ b/pkg/cli/tokens_bootstrap_test.go @@ -75,18 +75,18 @@ This workflow uses Codex.`, assert.True(t, hasSystemSecret, "Should include system secret GH_AW_GITHUB_TOKEN") // Should include engine secrets for copilot, claude, and codex - engineSecrets := make(map[string]bool) + engineSecrets := make(map[string]struct{}) for _, req := range requirements { if req.IsEngineSecret { - engineSecrets[req.Name] = true + engineSecrets[req.Name] = struct{}{} assert.NotEmpty(t, req.EngineName, "Engine secret should have engine name") assert.False(t, req.Optional, "Engine secret %s should be required", req.Name) } } - assert.True(t, engineSecrets["COPILOT_GITHUB_TOKEN"], "Should include COPILOT_GITHUB_TOKEN") - assert.True(t, engineSecrets["ANTHROPIC_API_KEY"], "Should include ANTHROPIC_API_KEY") - assert.True(t, engineSecrets["OPENAI_API_KEY"], "Should include OPENAI_API_KEY") + assert.Contains(t, engineSecrets, "COPILOT_GITHUB_TOKEN", "Should include COPILOT_GITHUB_TOKEN") + assert.Contains(t, engineSecrets, "ANTHROPIC_API_KEY", "Should include ANTHROPIC_API_KEY") + assert.Contains(t, engineSecrets, "OPENAI_API_KEY", "Should include OPENAI_API_KEY") }) t.Run("filters by engine", func(t *testing.T) { diff --git a/pkg/parser/mcp_test.go b/pkg/parser/mcp_test.go index dcd363a10e6..e1d29f5df5f 100644 --- a/pkg/parser/mcp_test.go +++ b/pkg/parser/mcp_test.go @@ -835,13 +835,13 @@ func TestParseMCPConfig(t *testing.T) { // due to map iteration order, so check for presence rather than exact order if result.Container != "" { // Check that all expected elements are present in args - expectedElements := make(map[string]bool) + expectedElements := make(map[string]struct{}) for _, arg := range tt.expected.Args { - expectedElements[arg] = true + expectedElements[arg] = struct{}{} } - actualElements := make(map[string]bool) + actualElements := make(map[string]struct{}) for _, arg := range result.Args { - actualElements[arg] = true + actualElements[arg] = struct{}{} } if !reflect.DeepEqual(expectedElements, actualElements) { t.Errorf("Expected args elements %v, got %v", tt.expected.Args, result.Args) diff --git a/pkg/parser/schema_deprecated_test.go b/pkg/parser/schema_deprecated_test.go index 55a845d6f8f..eb823384da1 100644 --- a/pkg/parser/schema_deprecated_test.go +++ b/pkg/parser/schema_deprecated_test.go @@ -79,13 +79,13 @@ func TestFindDeprecatedFieldsInFrontmatter(t *testing.T) { } // Check that all expected fields were found - foundMap := make(map[string]bool) + foundMap := make(map[string]struct{}) for _, field := range found { - foundMap[field.Name] = true + foundMap[field.Name] = struct{}{} } for _, wantField := range tt.want { - if !foundMap[wantField] { + if _, ok := foundMap[wantField]; !ok { t.Errorf("Expected to find deprecated field '%s', but it was not found", wantField) } } @@ -348,9 +348,9 @@ func TestFindDeprecatedFieldsInFrontmatterDeep(t *testing.T) { t.Run(tt.name, func(t *testing.T) { found := FindDeprecatedFieldsInFrontmatterDeep(tt.frontmatter, fields) - foundPaths := make(map[string]bool) + foundPaths := make(map[string]struct{}) for _, f := range found { - foundPaths[f.Path] = true + foundPaths[f.Path] = struct{}{} } if len(found) != len(tt.wantPaths) { @@ -358,7 +358,7 @@ func TestFindDeprecatedFieldsInFrontmatterDeep(t *testing.T) { len(found), len(tt.wantPaths), foundPaths, tt.wantPaths) } for _, p := range tt.wantPaths { - if !foundPaths[p] { + if _, ok := foundPaths[p]; !ok { t.Errorf("expected path %q to be found, got %v", p, foundPaths) } } diff --git a/pkg/sliceutil/sliceutil_test.go b/pkg/sliceutil/sliceutil_test.go index 319ab3bea0d..d2a046409e3 100644 --- a/pkg/sliceutil/sliceutil_test.go +++ b/pkg/sliceutil/sliceutil_test.go @@ -167,7 +167,7 @@ func TestMapKeys(t *testing.T) { }) t.Run("single entry map", func(t *testing.T) { - m := map[string]bool{"only": true} + m := map[string]struct{}{"only": {}} result := MapKeys(m) assert.Equal(t, []string{"only"}, result, "MapKeys should return the single key from a one-entry map") diff --git a/pkg/workflow/agentic_engine_test.go b/pkg/workflow/agentic_engine_test.go index 15ee8a7cce7..0c2f99ad76a 100644 --- a/pkg/workflow/agentic_engine_test.go +++ b/pkg/workflow/agentic_engine_test.go @@ -179,10 +179,10 @@ func TestEngineRegistry_GetAllAgentManifestFolders(t *testing.T) { t.Run("no duplicates in result", func(t *testing.T) { registry := NewEngineRegistry() folders := registry.GetAllAgentManifestFolders() - seen := make(map[string]bool) + seen := make(map[string]struct{}) for _, folder := range folders { - assert.False(t, seen[folder], "manifest folders should not contain duplicates, found %q twice", folder) - seen[folder] = true + assert.NotContains(t, seen, folder, "manifest folders should not contain duplicates, found %q twice", folder) + seen[folder] = struct{}{} } }) @@ -217,10 +217,10 @@ func TestEngineRegistry_GetAllAgentManifestFiles(t *testing.T) { t.Run("no duplicates in result", func(t *testing.T) { registry := NewEngineRegistry() files := registry.GetAllAgentManifestFiles() - seen := make(map[string]bool) + seen := make(map[string]struct{}) for _, file := range files { - assert.False(t, seen[file], "manifest files should not contain duplicates, found %q twice", file) - seen[file] = true + assert.NotContains(t, seen, file, "manifest files should not contain duplicates, found %q twice", file) + seen[file] = struct{}{} } }) diff --git a/pkg/workflow/bash_merge_test.go b/pkg/workflow/bash_merge_test.go index 89703cf3d03..a6b22bc5fcc 100644 --- a/pkg/workflow/bash_merge_test.go +++ b/pkg/workflow/bash_merge_test.go @@ -120,13 +120,13 @@ func TestBashToolsMergeCustomWithDefaults(t *testing.T) { } // Check all expected tools are present - actualMap := make(map[string]bool) + actualMap := make(map[string]struct{}) for _, tool := range actual { - actualMap[tool] = true + actualMap[tool] = struct{}{} } for _, expected := range tt.expected { - if !actualMap[expected] { + if _, ok := actualMap[expected]; !ok { t.Errorf("Expected tool '%s' not found in actual tools: %v", expected, actual) } } diff --git a/pkg/workflow/claude_engine_tools_test.go b/pkg/workflow/claude_engine_tools_test.go index d7873e0992c..b159ed12c53 100644 --- a/pkg/workflow/claude_engine_tools_test.go +++ b/pkg/workflow/claude_engine_tools_test.go @@ -336,17 +336,17 @@ func TestClaudeEngineComputeAllowedTools(t *testing.T) { result := engine.computeAllowedClaudeToolsString(tt.tools, nil, cacheMemoryConfig, nil, nil) // Parse expected and actual results into sets for comparison - expectedTools := make(map[string]bool) + expectedTools := make(map[string]struct{}) if tt.expected != "" { for tool := range strings.SplitSeq(tt.expected, ",") { - expectedTools[strings.TrimSpace(tool)] = true + expectedTools[strings.TrimSpace(tool)] = struct{}{} } } - actualTools := make(map[string]bool) + actualTools := make(map[string]struct{}) if result != "" { for tool := range strings.SplitSeq(result, ",") { - actualTools[strings.TrimSpace(tool)] = true + actualTools[strings.TrimSpace(tool)] = struct{}{} } } @@ -358,13 +358,13 @@ func TestClaudeEngineComputeAllowedTools(t *testing.T) { } for expectedTool := range expectedTools { - if !actualTools[expectedTool] { + if _, ok := actualTools[expectedTool]; !ok { t.Errorf("Expected tool '%s' not found in result: '%s'", expectedTool, result) } } for actualTool := range actualTools { - if !expectedTools[actualTool] { + if _, ok := expectedTools[actualTool]; !ok { t.Errorf("Unexpected tool '%s' found in result: '%s'", actualTool, result) } } diff --git a/pkg/workflow/compiler_jobs_test.go b/pkg/workflow/compiler_jobs_test.go index 6c2ca1a505b..37520994503 100644 --- a/pkg/workflow/compiler_jobs_test.go +++ b/pkg/workflow/compiler_jobs_test.go @@ -346,17 +346,17 @@ func TestGetCustomJobsDependingOnPreActivationExcludesActivationDependents(t *te for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { result := compiler.getCustomJobsDependingOnPreActivation(tt.customJobs) - resultSet := make(map[string]bool, len(result)) + resultSet := make(map[string]struct{}, len(result)) for _, j := range result { - resultSet[j] = true + resultSet[j] = struct{}{} } for _, expected := range tt.expectedJobs { - if !resultSet[expected] { + if _, ok := resultSet[expected]; !ok { t.Errorf("Expected job %q in result, got: %v", expected, result) } } for _, excluded := range tt.excludedJobs { - if resultSet[excluded] { + if _, ok := resultSet[excluded]; ok { t.Errorf("Job %q should be excluded from result, got: %v", excluded, result) } } diff --git a/pkg/workflow/custom_job_condition_test.go b/pkg/workflow/custom_job_condition_test.go index ebae3f2e3d4..56de2cd6956 100644 --- a/pkg/workflow/custom_job_condition_test.go +++ b/pkg/workflow/custom_job_condition_test.go @@ -213,13 +213,13 @@ func TestGetCustomJobsDependingOnPreActivation(t *testing.T) { t.Run(tt.name, func(t *testing.T) { result := c.getCustomJobsDependingOnPreActivation(tt.customJobs) // Convert to maps for easier comparison (order doesn't matter) - resultMap := make(map[string]bool) + resultMap := make(map[string]struct{}) for _, r := range result { - resultMap[r] = true + resultMap[r] = struct{}{} } - expectedMap := make(map[string]bool) + expectedMap := make(map[string]struct{}) for _, e := range tt.expected { - expectedMap[e] = true + expectedMap[e] = struct{}{} } if len(resultMap) != len(expectedMap) { @@ -227,7 +227,7 @@ func TestGetCustomJobsDependingOnPreActivation(t *testing.T) { return } for k := range expectedMap { - if !resultMap[k] { + if _, ok := resultMap[k]; !ok { t.Errorf("getCustomJobsDependingOnPreActivation() returned %v, want %v", result, tt.expected) return } diff --git a/pkg/workflow/domains_blocked_test.go b/pkg/workflow/domains_blocked_test.go index 25e4d756cd6..6772038c2c8 100644 --- a/pkg/workflow/domains_blocked_test.go +++ b/pkg/workflow/domains_blocked_test.go @@ -62,11 +62,11 @@ func TestGetBlockedDomains(t *testing.T) { }, expected: func() []string { // Get python ecosystem domains and add custom domain - domainMap := make(map[string]bool) + domainMap := make(map[string]struct{}) for _, d := range getEcosystemDomains("python") { - domainMap[d] = true + domainMap[d] = struct{}{} } - domainMap["tracker.example.com"] = true + domainMap["tracker.example.com"] = struct{}{} domains := make([]string, 0, len(domainMap)) for d := range domainMap { diff --git a/pkg/workflow/domains_test.go b/pkg/workflow/domains_test.go index 46b4ca35e18..b11f52ef50d 100644 --- a/pkg/workflow/domains_test.go +++ b/pkg/workflow/domains_test.go @@ -1295,17 +1295,17 @@ func TestMergeAPITargetDomains(t *testing.T) { t.Run(tt.name, func(t *testing.T) { result := mergeAPITargetDomains(tt.domainsStr, tt.apiTarget) domains := strings.Split(result, ",") - domainSet := make(map[string]bool) + domainSet := make(map[string]struct{}) for _, d := range domains { - domainSet[d] = true + domainSet[d] = struct{}{} } for _, want := range tt.wantIn { - if !domainSet[want] { + if _, ok := domainSet[want]; !ok { t.Errorf("Expected domain %q in result %q, but not found", want, result) } } for _, notWant := range tt.wantNotIn { - if domainSet[notWant] { + if _, ok := domainSet[notWant]; ok { t.Errorf("Did not expect domain %q in result %q", notWant, result) } } diff --git a/pkg/workflow/ecosystem_domains_test.go b/pkg/workflow/ecosystem_domains_test.go index d31ff3f0cbd..c6fa23104b3 100644 --- a/pkg/workflow/ecosystem_domains_test.go +++ b/pkg/workflow/ecosystem_domains_test.go @@ -492,13 +492,13 @@ func TestEcosystemDomainsUniqueness(t *testing.T) { for _, category := range ecosystemCategories { t.Run("getEcosystemDomains_"+category+"_uniqueness", func(t *testing.T) { domains := getEcosystemDomains(category) - seen := make(map[string]bool) + seen := make(map[string]struct{}) for _, domain := range domains { - if seen[domain] { + if _, ok := seen[domain]; ok { t.Errorf("getEcosystemDomains(%q) returned duplicate domain: %s", category, domain) } - seen[domain] = true + seen[domain] = struct{}{} } }) } diff --git a/pkg/workflow/expression_extraction_fuzz_test.go b/pkg/workflow/expression_extraction_fuzz_test.go index 30cbc4a1ab6..06e00c7feec 100644 --- a/pkg/workflow/expression_extraction_fuzz_test.go +++ b/pkg/workflow/expression_extraction_fuzz_test.go @@ -82,12 +82,12 @@ func FuzzExtractTerminalSubExpressions(f *testing.F) { } // No duplicate tokens. - seen := make(map[string]bool, len(result)) + seen := make(map[string]struct{}, len(result)) for _, tok := range result { - if seen[tok] { + if _, ok := seen[tok]; ok { t.Errorf("extractTerminalSubExpressions(%q) returned duplicate token %q", expr, tok) } - seen[tok] = true + seen[tok] = struct{}{} } }) } @@ -269,7 +269,7 @@ func FuzzExtractExpressions(f *testing.F) { } // Every mapping must have non-empty fields and a valid env var. - envVarSeen := make(map[string]bool, len(mappings)) + envVarSeen := make(map[string]struct{}, len(mappings)) for _, m := range mappings { if m.Original == "" { t.Errorf("ExtractExpressions(%q) returned mapping with empty Original", markdown) @@ -286,10 +286,10 @@ func FuzzExtractExpressions(f *testing.F) { if m.EnvVar != strings.ToUpper(m.EnvVar) { t.Errorf("ExtractExpressions(%q): EnvVar %q is not uppercase", markdown, m.EnvVar) } - if envVarSeen[m.EnvVar] { + if _, ok := envVarSeen[m.EnvVar]; ok { t.Errorf("ExtractExpressions(%q): duplicate EnvVar %q", markdown, m.EnvVar) } - envVarSeen[m.EnvVar] = true + envVarSeen[m.EnvVar] = struct{}{} } // For each compound mapping, every qualifying leaf sub-expression must have diff --git a/pkg/workflow/git_commands_test.go b/pkg/workflow/git_commands_test.go index bc1f8578735..61cc627db54 100644 --- a/pkg/workflow/git_commands_test.go +++ b/pkg/workflow/git_commands_test.go @@ -206,14 +206,14 @@ func TestAdditionalClaudeToolsForSafeOutputs(t *testing.T) { } // Check if we have the expected editing tools - foundEditingTools := make(map[string]bool) + foundEditingTools := make(map[string]struct{}) hasWriteTool := false for _, tool := range resultTools { tool = strings.TrimSpace(tool) for _, expectedTool := range expectedEditingTools { if tool == expectedTool { - foundEditingTools[expectedTool] = true + foundEditingTools[expectedTool] = struct{}{} } } if tool == expectedWriteTool { @@ -231,7 +231,7 @@ func TestAdditionalClaudeToolsForSafeOutputs(t *testing.T) { // Only check if we started with empty tools - if there were pre-existing tools, they should remain if len(tt.tools) == 0 { for _, tool := range expectedEditingTools { - if foundEditingTools[tool] { + if _, ok := foundEditingTools[tool]; ok { t.Errorf("Unexpected editing tool %s found when not expected", tool) } } @@ -241,7 +241,7 @@ func TestAdditionalClaudeToolsForSafeOutputs(t *testing.T) { // Check that all expected editing tools are present (not including Write, which is handled separately) for _, expectedTool := range expectedEditingTools { - if !foundEditingTools[expectedTool] { + if _, ok := foundEditingTools[expectedTool]; !ok { t.Errorf("Expected editing tool %s to be present", expectedTool) } } diff --git a/pkg/workflow/github_toolsets_test.go b/pkg/workflow/github_toolsets_test.go index 8fdb9ca433d..fbf0b2ca547 100644 --- a/pkg/workflow/github_toolsets_test.go +++ b/pkg/workflow/github_toolsets_test.go @@ -118,12 +118,12 @@ func TestParseGitHubToolsets(t *testing.T) { t.Errorf("Expected %d toolsets for 'all', got %d: %v", expectedCount, len(result), result) } // Verify excluded toolsets are not present - resultMap := make(map[string]bool) + resultMap := make(map[string]struct{}) for _, ts := range result { - resultMap[ts] = true + resultMap[ts] = struct{}{} } for _, ex := range GitHubToolsetsExcludedFromAll { - if resultMap[ex] { + if _, ok := resultMap[ex]; ok { t.Errorf("Excluded toolset %q should not be present in 'all' expansion", ex) } } @@ -136,13 +136,13 @@ func TestParseGitHubToolsets(t *testing.T) { } // Check that all expected toolsets are present (order doesn't matter for some tests) - resultMap := make(map[string]bool) + resultMap := make(map[string]struct{}) for _, ts := range result { - resultMap[ts] = true + resultMap[ts] = struct{}{} } for _, expected := range tt.expected { - if !resultMap[expected] { + if _, ok := resultMap[expected]; !ok { t.Errorf("Expected toolset %s not found in result: %v", expected, result) } } @@ -198,12 +198,12 @@ func TestParseGitHubToolsetsDeduplication(t *testing.T) { } // Verify no duplicates - seen := make(map[string]bool) + seen := make(map[string]struct{}) for _, toolset := range result { - if seen[toolset] { + if _, ok := seen[toolset]; ok { t.Errorf("Found duplicate toolset: %s", toolset) } - seen[toolset] = true + seen[toolset] = struct{}{} } }) } diff --git a/pkg/workflow/known_needs_expressions_test.go b/pkg/workflow/known_needs_expressions_test.go index da34ef60371..8a80fb7466c 100644 --- a/pkg/workflow/known_needs_expressions_test.go +++ b/pkg/workflow/known_needs_expressions_test.go @@ -583,17 +583,17 @@ func TestFilterExpressionsForActivation(t *testing.T) { t.Run(tt.name, func(t *testing.T) { result := filterExpressionsForActivation(tt.mappings, tt.customJobs, tt.beforeActivationJobs) - resultSet := make(map[string]bool, len(result)) + resultSet := make(map[string]struct{}, len(result)) for _, m := range result { - resultSet[m.Content] = true + resultSet[m.Content] = struct{}{} } for _, expected := range tt.expectedContents { - assert.True(t, resultSet[expected], + assert.Contains(t, resultSet, expected, "Expected expression %q to be kept in result, got: %v", expected, resultSet) } for _, excluded := range tt.excludedContents { - assert.False(t, resultSet[excluded], + assert.NotContains(t, resultSet, excluded, "Expected expression %q to be filtered out of result, got: %v", excluded, resultSet) } }) diff --git a/pkg/workflow/maintenance_workflow_test.go b/pkg/workflow/maintenance_workflow_test.go index b10331bfdcb..4aab2637675 100644 --- a/pkg/workflow/maintenance_workflow_test.go +++ b/pkg/workflow/maintenance_workflow_test.go @@ -1784,12 +1784,12 @@ func TestCollectSideRepoTargets(t *testing.T) { return } // Use a set-based comparison so the test is not sensitive to ordering. - gotSet := make(map[string]bool, len(got)) + gotSet := make(map[string]struct{}, len(got)) for _, r := range got { - gotSet[r] = true + gotSet[r] = struct{}{} } for _, repo := range tt.expectedRepos { - if !gotSet[repo] { + if _, ok := gotSet[repo]; !ok { t.Errorf("expected target %q not found in results %v", repo, got) } } diff --git a/pkg/workflow/tools_types_test.go b/pkg/workflow/tools_types_test.go index ada0c9b98a7..fa5218849b8 100644 --- a/pkg/workflow/tools_types_test.go +++ b/pkg/workflow/tools_types_test.go @@ -170,23 +170,19 @@ func TestGetToolNames(t *testing.T) { } // Check that all expected names are present - expectedNames := map[string]bool{ - "github": false, - "bash": false, - "edit": false, - "my-custom": false, + expectedNames := map[string]struct{}{ + "github": {}, + "bash": {}, + "edit": {}, + "my-custom": {}, } for _, name := range names { - if _, ok := expectedNames[name]; ok { - expectedNames[name] = true - } + delete(expectedNames, name) } - for name, found := range expectedNames { - if !found { - t.Errorf("expected to find tool %q in names list", name) - } + for name := range expectedNames { + t.Errorf("expected to find tool %q in names list", name) } })