diff --git a/cmd/gh-aw/main_help_text_test.go b/cmd/gh-aw/main_help_text_test.go index 8f1aac0e641..7c9cc5031f6 100644 --- a/cmd/gh-aw/main_help_text_test.go +++ b/cmd/gh-aw/main_help_text_test.go @@ -77,7 +77,7 @@ func TestCompileGhAwRefMutuallyExclusiveFlags(t *testing.T) { err := rootCmd.Execute() require.Error(t, err) - assert.Contains(t, err.Error(), "if any flags in the group", "expected mutually exclusive flag-group error") + require.ErrorContains(t, err, "if any flags in the group", "expected mutually exclusive flag-group error") }) } } diff --git a/docs/adr/47252-testify-error-assertion-conventions.md b/docs/adr/47252-testify-error-assertion-conventions.md new file mode 100644 index 00000000000..cfd8c20f570 --- /dev/null +++ b/docs/adr/47252-testify-error-assertion-conventions.md @@ -0,0 +1,55 @@ +# ADR-47252: Testify Error-Assertion Conventions — ErrorContains and require.* + +**Date**: 2026-07-22 +**Status**: Draft +**Deciders**: pelikhan (PR author), go-fan module review recommendations + +--- + +### Context + +The Go test suite used `assert.Contains(t, err.Error(), "…")` as a widespread pattern for checking error message substrings. This pattern has a latent defect: if `err` is `nil`, calling `.Error()` on it panics and crashes the entire test binary rather than recording a clean test failure. Separately, several error assertions used `assert.*` even when preceded by a `require.Error` precondition — meaning a failure in the preceding check would still allow subsequent assertions to execute, leading to confusing secondary failures. `.golangci.yml` already enables `testifylint` with `enable-all: true`, which flags both of these patterns. + +### Decision + +We will standardise on two rules for error assertions in Go tests: + +1. **Use `ErrorContains(t, err, "…")` instead of `assert.Contains(t, err.Error(), "…")`** — `ErrorContains` validates that `err != nil` before inspecting the message, eliminating the nil-deref crash risk. +2. **Use `require.*` error assertions (not `assert.*`) after a `require.Error` precondition, and for any assertion whose failure would invalidate immediately following code** — `require` stops the test on first failure rather than allowing cascading false positives. + +This decision is enforced mechanically across all `*_test.go` files; no production code was changed. + +### Alternatives Considered + +#### Alternative 1: Guard each call site with an explicit nil check + +Add `if err != nil { assert.Contains(t, err.Error(), "…") }` before every affected assertion. This eliminates the panic without changing the assertion library API but is highly verbose (600+ sites), increases maintenance burden, and still does not address the `assert` vs `require` semantics issue. + +#### Alternative 2: Keep `assert.Contains(t, err.Error(), "…")` and rely on test author discipline + +Accept the current pattern and document that callers must ensure `err != nil` before calling `.Error()`. This has zero migration cost but preserves the crash risk and contradicts the existing `testifylint enable-all: true` policy, which was already configured to flag these patterns. + +#### Alternative 3: Wrap error assertions in a custom helper function + +Introduce a project-local `assertErrorContains(t, err, substr)` helper that handles the nil check internally. This is technically valid but duplicates functionality already provided by `testify` (`ErrorContains` was added in testify v1.7.1), adds an unnecessary abstraction, and reduces newcomer familiarity with the standard library. + +### Consequences + +#### Positive +- Nil-deref crash risk in error assertion paths is eliminated; a nil `err` now produces a clean `FAIL` line rather than a test binary crash. +- Test failures are reported at the first meaningful assertion (`require` stops execution), removing cascading false positives that obscure root causes. +- The codebase is now fully compliant with `testifylint enable-all: true`; no suppression flags or `//nolint` directives are needed. +- The pattern is idiomatic and familiar to any Go developer using testify — no custom DSL to learn. + +#### Negative +- This is a 600+ file, 624-line mechanical change. Even though it is purely additive/substitutive in `*_test.go` files, the large diff creates merge conflicts for any concurrent branches that touch the same test files. +- Once merged, any contributor who reverts to the old `assert.Contains(t, err.Error(), "…")` pattern will see a CI lint failure — the stricter convention is now enforced by CI, not just recommended. + +#### Neutral +- No change to test behaviour for passing tests; only the failure mode changes. +- No CI configuration changes required (`testifylint enable-all: true` was already present). +- Five files had their `assert` import removed as a side effect of the migration; unrelated `assert.*` calls in those files were also migrated as part of the sweep. + +--- + +*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* diff --git a/pkg/actionpins/spec_test.go b/pkg/actionpins/spec_test.go index 8212909a906..df78a7cdc8d 100644 --- a/pkg/actionpins/spec_test.go +++ b/pkg/actionpins/spec_test.go @@ -322,7 +322,7 @@ func TestSpec_PublicAPI_ResolveActionPin_EnforcePinned(t *testing.T) { result, err := actionpins.ResolveActionPin("does-not-exist/x", "v1", ctx) if tt.wantErr { require.Error(t, err, "enforce mode should return an error for this scenario") - assert.Contains(t, err.Error(), tt.wantErrContains) + require.ErrorContains(t, err, tt.wantErrContains) assert.Empty(t, result, "erroring enforce mode should not return a pinned reference") } else { require.NoError(t, err, "non-error scenario should not return an error") @@ -895,7 +895,7 @@ func TestSpec_PublicAPI_ResolveActionPin_MappingTargetUnknown(t *testing.T) { require.NotPanics(t, func() { result, err := actionpins.ResolveActionPin("actions/checkout", "v4", ctx) - require.NoError(t, err) + assert.NoError(t, err) assert.Empty(t, result, "mapping to unknown repo should produce unresolved empty result") }) } diff --git a/pkg/agentdrain/anomaly_test.go b/pkg/agentdrain/anomaly_test.go index 7bfbe99cb3a..bc7231140fa 100644 --- a/pkg/agentdrain/anomaly_test.go +++ b/pkg/agentdrain/anomaly_test.go @@ -299,7 +299,7 @@ func TestNewAnomalyDetector_ThresholdBoundaries(t *testing.T) { detector, err := NewAnomalyDetector(tt.simThreshold, tt.rareThreshold) if tt.wantErr != "" { require.Error(t, err, "NewAnomalyDetector should reject invalid thresholds") - assert.Contains(t, err.Error(), tt.wantErr, "error should describe invalid threshold") + require.ErrorContains(t, err, tt.wantErr, "error should describe invalid threshold") require.Nil(t, detector, "NewAnomalyDetector should return nil detector on validation error") return } @@ -529,7 +529,7 @@ func TestAnalyzeEvent_Variants(t *testing.T) { result, report, err := m.AnalyzeEvent(tt.evt) if tt.wantErr { require.Error(t, err, "AnalyzeEvent should return an error") - assert.Contains(t, err.Error(), tt.wantErrMsg, "error message mismatch") + require.ErrorContains(t, err, tt.wantErrMsg, "error message mismatch") assert.Nil(t, result, "AnalyzeEvent should return nil result on error") assert.Nil(t, report, "AnalyzeEvent should return nil report on error") return diff --git a/pkg/agentdrain/miner_test.go b/pkg/agentdrain/miner_test.go index f75f350cf26..e5fc3d7978e 100644 --- a/pkg/agentdrain/miner_test.go +++ b/pkg/agentdrain/miner_test.go @@ -264,7 +264,7 @@ func TestTrainEmptyLine(t *testing.T) { result, err := m.Train(" \t\n ") assert.Nil(t, result, "Train should return nil result for whitespace-only input") require.Error(t, err, "Train should return an error for whitespace-only input") - assert.Contains(t, err.Error(), "empty line after masking", "Train error should explain empty line after masking") + require.ErrorContains(t, err, "empty line after masking", "Train error should explain empty line after masking") } func TestNewMaskerInvalidPattern(t *testing.T) { @@ -278,7 +278,7 @@ func TestNewMaskerInvalidPattern(t *testing.T) { assert.Nil(t, masker, "NewMasker should return nil masker for invalid regex pattern") require.Error(t, err, "NewMasker should fail when a regex pattern is invalid") - assert.Contains(t, err.Error(), `mask rule "invalid"`, "NewMasker error should identify the failing rule") + require.ErrorContains(t, err, `mask rule "invalid"`, "NewMasker error should identify the failing rule") } func TestConcurrency(t *testing.T) { diff --git a/pkg/cli/actions_build_command_test.go b/pkg/cli/actions_build_command_test.go index b530af855df..c5658a3c3ff 100644 --- a/pkg/cli/actions_build_command_test.go +++ b/pkg/cli/actions_build_command_test.go @@ -24,7 +24,7 @@ func TestActionsBuildCommand_NoActionsDir(t *testing.T) { // Test with non-existent actions directory err = ActionsBuildCommand() require.Error(t, err, "Should error when actions/ directory does not exist") - assert.Contains(t, err.Error(), "actions/ directory does not exist", "Error should mention missing directory") + require.ErrorContains(t, err, "actions/ directory does not exist", "Error should mention missing directory") } func TestActionsValidateCommand_NoActionsDir(t *testing.T) { @@ -233,7 +233,7 @@ runs: if tt.expectError { require.Error(t, err, "Expected an error") if tt.errorContains != "" { - assert.Contains(t, err.Error(), tt.errorContains, "Error should contain expected message") + require.ErrorContains(t, err, tt.errorContains, "Error should contain expected message") } } else { require.NoError(t, err, "Should not error for valid action.yml") diff --git a/pkg/cli/add_command_test.go b/pkg/cli/add_command_test.go index 76b5484d730..ee6d183faa3 100644 --- a/pkg/cli/add_command_test.go +++ b/pkg/cli/add_command_test.go @@ -130,7 +130,7 @@ func TestAddWorkflows(t *testing.T) { if tt.expectError { require.Error(t, err, "Expected error for test case: %s", tt.name) if tt.errorContains != "" { - assert.Contains(t, err.Error(), tt.errorContains, "Error should contain expected message") + require.ErrorContains(t, err, tt.errorContains, "Error should contain expected message") } } else { assert.NoError(t, err, "Should not error for test case: %s", tt.name) @@ -381,14 +381,14 @@ func TestRejectBootstrapProfileForRegularAdd(t *testing.T) { t.Run("rejects regular add for packages with manifest config", func(t *testing.T) { err := rejectBootstrapProfileForRegularAdd([]string{"githubnext/central-agentic-ops"}, profileWithConfig) require.Error(t, err) - assert.Contains(t, err.Error(), "package githubnext/central-agentic-ops declares aw.yml config") - assert.Contains(t, err.Error(), "gh aw add-wizard githubnext/central-agentic-ops") + require.ErrorContains(t, err, "package githubnext/central-agentic-ops declares aw.yml config") + require.ErrorContains(t, err, "gh aw add-wizard githubnext/central-agentic-ops") }) t.Run("uses requested sources in the add-wizard guidance", func(t *testing.T) { err := rejectBootstrapProfileForRegularAdd([]string{"githubnext/central-agentic-ops", "./local-workflow.md"}, profileWithConfig) require.Error(t, err) - assert.Contains(t, err.Error(), "gh aw add-wizard githubnext/central-agentic-ops ./local-workflow.md") + require.ErrorContains(t, err, "gh aw add-wizard githubnext/central-agentic-ops ./local-workflow.md") }) t.Run("allows packages without manifest config", func(t *testing.T) { @@ -558,7 +558,7 @@ source: octo/other/.github/workflows/dependabot.md@main skip, err := validateWorkflowDestination(workflowsDir, "dependabot", "githubnext/central-agentic-ops", AddOptions{}) require.Error(t, err) assert.False(t, skip) - assert.Contains(t, err.Error(), "workflow 'dependabot' already exists") + require.ErrorContains(t, err, "workflow 'dependabot' already exists") } // TestAddMultipleWorkflowsNameFlag verifies that --name is not allowed when multiple workflows are specified. @@ -570,7 +570,7 @@ func TestAddMultipleWorkflowsNameFlag(t *testing.T) { err := cmd.Execute() require.Error(t, err, "Should error when --name is used with multiple workflows") - assert.Contains(t, err.Error(), "--name flag cannot be used when adding multiple workflows", "Error should mention --name restriction") + require.ErrorContains(t, err, "--name flag cannot be used when adding multiple workflows", "Error should mention --name restriction") } // setupMinimalGitRepo initialises a bare-minimum git repo in dir and returns the @@ -817,7 +817,7 @@ func TestAddWorkflowWithTracking_ActionWorkflow_Force(t *testing.T) { // Without --force: should fail err := addWorkflowWithTracking(context.Background(), resolved, nil, AddOptions{}) require.Error(t, err) - assert.Contains(t, err.Error(), "already exists") + require.ErrorContains(t, err, "already exists") // With --force: should overwrite err = addWorkflowWithTracking(context.Background(), resolved, nil, AddOptions{Force: true}) @@ -863,7 +863,7 @@ func TestAddWorkflowsWithTracking_RollsBackWrittenFilesOnWriteFailure(t *testing Quiet: true, }) require.Error(t, err) - assert.Contains(t, err.Error(), "failed to write destination file") + require.ErrorContains(t, err, "failed to write destination file") _, statErr := os.Stat(filepath.Join(workflowsDir, "ok.md")) assert.True(t, os.IsNotExist(statErr), "successful writes from this operation should be rolled back on later write failure") @@ -911,7 +911,7 @@ func TestAddSkillFileWithTracking_RejectsInvalidPaths(t *testing.T) { err := addSkillFileWithTracking(resolved, nil, AddOptions{Quiet: true}, gitRoot) require.Error(t, err) - assert.Contains(t, err.Error(), "escapes destination skill directory") + require.ErrorContains(t, err, "escapes destination skill directory") }) t.Run("rejects source path when skill root cannot be determined", func(t *testing.T) { @@ -926,7 +926,7 @@ func TestAddSkillFileWithTracking_RejectsInvalidPaths(t *testing.T) { err := addSkillFileWithTracking(resolved, nil, AddOptions{Quiet: true}, gitRoot) require.Error(t, err) - assert.Contains(t, err.Error(), "failed to determine relative path") + require.ErrorContains(t, err, "failed to determine relative path") }) } @@ -959,7 +959,7 @@ func TestAddCopilotRequestsPermissionToContent(t *testing.T) { content := "---\nengine: copilot\npermissions: read-all\n---\nDo the thing.\n" _, err := addCopilotRequestsPermissionToContent(content) require.Error(t, err) - assert.Contains(t, err.Error(), "non-mapping scalar") + require.ErrorContains(t, err, "non-mapping scalar") }) } diff --git a/pkg/cli/add_package_manifest_test.go b/pkg/cli/add_package_manifest_test.go index ef82ee215dd..a0ff5fd6696 100644 --- a/pkg/cli/add_package_manifest_test.go +++ b/pkg/cli/add_package_manifest_test.go @@ -328,7 +328,7 @@ files: _, err := resolveRepositoryPackage(t.Context(), &RepoSpec{RepoSlug: "owner/repo"}, "") require.Error(t, err) - assert.Contains(t, err.Error(), `name must be a non-empty string`) + require.ErrorContains(t, err, `name must be a non-empty string`) }) t.Run("requires aw manifest when only legacy alias exists", func(t *testing.T) { @@ -348,7 +348,7 @@ files: _, err := resolveRepositoryPackage(t.Context(), &RepoSpec{RepoSlug: "owner/repo"}, "") require.Error(t, err) assert.Equal(t, []string{"aw.yml"}, requestedPaths) - assert.Contains(t, err.Error(), `no aw.yml manifest found`) + require.ErrorContains(t, err, `no aw.yml manifest found`) }) t.Run("accepts manifest-version and compatible min-version", func(t *testing.T) { @@ -444,7 +444,7 @@ name: Repo Assist _, err := resolveRepositoryPackage(t.Context(), &RepoSpec{RepoSlug: "owner/repo"}, "") require.Error(t, err) - assert.Contains(t, err.Error(), `manifest-version`) + require.ErrorContains(t, err, `manifest-version`) }) t.Run("accepts branding field", func(t *testing.T) { @@ -539,7 +539,7 @@ bootstrap: _, err := resolveRepositoryPackage(t.Context(), &RepoSpec{RepoSlug: "owner/repo"}, "") require.Error(t, err, "old bootstrap key must produce an error, not be silently ignored") - assert.Contains(t, err.Error(), "bootstrap") + require.ErrorContains(t, err, "bootstrap") }) t.Run("rejects unsupported branding icon", func(t *testing.T) { @@ -556,7 +556,7 @@ branding: _, err := resolveRepositoryPackage(t.Context(), &RepoSpec{RepoSlug: "owner/repo"}, "") require.Error(t, err) - assert.Contains(t, err.Error(), `icon`) + require.ErrorContains(t, err, `icon`) }) t.Run("rejects docs field", func(t *testing.T) { @@ -571,7 +571,7 @@ docs: docs/overview.md _, err := resolveRepositoryPackage(t.Context(), &RepoSpec{RepoSlug: "owner/repo"}, "") require.Error(t, err) - assert.Contains(t, err.Error(), `docs`) + require.ErrorContains(t, err, `docs`) }) t.Run("rejects non-string emoji field", func(t *testing.T) { @@ -587,7 +587,7 @@ emoji: _, err := resolveRepositoryPackage(t.Context(), &RepoSpec{RepoSlug: "owner/repo"}, "") require.Error(t, err) - assert.Contains(t, err.Error(), `emoji`) + require.ErrorContains(t, err, `emoji`) }) t.Run("rejects non-string license field", func(t *testing.T) { @@ -603,7 +603,7 @@ license: _, err := resolveRepositoryPackage(t.Context(), &RepoSpec{RepoSlug: "owner/repo"}, "") require.Error(t, err) - assert.Contains(t, err.Error(), `license`) + require.ErrorContains(t, err, `license`) }) t.Run("rejects incompatible min-version", func(t *testing.T) { @@ -618,7 +618,7 @@ name: Repo Assist _, err := resolveRepositoryPackage(t.Context(), &RepoSpec{RepoSlug: "owner/repo"}, "") require.Error(t, err) - assert.Contains(t, err.Error(), `requires gh-aw`) + require.ErrorContains(t, err, `requires gh-aw`) }) t.Run("requires package README", func(t *testing.T) { @@ -638,7 +638,7 @@ files: _, err := resolveRepositoryPackage(t.Context(), &RepoSpec{RepoSlug: "owner/repo"}, "") require.Error(t, err) - assert.Contains(t, err.Error(), `missing required README.md`) + require.ErrorContains(t, err, `missing required README.md`) }) t.Run("reports nested package path when README is missing", func(t *testing.T) { @@ -658,8 +658,8 @@ files: _, err := resolveRepositoryPackage(t.Context(), &RepoSpec{RepoSlug: "owner/repo", PackagePath: "packages/repo-assist"}, "") require.Error(t, err) - assert.Contains(t, err.Error(), `owner/repo/packages/repo-assist`) - assert.Contains(t, err.Error(), `packages/repo-assist/README.md`) + require.ErrorContains(t, err, `owner/repo/packages/repo-assist`) + require.ErrorContains(t, err, `packages/repo-assist/README.md`) }) t.Run("rejects unknown manifest fields", func(t *testing.T) { @@ -674,7 +674,7 @@ unknown-field: true _, err := resolveRepositoryPackage(t.Context(), &RepoSpec{RepoSlug: "owner/repo"}, "") require.Error(t, err) - assert.Contains(t, err.Error(), `unknown-field`) + require.ErrorContains(t, err, `unknown-field`) }) t.Run("resolves nested package manifests", func(t *testing.T) { @@ -845,7 +845,7 @@ func TestResolveWorkflows_RepositoryPackageRejectsPrivateTrue(t *testing.T) { _, err := ResolveWorkflows(context.Background(), []string{"owner/repo"}, false) require.Error(t, err) - assert.Contains(t, err.Error(), `workflow "workflows/review.md" sets private: true`) + require.ErrorContains(t, err, `workflow "workflows/review.md" sets private: true`) } func TestResolveWorkflows_NestedRepositoryPackage(t *testing.T) { @@ -1145,7 +1145,7 @@ func TestParseRepositoryPackageSpec(t *testing.T) { assert.Equal(t, tt.wantOK, ok) if tt.wantErr != "" { require.Error(t, err) - assert.Contains(t, err.Error(), tt.wantErr) + require.ErrorContains(t, err, tt.wantErr) return } require.NoError(t, err) @@ -1332,7 +1332,7 @@ files: _, err := resolveRepositoryPackage(t.Context(), &RepoSpec{RepoSlug: "owner/repo"}, "") require.Error(t, err) - assert.Contains(t, err.Error(), "duplicate workflow filename") + require.ErrorContains(t, err, "duplicate workflow filename") }) } diff --git a/pkg/cli/add_wildcard_test.go b/pkg/cli/add_wildcard_test.go index bb4805b3179..6bf4a89e8d7 100644 --- a/pkg/cli/add_wildcard_test.go +++ b/pkg/cli/add_wildcard_test.go @@ -266,7 +266,7 @@ func TestExpandLocalWildcardWorkflows_NoMatches(t *testing.T) { _, err = expandLocalWildcardWorkflows(specs, false) // Should error because no workflows found after expansion require.Error(t, err, "Should error when no workflows match") - assert.Contains(t, err.Error(), "no workflows to add after expansion") + require.ErrorContains(t, err, "no workflows to add after expansion") } // TestAddWorkflowWithTracking_WildcardDuplicateHandling tests that when adding workflows from wildcard, diff --git a/pkg/cli/add_workflow_resolution_redirect_test.go b/pkg/cli/add_workflow_resolution_redirect_test.go index 3ce9504094d..a108f47812b 100644 --- a/pkg/cli/add_workflow_resolution_redirect_test.go +++ b/pkg/cli/add_workflow_resolution_redirect_test.go @@ -71,7 +71,7 @@ func TestResolveAddWorkflowSpecAndContent(t *testing.T) { WorkflowName: "a", }, false) require.Error(t, err, "redirect loop should fail") - assert.Contains(t, err.Error(), "redirect loop detected", "error should mention loop detection") + require.ErrorContains(t, err, "redirect loop detected", "error should mention loop detection") }) t.Run("local workflows are not redirected", func(t *testing.T) { diff --git a/pkg/cli/audit_test.go b/pkg/cli/audit_test.go index c17ca38cba4..4dc0a88a679 100644 --- a/pkg/cli/audit_test.go +++ b/pkg/cli/audit_test.go @@ -1159,7 +1159,7 @@ func TestRunAuditMulti_Validation(t *testing.T) { t.Run(tt.name, func(t *testing.T) { err := runAuditMulti(t.Context(), tt.args, "", "", false, false, "pretty", nil) require.Error(t, err, "runAuditMulti should return an error for invalid input") - assert.Contains(t, err.Error(), tt.wantErr, "error message should be descriptive") + require.ErrorContains(t, err, tt.wantErr, "error message should be descriptive") }) } } @@ -1182,7 +1182,7 @@ func TestAuditCommandStdinRejectsPositionalArgs(t *testing.T) { cmd.SetErr(nil) err := cmd.Execute() require.Error(t, err, "audit --stdin with a positional arg should return an error") - assert.Contains(t, err.Error(), "positional arguments are not allowed with --stdin", "error message should explain the conflict") + require.ErrorContains(t, err, "positional arguments are not allowed with --stdin", "error message should explain the conflict") } func TestAuditCommandRequiresArgsOrStdin(t *testing.T) { @@ -1192,7 +1192,7 @@ func TestAuditCommandRequiresArgsOrStdin(t *testing.T) { cmd.SetErr(nil) err := cmd.Execute() require.Error(t, err, "audit with no args and no --stdin should return an error") - assert.Contains(t, err.Error(), "at least one run ID or URL is required", "error message should prompt for required input") + require.ErrorContains(t, err, "at least one run ID or URL is required", "error message should prompt for required input") } func TestAuditCommandVariantWithoutExperiment(t *testing.T) { @@ -1202,7 +1202,7 @@ func TestAuditCommandVariantWithoutExperiment(t *testing.T) { cmd.SetErr(io.Discard) err := cmd.Execute() require.Error(t, err, "--variant without --experiment should return an error") - assert.Contains(t, err.Error(), "--variant requires --experiment", "error message should explain the requirement") + require.ErrorContains(t, err, "--variant requires --experiment", "error message should explain the requirement") } func TestAuditCommandExperimentAndVariantFlagsAreAccepted(t *testing.T) { diff --git a/pkg/cli/codemod_top_level_env_secrets_test.go b/pkg/cli/codemod_top_level_env_secrets_test.go index a1beed975d8..0799b232725 100644 --- a/pkg/cli/codemod_top_level_env_secrets_test.go +++ b/pkg/cli/codemod_top_level_env_secrets_test.go @@ -32,10 +32,20 @@ env: _, applied, err := codemod.Apply(content, frontmatter) require.Error(t, err, "should return an error for top-level env secrets") assert.False(t, applied, "should not modify the file") - assert.Contains(t, err.Error(), "top-level env: contains secrets") - assert.Contains(t, err.Error(), "${{ secrets.GITHUB_TOKEN }}") - assert.Contains(t, err.Error(), "Manual fix required") - assert.Contains(t, err.Error(), "https://github.github.com/gh-aw/reference/engines/") + codemodErr := err + for _, tc := range []struct { + name string + msg string + }{ + {name: "contains_secrets_message", msg: "top-level env: contains secrets"}, + {name: "github_token_reference", msg: "${{ secrets.GITHUB_TOKEN }}"}, + {name: "manual_fix_guidance", msg: "Manual fix required"}, + {name: "documentation_link", msg: "https://github.github.com/gh-aw/reference/engines/"}, + } { + t.Run(tc.name, func(t *testing.T) { + require.ErrorContains(t, codemodErr, tc.msg) + }) + } }) t.Run("returns deduplicated guided error with multiple secret references", func(t *testing.T) { @@ -57,7 +67,7 @@ env: _, applied, err := codemod.Apply(content, frontmatter) require.Error(t, err) assert.False(t, applied) - assert.Contains(t, err.Error(), "top-level env: contains secrets") + require.ErrorContains(t, err, "top-level env: contains secrets") assert.Equal(t, 1, strings.Count(err.Error(), "${{ secrets.GITHUB_PERSONAL_ACCESS_TOKEN || secrets.GITHUB_TOKEN }}")) }) diff --git a/pkg/cli/compile_args_test.go b/pkg/cli/compile_args_test.go index 09100c694c1..647f4695c2a 100644 --- a/pkg/cli/compile_args_test.go +++ b/pkg/cli/compile_args_test.go @@ -42,7 +42,7 @@ func TestExpandCompileArg_LocalDirectory_Empty(t *testing.T) { tmpDir := t.TempDir() _, err := expandCompileArg(tmpDir, false) require.Error(t, err, "empty directory should return an error") - assert.Contains(t, err.Error(), "no workflow markdown files found", "error should mention no workflow files") + require.ErrorContains(t, err, "no workflow markdown files found", "error should mention no workflow files") } func TestExpandCompileArg_URLPassthrough(t *testing.T) { diff --git a/pkg/cli/compile_guard_policy_test.go b/pkg/cli/compile_guard_policy_test.go index 4fd3a96fcce..ed65e0a4053 100644 --- a/pkg/cli/compile_guard_policy_test.go +++ b/pkg/cli/compile_guard_policy_test.go @@ -139,7 +139,7 @@ This workflow specifies repos without min-integrity. if tt.expectError { require.Error(t, err, "Expected compilation to fail") if tt.errorContains != "" { - assert.Contains(t, err.Error(), tt.errorContains, "Error should mention %q", tt.errorContains) + require.ErrorContains(t, err, tt.errorContains, "Error should mention %q", tt.errorContains) } } else { assert.NoError(t, err, "Expected compilation to succeed") @@ -468,7 +468,7 @@ This workflow sets trusted-users without min-integrity (should fail). compiler := workflow.NewCompiler() err = CompileWorkflowWithValidation(context.Background(), compiler, workflowPath, CompileValidationOptions{}) require.Error(t, err, "Expected compilation to fail without min-integrity") - assert.Contains(t, err.Error(), "min-integrity", "Error should mention min-integrity requirement") + require.ErrorContains(t, err, "min-integrity", "Error should mention min-integrity requirement") } // TestGuardPolicyToolCallLimitsCompilation verifies that max-calls entries under @@ -534,5 +534,5 @@ tools: compiler := workflow.NewCompiler() err = CompileWorkflowWithValidation(context.Background(), compiler, workflowPath, CompileValidationOptions{}) require.Error(t, err, "Expected compilation to fail for unknown tool name") - assert.Contains(t, err.Error(), "Unknown GitHub tool(s): issue_read:1", "Compiler must treat colon entry as literal tool name") + require.ErrorContains(t, err, "Unknown GitHub tool(s): issue_read:1", "Compiler must treat colon entry as literal tool name") } diff --git a/pkg/cli/compile_repository_manifest_test.go b/pkg/cli/compile_repository_manifest_test.go index 2549b181e3b..66fe4c61e2d 100644 --- a/pkg/cli/compile_repository_manifest_test.go +++ b/pkg/cli/compile_repository_manifest_test.go @@ -50,7 +50,7 @@ name: Repo Assist _, err = CompileWorkflows(context.Background(), CompileConfig{}) require.Error(t, err) - assert.Contains(t, err.Error(), `requires gh-aw`) + require.ErrorContains(t, err, `requires gh-aw`) } func TestCompileWorkflows_JSONOutputIncludesManifestValidationResult(t *testing.T) { @@ -206,7 +206,7 @@ name: Repo Assist _, err = CompileWorkflows(context.Background(), CompileConfig{}) require.Error(t, err) - assert.Contains(t, err.Error(), "missing required README.md") + require.ErrorContains(t, err, "missing required README.md") } func TestCompileWorkflows_RejectsManifestWorkflowWithPrivateTrue(t *testing.T) { @@ -236,7 +236,7 @@ files: _, err = CompileWorkflows(context.Background(), CompileConfig{}) require.Error(t, err) - assert.Contains(t, err.Error(), `workflow "workflows/review.md" sets private: true`) + require.ErrorContains(t, err, `workflow "workflows/review.md" sets private: true`) } func TestValidateRepositoryManifestForCompilation_PropagatesGitRootErrors(t *testing.T) { @@ -253,6 +253,6 @@ func TestValidateRepositoryManifestForCompilation_PropagatesGitRootErrors(t *tes var results []ValidationResult err := validateRepositoryManifestForCompilation(CompileConfig{}, stats, &results) require.Error(t, err) - assert.Contains(t, err.Error(), "failed to find git root for manifest validation") - assert.Contains(t, err.Error(), "permission denied") + require.ErrorContains(t, err, "failed to find git root for manifest validation") + require.ErrorContains(t, err, "permission denied") } diff --git a/pkg/cli/compile_safe_update_integration_test.go b/pkg/cli/compile_safe_update_integration_test.go index b2f052794f9..3f9b67dd115 100644 --- a/pkg/cli/compile_safe_update_integration_test.go +++ b/pkg/cli/compile_safe_update_integration_test.go @@ -326,7 +326,7 @@ func TestSafeUpdateNoFlagAllowsNewSecret(t *testing.T) { output, err := cmd.CombinedOutput() outputStr := string(output) - assert.NoError(t, err, "compile with strict: false should succeed without safe update warning\nOutput:\n%s", outputStr) + require.NoError(t, err, "compile with strict: false should succeed without safe update warning\nOutput:\n%s", outputStr) assert.False(t, strings.Contains(outputStr, "safe update mode"), "output should not mention safe update mode when strict mode is disabled") t.Logf("Compilation without safe update enforcement succeeded as expected.\nOutput:\n%s", outputStr) diff --git a/pkg/cli/completion_command_test.go b/pkg/cli/completion_command_test.go index bb08e0e1aa1..fb241ec2591 100644 --- a/pkg/cli/completion_command_test.go +++ b/pkg/cli/completion_command_test.go @@ -76,7 +76,7 @@ func TestCompletionCommand_InvalidShell(t *testing.T) { err := rootCmd.Execute() require.Error(t, err) - assert.Contains(t, err.Error(), "invalid argument") + require.ErrorContains(t, err, "invalid argument") } func TestCompletionCommand_NoArgs(t *testing.T) { @@ -88,7 +88,7 @@ func TestCompletionCommand_NoArgs(t *testing.T) { err := rootCmd.Execute() require.Error(t, err) - assert.Contains(t, err.Error(), "accepts 1 arg(s)") + require.ErrorContains(t, err, "accepts 1 arg(s)") } func TestCompletionCommand_InstallSubcommand(t *testing.T) { diff --git a/pkg/cli/deploy_command_test.go b/pkg/cli/deploy_command_test.go index 7bb1c3292ee..6af7f7ccd0a 100644 --- a/pkg/cli/deploy_command_test.go +++ b/pkg/cli/deploy_command_test.go @@ -27,7 +27,7 @@ func TestNewDeployCommand_RequiresWorkflowArg(t *testing.T) { err := cmd.Args(cmd, []string{}) require.Error(t, err) - assert.Contains(t, err.Error(), "missing workflow specification") + require.ErrorContains(t, err, "missing workflow specification") } func TestNewDeployCommand_RegistersCoreFlags(t *testing.T) { @@ -86,7 +86,7 @@ func TestNewDeployCommand_RequiresRepoFlag(t *testing.T) { require.NotNil(t, cmd) err := runDeployCommand(cmd, []string{"githubnext/agentics/ci-doctor"}, func(string) error { return nil }) require.Error(t, err) - assert.Contains(t, err.Error(), "either --repo (owner/repo) or --org must be provided") + require.ErrorContains(t, err, "either --repo (owner/repo) or --org must be provided") } func TestRunDeployCommand_RejectsRepoAndOrgTogether(t *testing.T) { @@ -96,7 +96,7 @@ func TestRunDeployCommand_RejectsRepoAndOrgTogether(t *testing.T) { require.NoError(t, cmd.Flags().Set("org", "octo")) err := runDeployCommand(cmd, []string{"githubnext/agentics/ci-doctor"}, func(string) error { return nil }) require.Error(t, err) - assert.Contains(t, err.Error(), "cannot specify both --repo and --org") + require.ErrorContains(t, err, "cannot specify both --repo and --org") } func TestRunDeployCommand_RequiresOrgWhenReposProvided(t *testing.T) { @@ -106,7 +106,7 @@ func TestRunDeployCommand_RequiresOrgWhenReposProvided(t *testing.T) { err := runDeployCommand(cmd, []string{"githubnext/agentics/ci-doctor"}, func(string) error { return nil }) require.Error(t, err) - assert.Contains(t, err.Error(), "--repos requires --org") + require.ErrorContains(t, err, "--repos requires --org") } func TestRunDeployCommand_RoutesToOrgRunner(t *testing.T) { @@ -275,7 +275,7 @@ func TestParseDeployCommandOptions_NameFlagWithMultipleWorkflows(t *testing.T) { return nil }) require.Error(t, err) - assert.Contains(t, err.Error(), "--name flag cannot be used when adding multiple workflows at once") + require.ErrorContains(t, err, "--name flag cannot be used when adding multiple workflows at once") assert.Equal(t, AddOptions{}, opts) assert.Zero(t, coolDown) assert.False(t, validateEngineCalled) @@ -290,7 +290,7 @@ func TestParseDeployCommandOptions_InvalidCoolDown(t *testing.T) { opts, coolDown, err := parseDeployCommandOptions(cmd, []string{"a"}, func(string) error { return nil }) require.Error(t, err) - assert.Contains(t, err.Error(), "invalid --cool-down value") + require.ErrorContains(t, err, "invalid --cool-down value") assert.Equal(t, AddOptions{}, opts) assert.Zero(t, coolDown) } diff --git a/pkg/cli/deploy_org_test.go b/pkg/cli/deploy_org_test.go index ea1b98e9e6a..ea0bed3a100 100644 --- a/pkg/cli/deploy_org_test.go +++ b/pkg/cli/deploy_org_test.go @@ -15,13 +15,13 @@ import ( func TestRunDeployForOrgEmptyOrg(t *testing.T) { err := runDeployForOrg(context.Background(), " ", nil, []string{"githubnext/agentics/ci-doctor"}, AddOptions{}, time.Hour, false, false) require.Error(t, err) - assert.Contains(t, err.Error(), "--org cannot be empty") + require.ErrorContains(t, err, "--org cannot be empty") } func TestRunDeployForOrgInvalidRepoGlob(t *testing.T) { err := runDeployForOrg(context.Background(), "octo", []string{"["}, []string{"githubnext/agentics/ci-doctor"}, AddOptions{}, time.Hour, false, false) require.Error(t, err) - assert.Contains(t, err.Error(), "invalid --repos pattern") + require.ErrorContains(t, err, "invalid --repos pattern") } func TestRunDeployForOrgCreatePRRequiresYesInCI(t *testing.T) { @@ -31,7 +31,7 @@ func TestRunDeployForOrgCreatePRRequiresYesInCI(t *testing.T) { err := runDeployForOrg(context.Background(), "octo", nil, []string{"githubnext/agentics/ci-doctor"}, AddOptions{}, time.Hour, false, false) require.Error(t, err) - assert.Contains(t, err.Error(), "--yes") + require.ErrorContains(t, err, "--yes") } func TestRunDeployForOrgAppliesAcrossRepositories(t *testing.T) { @@ -77,5 +77,5 @@ func TestRunDeployForOrgReportsFailureWhenAllReposFail(t *testing.T) { err := runDeployForOrg(context.Background(), "octo", nil, []string{"githubnext/agentics/ci-doctor"}, AddOptions{}, time.Hour, true, false) require.Error(t, err) - assert.Contains(t, err.Error(), "failed to deploy workflows to any repository") + require.ErrorContains(t, err, "failed to deploy workflows to any repository") } diff --git a/pkg/cli/doctor_command_test.go b/pkg/cli/doctor_command_test.go index af5d22ee9ca..6e5c556ace9 100644 --- a/pkg/cli/doctor_command_test.go +++ b/pkg/cli/doctor_command_test.go @@ -284,6 +284,6 @@ func TestRunSetupRepositoryCheckAutoDetectsDefaultGHHost(t *testing.T) { RequireOwnerType: "any", }, runtime) require.Error(t, err) - assert.Contains(t, err.Error(), "git checkout") + require.ErrorContains(t, err, "git checkout") assert.Equal(t, "ghes.example.com", getGHHostFromCommandEnv(workflow.ExecGH("auth", "status"))) } diff --git a/pkg/cli/engine_secrets_test.go b/pkg/cli/engine_secrets_test.go index 81d6ea07ad9..3d25361707e 100644 --- a/pkg/cli/engine_secrets_test.go +++ b/pkg/cli/engine_secrets_test.go @@ -497,7 +497,7 @@ func TestGetEngineSecretNameAndValue(t *testing.T) { _, _, _, err := GetEngineSecretNameAndValue("unknown-engine", existingSecrets) require.Error(t, err, "Should error for unknown engine") - assert.Contains(t, err.Error(), "unknown engine", "Error should mention unknown engine") + require.ErrorContains(t, err, "unknown engine", "Error should mention unknown engine") }) t.Run("no alternative secret in repo", func(t *testing.T) { diff --git a/pkg/cli/env_command_test.go b/pkg/cli/env_command_test.go index 1292b353f32..52a6a503984 100644 --- a/pkg/cli/env_command_test.go +++ b/pkg/cli/env_command_test.go @@ -71,7 +71,7 @@ func TestResolveDefaultsTarget(t *testing.T) { t.Run("update requires scope", func(t *testing.T) { _, err := resolveDefaultsTarget("", "", "", "", true) require.Error(t, err) - assert.Contains(t, err.Error(), "scope is required") + require.ErrorContains(t, err, "scope is required") }) t.Run("org scope infers owner from repo", func(t *testing.T) { @@ -84,7 +84,7 @@ func TestResolveDefaultsTarget(t *testing.T) { t.Run("ent scope requires enterprise", func(t *testing.T) { _, err := resolveDefaultsTarget(defaultsScopeEnt, "", "", "", false) require.Error(t, err) - assert.Contains(t, err.Error(), "--enterprise") + require.ErrorContains(t, err, "--enterprise") }) } @@ -149,7 +149,7 @@ func TestDefaultsFileYAMLNullDelete(t *testing.T) { func TestDefaultsParseFileDisallowsUnknownFields(t *testing.T) { _, err := defaultsParseFile("defaults.yml", []byte("default_max_turns: \"42\"\ndefault_model_copliot: gpt-5-mini\n")) require.Error(t, err) - assert.Contains(t, err.Error(), "default_model_copliot") + require.ErrorContains(t, err, "default_model_copliot") } func TestDefaultsValidateFile(t *testing.T) { @@ -189,14 +189,24 @@ func TestDefaultsValidateFile(t *testing.T) { DefaultModelCopilot: new(" "), }) require.Error(t, err) - assert.Contains(t, err.Error(), "default_max_ai_credits must be a non-zero integer when set") - assert.Contains(t, err.Error(), "default_max_turn_cache_misses must be a positive integer when set") - assert.Contains(t, err.Error(), "default_detection_max_ai_credits must be a non-zero integer when set") - assert.Contains(t, err.Error(), "default_max_daily_ai_credits must be a non-zero integer when set") - assert.Contains(t, err.Error(), "default_max_turns must be a positive integer when set") - assert.Contains(t, err.Error(), "default_timeout_minutes must be a positive integer when set") - assert.Contains(t, err.Error(), "default_utc must be a numeric UTC offset") - assert.Contains(t, err.Error(), "default_model_copilot cannot be empty when set") + validationErr := err + for _, tc := range []struct { + name string + expectedErrMessage string + }{ + {name: "max_ai_credits", expectedErrMessage: "default_max_ai_credits must be a non-zero integer when set"}, + {name: "max_turn_cache_misses", expectedErrMessage: "default_max_turn_cache_misses must be a positive integer when set"}, + {name: "detection_max_ai_credits", expectedErrMessage: "default_detection_max_ai_credits must be a non-zero integer when set"}, + {name: "max_daily_ai_credits", expectedErrMessage: "default_max_daily_ai_credits must be a non-zero integer when set"}, + {name: "max_turns", expectedErrMessage: "default_max_turns must be a positive integer when set"}, + {name: "timeout_minutes", expectedErrMessage: "default_timeout_minutes must be a positive integer when set"}, + {name: "utc", expectedErrMessage: "default_utc must be a numeric UTC offset"}, + {name: "model_copilot", expectedErrMessage: "default_model_copilot cannot be empty when set"}, + } { + t.Run(tc.name, func(t *testing.T) { + require.ErrorContains(t, validationErr, tc.expectedErrMessage) + }) + } }) } diff --git a/pkg/cli/error_formatting_test.go b/pkg/cli/error_formatting_test.go index 73679f14d53..1dd866cc0e9 100644 --- a/pkg/cli/error_formatting_test.go +++ b/pkg/cli/error_formatting_test.go @@ -78,7 +78,7 @@ func TestResolveWorkflowErrorFormatting(t *testing.T) { require.Error(t, err, "Expected error for non-existent file") // Error message should contain helpful information - assert.Contains(t, err.Error(), "not found", "Error should mention file not found") + require.ErrorContains(t, err, "not found", "Error should mention file not found") } // TestConsoleFormatErrorMessageUsage verifies console.FormatErrorMessage is used correctly diff --git a/pkg/cli/firewall_policy_test.go b/pkg/cli/firewall_policy_test.go index a160ade94c7..2eef4eadc62 100644 --- a/pkg/cli/firewall_policy_test.go +++ b/pkg/cli/firewall_policy_test.go @@ -685,7 +685,7 @@ func TestDetectFirewallAuditArtifacts(t *testing.T) { _, _, err := detectFirewallAuditArtifacts(dir) require.Error(t, err, "Should return an error when run dir is unreadable") - assert.Contains(t, err.Error(), "detectFirewallAuditArtifacts", "Error should identify the function") + require.ErrorContains(t, err, "detectFirewallAuditArtifacts", "Error should identify the function") }) } diff --git a/pkg/cli/fix_codemods_test.go b/pkg/cli/fix_codemods_test.go index 63356192b0b..43b05ae61a4 100644 --- a/pkg/cli/fix_codemods_test.go +++ b/pkg/cli/fix_codemods_test.go @@ -166,7 +166,7 @@ func TestGetCodemods_UnknownDisabledCodemodReturnsError(t *testing.T) { codemods, err := GetCodemods([]string{"not-a-real-codemod"}) require.Error(t, err) assert.Nil(t, codemods) - assert.Contains(t, err.Error(), "unknown codemod ID(s): not-a-real-codemod") + require.ErrorContains(t, err, "unknown codemod ID(s): not-a-real-codemod") } func TestGetAllCodemods_InExpectedOrder(t *testing.T) { diff --git a/pkg/cli/forecast_compliance_fixtures_formal_test.go b/pkg/cli/forecast_compliance_fixtures_formal_test.go index e8aff3c40e1..7ee991bda1a 100644 --- a/pkg/cli/forecast_compliance_fixtures_formal_test.go +++ b/pkg/cli/forecast_compliance_fixtures_formal_test.go @@ -326,7 +326,7 @@ func TestFormal_P11_FlagValidation_Days(t *testing.T) { err := RunForecast(cfg) require.Error(t, err, "P11: days=%d must return an error (only 7 and 30 are valid)", days) - assert.Contains(t, err.Error(), "must be 7 or 30", + require.ErrorContains(t, err, "must be 7 or 30", "P11: error message must document the allowed values") } diff --git a/pkg/cli/gateway_logs_test.go b/pkg/cli/gateway_logs_test.go index 9695f41809c..7e2142b050f 100644 --- a/pkg/cli/gateway_logs_test.go +++ b/pkg/cli/gateway_logs_test.go @@ -116,7 +116,7 @@ func TestParseGatewayLogsFileNotFound(t *testing.T) { require.Error(t, err) assert.Nil(t, metrics) - assert.Contains(t, err.Error(), "gateway.jsonl not found") + require.ErrorContains(t, err, "gateway.jsonl not found") } func TestGatewayToolMetrics(t *testing.T) { diff --git a/pkg/cli/health_command_test.go b/pkg/cli/health_command_test.go index 3a3cbb975c6..8f6e4990359 100644 --- a/pkg/cli/health_command_test.go +++ b/pkg/cli/health_command_test.go @@ -71,7 +71,7 @@ func TestHealthConfigValidation(t *testing.T) { err := RunHealth(tt.config) if tt.wantDaysErr { require.Error(t, err, "RunHealth should return a validation error for: %s", tt.name) - assert.Contains(t, err.Error(), tt.errContains, "Error message should describe the validation failure") + require.ErrorContains(t, err, tt.errContains, "Error message should describe the validation failure") } else { // Valid days values pass days validation; any error comes from GitHub API access if err != nil { @@ -101,8 +101,8 @@ func TestRunHealthInvalidDays(t *testing.T) { config := HealthConfig{Days: tt.days, Threshold: 80.0} err := RunHealth(config) require.Error(t, err, "RunHealth should return an error for days=%d", tt.days) - assert.Contains(t, err.Error(), tt.errContains, "Error should describe the invalid days value") - assert.Contains(t, err.Error(), "Must be 7, 30, or 90", "Error should list the valid days options") + require.ErrorContains(t, err, tt.errContains, "Error should describe the invalid days value") + require.ErrorContains(t, err, "Must be 7, 30, or 90", "Error should list the valid days options") }) } } diff --git a/pkg/cli/import_url_fetcher_test.go b/pkg/cli/import_url_fetcher_test.go index f2b69ce58fc..cd0e57a0760 100644 --- a/pkg/cli/import_url_fetcher_test.go +++ b/pkg/cli/import_url_fetcher_test.go @@ -80,7 +80,7 @@ func TestFetchImportURL_NotFound(t *testing.T) { _, err := FetchImportURL(context.Background(), srv.URL+"/missing.md", FetchOptions{HTTPClient: srv.Client()}) require.Error(t, err) - assert.Contains(t, err.Error(), "404") + require.ErrorContains(t, err, "404") } func TestFetchImportURL_Unauthorized(t *testing.T) { @@ -91,7 +91,7 @@ func TestFetchImportURL_Unauthorized(t *testing.T) { _, err := FetchImportURL(context.Background(), srv.URL+"/private.md", FetchOptions{HTTPClient: srv.Client()}) require.Error(t, err) - assert.Contains(t, err.Error(), "401") + require.ErrorContains(t, err, "401") } func TestFetchImportURL_SizeCap(t *testing.T) { @@ -108,7 +108,7 @@ func TestFetchImportURL_SizeCap(t *testing.T) { _, err := FetchImportURL(context.Background(), srv.URL+"/big.md", FetchOptions{HTTPClient: srv.Client()}) require.Error(t, err) - assert.Contains(t, err.Error(), "size limit") + require.ErrorContains(t, err, "size limit") } func TestFetchImportURL_HeadFallbackToGET(t *testing.T) { @@ -478,5 +478,5 @@ func TestFetchImportURL_400ReturnsError(t *testing.T) { }) require.Error(t, err, "400 must be returned as an error") - assert.Contains(t, err.Error(), "400", "error must mention the status code") + require.ErrorContains(t, err, "400", "error must mention the status code") } diff --git a/pkg/cli/interfaces_test.go b/pkg/cli/interfaces_test.go index 826e337d390..321ea60cc0c 100644 --- a/pkg/cli/interfaces_test.go +++ b/pkg/cli/interfaces_test.go @@ -123,7 +123,7 @@ func TestInstallShellCompletion_TypeAssertion(t *testing.T) { // Should fail type assertion err := InstallShellCompletion(false, nil) require.Error(t, err, "Should fail with nil rootCmd") - assert.Contains(t, err.Error(), "must be a *cobra.Command", + require.ErrorContains(t, err, "must be a *cobra.Command", "Should fail with type assertion error message") }) } diff --git a/pkg/cli/lint_command_test.go b/pkg/cli/lint_command_test.go index 01a23c90f59..1ae5aef5d3e 100644 --- a/pkg/cli/lint_command_test.go +++ b/pkg/cli/lint_command_test.go @@ -77,6 +77,6 @@ func TestResolveLockFilesForLint(t *testing.T) { t.Run("rejects non lock file path", func(t *testing.T) { _, err := resolveLockFilesForLint([]string{nonLock}, tempDir) require.Error(t, err, "should reject non-lock file path") - assert.Contains(t, err.Error(), "is not a .lock.yml file or directory", "error should explain allowed path types") + require.ErrorContains(t, err, "is not a .lock.yml file or directory", "error should explain allowed path types") }) } diff --git a/pkg/cli/logs_command_test.go b/pkg/cli/logs_command_test.go index 3a1c2c7fb25..66586c96a18 100644 --- a/pkg/cli/logs_command_test.go +++ b/pkg/cli/logs_command_test.go @@ -314,7 +314,7 @@ func TestLogsCommandStdinRejectsPositionalArgs(t *testing.T) { cmd.SetErr(nil) err := cmd.Execute() require.Error(t, err, "logs --stdin with a positional arg should return an error") - assert.Contains(t, err.Error(), "positional arguments are not allowed with --stdin", "error message should explain the conflict") + require.ErrorContains(t, err, "positional arguments are not allowed with --stdin", "error message should explain the conflict") } // TestLogsCommand_RepoBypassesLocalWorkflowResolution verifies that specifying diff --git a/pkg/cli/logs_extract_zip_test.go b/pkg/cli/logs_extract_zip_test.go index d7f662007cb..461427b2c42 100644 --- a/pkg/cli/logs_extract_zip_test.go +++ b/pkg/cli/logs_extract_zip_test.go @@ -104,7 +104,7 @@ func TestExtractZipFileZipSlipPrevention(t *testing.T) { // Extract the file - should fail with error err = extractZipFile(zipReader.File[0], tempDir, false) require.Error(t, err, "extractZipFile should fail for path traversal") - assert.Contains(t, err.Error(), "invalid file path", "Error should mention invalid path") + require.ErrorContains(t, err, "invalid file path", "Error should mention invalid path") } // TestExtractZipFilePreservesMode tests that file permissions are preserved @@ -224,7 +224,7 @@ func TestExtractZipFileErrorHandling(t *testing.T) { // Likely running with elevated privileges. t.Skip("expected extraction to fail in read-only directory, but it succeeded (likely elevated privileges)") } - assert.Contains(t, err.Error(), "failed to create", "Error should mention creation failure") + assert.ErrorContains(t, err, "failed to create", "Error should mention creation failure") }) t.Run("validates error return signature for writable file close", func(t *testing.T) { diff --git a/pkg/cli/logs_usage_activity_test.go b/pkg/cli/logs_usage_activity_test.go index 30402a46c26..61b43e5111e 100644 --- a/pkg/cli/logs_usage_activity_test.go +++ b/pkg/cli/logs_usage_activity_test.go @@ -171,7 +171,7 @@ func TestLoadUsageActivitySummaryRejectsUnsupportedSchema(t *testing.T) { summary, err := loadUsageActivitySummary(runDir) require.Error(t, err, "unsupported activity summary schema should return an error") assert.Nil(t, summary, "unsupported schema should not be returned") - assert.Contains(t, err.Error(), "unsupported usage activity summary schema", "schema validation error should explain the mismatch") + require.ErrorContains(t, err, "unsupported usage activity summary schema", "schema validation error should explain the mismatch") } func TestApplyUsageActivitySummaryDoesNotOverwriteExistingData(t *testing.T) { diff --git a/pkg/cli/mcp_argument_validation_test.go b/pkg/cli/mcp_argument_validation_test.go index a783639a239..f9be889c0eb 100644 --- a/pkg/cli/mcp_argument_validation_test.go +++ b/pkg/cli/mcp_argument_validation_test.go @@ -89,7 +89,7 @@ func TestExtractUnknownParamsFromSchemaError(t *testing.T) { "workflow-name": "typo", }) require.Error(t, err) - assert.Contains(t, err.Error(), "unexpected additional properties") + require.ErrorContains(t, err, "unexpected additional properties") assert.Equal(t, []string{"workflow-name"}, extractUnknownParams(err.Error())) } diff --git a/pkg/cli/mcp_list_tools_test.go b/pkg/cli/mcp_list_tools_test.go index 1f5d1a7601c..dddcbb21be5 100644 --- a/pkg/cli/mcp_list_tools_test.go +++ b/pkg/cli/mcp_list_tools_test.go @@ -16,7 +16,6 @@ import ( "github.com/github/gh-aw/pkg/parser" "github.com/modelcontextprotocol/go-sdk/mcp" "github.com/spf13/cobra" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -388,6 +387,6 @@ func TestMCPListToolsRequiresServerFlagWithGuidance(t *testing.T) { cmd.SetArgs([]string{}) err := cmd.Execute() require.Error(t, err, "mcp list-tools without --server should fail") - assert.Contains(t, err.Error(), "missing required flag: --server", "error should clearly identify the missing required flag") - assert.Contains(t, err.Error(), "gh aw mcp list-tools --server github", "error should include guidance with a concrete example") + require.ErrorContains(t, err, "missing required flag: --server", "error should clearly identify the missing required flag") + require.ErrorContains(t, err, "gh aw mcp list-tools --server github", "error should include guidance with a concrete example") } diff --git a/pkg/cli/mcp_server_add_test.go b/pkg/cli/mcp_server_add_test.go index 34e7663fe40..aaa68f30efd 100644 --- a/pkg/cli/mcp_server_add_test.go +++ b/pkg/cli/mcp_server_add_test.go @@ -169,7 +169,7 @@ func TestMCPServer_AddToolInvocation(t *testing.T) { if tt.allowToolError { if err != nil { for _, expectedErrPart := range tt.errContains { - assert.Contains(t, err.Error(), expectedErrPart, "Expected validation protocol error to include required detail") + assert.ErrorContains(t, err, expectedErrPart, "Expected validation protocol error to include required detail") } return } @@ -192,8 +192,11 @@ func TestMCPServer_AddToolInvocation(t *testing.T) { if tt.expectErr { require.Error(t, err, "Expected add tool call to fail for invalid input scenario") + addErr := err for _, expectedErrPart := range tt.errContains { - assert.Contains(t, err.Error(), expectedErrPart, "Expected error to include informative failure details") + t.Run(expectedErrPart, func(t *testing.T) { + require.ErrorContains(t, addErr, expectedErrPart, "Expected error to include informative failure details") + }) } return } diff --git a/pkg/cli/mcp_server_workflow_validation_test.go b/pkg/cli/mcp_server_workflow_validation_test.go index 9dffd60693f..fc3792ba051 100644 --- a/pkg/cli/mcp_server_workflow_validation_test.go +++ b/pkg/cli/mcp_server_workflow_validation_test.go @@ -49,7 +49,7 @@ func TestMCPValidateWorkflowName(t *testing.T) { } else { assert.Error(t, err, "Validation should fail for workflow: %s", tt.workflowName) if tt.errorContains != "" { - assert.Contains(t, err.Error(), tt.errorContains, "Error message should contain expected text") + assert.ErrorContains(t, err, tt.errorContains, "Error message should contain expected text") } } }) diff --git a/pkg/cli/outcome_eval_test.go b/pkg/cli/outcome_eval_test.go index 9d5a950451f..8d03c24c8e7 100644 --- a/pkg/cli/outcome_eval_test.go +++ b/pkg/cli/outcome_eval_test.go @@ -160,7 +160,7 @@ func TestValidateAPIEndpoint(t *testing.T) { return } require.Error(t, err) - assert.Contains(t, err.Error(), tt.wantErr) + require.ErrorContains(t, err, tt.wantErr) }) } } diff --git a/pkg/cli/remote_workflow_test.go b/pkg/cli/remote_workflow_test.go index a470d153e83..05d6d56ed0a 100644 --- a/pkg/cli/remote_workflow_test.go +++ b/pkg/cli/remote_workflow_test.go @@ -90,7 +90,7 @@ func TestFetchLocalWorkflow_NonExistentFile(t *testing.T) { require.Error(t, err, "should error for non-existent file") assert.Nil(t, result, "result should be nil on error") - assert.Contains(t, err.Error(), "not found", "error should mention file not found") + require.ErrorContains(t, err, "not found", "error should mention file not found") } func TestFetchLocalWorkflow_DirectoryInsteadOfFile(t *testing.T) { @@ -168,9 +168,9 @@ func TestResolveCommitSHAWithRetries_PermanentFailureDoesNotRetry(t *testing.T) assert.Empty(t, sha, "No SHA should be returned when resolution fails") assert.Equal(t, 1, resolveAttempts, "Permanent failures should not retry") assert.Equal(t, 0, sleepCalls, "No backoff sleep should happen for permanent failures") - assert.Contains(t, err.Error(), "Expected the GitHub API to return a commit SHA for the ref", + require.ErrorContains(t, err, "Expected the GitHub API to return a commit SHA for the ref", "Error should explain expected behavior") - assert.Contains(t, err.Error(), "@<40-char-sha>", "Error should include retry command with full SHA placeholder") + require.ErrorContains(t, err, "@<40-char-sha>", "Error should include retry command with full SHA placeholder") } func TestResolveCommitSHAWithRetries_TransientFailureExhaustsRetries(t *testing.T) { @@ -201,7 +201,7 @@ func TestResolveCommitSHAWithRetries_TransientFailureExhaustsRetries(t *testing. assert.Empty(t, sha, "No SHA should be returned when retries are exhausted") assert.Equal(t, 4, resolveAttempts, "Should attempt initial call plus three retries") assert.Equal(t, 3, sleepCalls, "Should sleep between each retry") - assert.Contains(t, err.Error(), "after 3 retries", "Error should report retry exhaustion") + require.ErrorContains(t, err, "after 3 retries", "Error should report retry exhaustion") } func TestResolveCommitSHAWithRetries_ContextCanceledDuringBackoff(t *testing.T) { @@ -229,8 +229,8 @@ func TestResolveCommitSHAWithRetries_ContextCanceledDuringBackoff(t *testing.T) sha, err := resolveCommitSHAWithRetries(ctx, "owner", "repo", "main", ".github/workflows/test.md", "", false) require.Error(t, err, "Cancellation during retry backoff should fail fast") assert.Empty(t, sha, "No SHA should be returned when retry wait is canceled") - assert.Contains(t, err.Error(), "retry wait was cancelled", "Error should explain cancellation reason") - assert.Contains(t, err.Error(), "@<40-char-sha>", "Error should include exact SHA retry guidance") + require.ErrorContains(t, err, "retry wait was cancelled", "Error should explain cancellation reason") + require.ErrorContains(t, err, "@<40-char-sha>", "Error should include exact SHA retry guidance") } func TestFetchIncludeFromSource_WorkflowSpecParsing(t *testing.T) { @@ -291,7 +291,7 @@ func TestFetchIncludeFromSource_WorkflowSpecParsing(t *testing.T) { if tt.expectError { require.Error(t, err, "expected error") if tt.errorContains != "" { - assert.Contains(t, err.Error(), tt.errorContains, "error should contain expected text") + require.ErrorContains(t, err, tt.errorContains, "error should contain expected text") } } else { require.NoError(t, err, "should not error") @@ -1180,7 +1180,7 @@ resources: resources, err := extractResources(content) require.Error(t, err, "should error when a resource entry contains macro syntax") assert.Nil(t, resources, "should return nil resources on error") - assert.Contains(t, err.Error(), "${{", "error message should mention the disallowed syntax") + require.ErrorContains(t, err, "${{", "error message should mention the disallowed syntax") } // TestExtractResources_AllMacrosRejected verifies that all-macro lists return an error. @@ -1292,7 +1292,7 @@ resources: tmpDir := t.TempDir() err := fetchAndSaveRemoteResources(t.Context(), content, spec, tmpDir, false, false, nil) require.Error(t, err, "should error when resources contain macro syntax") - assert.Contains(t, err.Error(), "${{", "error should mention the disallowed syntax") + require.ErrorContains(t, err, "${{", "error should mention the disallowed syntax") entries, readErr := os.ReadDir(tmpDir) require.NoError(t, readErr) @@ -1785,9 +1785,9 @@ safe-outputs: err := fetchAndSaveRemoteDispatchWorkflows(context.Background(), content, spec, workflowsDir, false, false, nil) require.Error(t, err, "should error when existing file has a different source repo") - assert.Contains(t, err.Error(), "target-workflow", "error should name the conflicting file") - assert.Contains(t, err.Error(), "otherorg/other-repo", "error should mention existing source") - assert.Contains(t, err.Error(), "github/gh-aw", "error should mention the intended source") + require.ErrorContains(t, err, "target-workflow", "error should name the conflicting file") + require.ErrorContains(t, err, "otherorg/other-repo", "error should mention existing source") + require.ErrorContains(t, err, "github/gh-aw", "error should mention the intended source") } // TestFetchDispatchWorkflows_SameSourceSkips verifies that an existing dispatch-workflow @@ -1852,8 +1852,8 @@ safe-outputs: err := fetchAndSaveRemoteDispatchWorkflows(context.Background(), content, spec, workflowsDir, false, false, nil) require.Error(t, err, "should error when existing file has no source field") - assert.Contains(t, err.Error(), "target-workflow", "error should name the conflicting file") - assert.Contains(t, err.Error(), "(no source field)", "error should show placeholder for missing source") + require.ErrorContains(t, err, "target-workflow", "error should name the conflicting file") + require.ErrorContains(t, err, "(no source field)", "error should show placeholder for missing source") } // TestFetchDispatchWorkflows_ForceOverwritesConflict verifies that --force bypasses conflict detection. @@ -1977,7 +1977,7 @@ resources: err := fetchAndSaveRemoteResources(t.Context(), content, spec, dir, false, false, nil) require.Error(t, err, "should error when markdown resource exists from a different source") - assert.Contains(t, err.Error(), "helper.md", "error should name the conflicting resource") + require.ErrorContains(t, err, "helper.md", "error should name the conflicting resource") } // TestFetchResources_NonMarkdownConflict verifies that a non-markdown resource that already @@ -2002,7 +2002,7 @@ resources: err := fetchAndSaveRemoteResources(t.Context(), content, spec, dir, false, false, nil) require.Error(t, err, "should error when non-markdown resource already exists") - assert.Contains(t, err.Error(), "helper.yml", "error should name the conflicting resource") + require.ErrorContains(t, err, "helper.yml", "error should name the conflicting resource") } // TestFetchResources_MarkdownSameSourceSkips verifies that an existing markdown resource @@ -2190,7 +2190,7 @@ safe-outputs: err := fetchAllRemoteDependencies(context.Background(), content, spec, tmpDir, false, false, nil) require.Error(t, err, "dispatch workflow conflict should be propagated") - assert.Contains(t, err.Error(), "dispatch workflow", "error should mention 'dispatch workflow'") + require.ErrorContains(t, err, "dispatch workflow", "error should mention 'dispatch workflow'") } // TestFetchAllRemoteDependencies_ResourceMacroErrorPropagated verifies that a resource @@ -2213,5 +2213,5 @@ resources: tmpDir := t.TempDir() err := fetchAllRemoteDependencies(context.Background(), content, spec, tmpDir, false, false, nil) require.Error(t, err, "resource macro error should be propagated") - assert.Contains(t, err.Error(), "failed to fetch resource dependencies", "error should be wrapped with dependency context") + require.ErrorContains(t, err, "failed to fetch resource dependencies", "error should be wrapped with dependency context") } diff --git a/pkg/cli/run_push_test.go b/pkg/cli/run_push_test.go index 9b0b4d242de..07249599a6e 100644 --- a/pkg/cli/run_push_test.go +++ b/pkg/cli/run_push_test.go @@ -493,7 +493,7 @@ func TestPushWorkflowFiles_WithStagedFiles(t *testing.T) { // Should return an error about staged files require.Error(t, err) - assert.Contains(t, err.Error(), "staged files") + require.ErrorContains(t, err, "staged files") } func TestCollectWorkflowFiles_AlwaysRecompiles(t *testing.T) { diff --git a/pkg/cli/run_workflow_validation_test.go b/pkg/cli/run_workflow_validation_test.go index b85a1b9c353..c58d7a64668 100644 --- a/pkg/cli/run_workflow_validation_test.go +++ b/pkg/cli/run_workflow_validation_test.go @@ -149,7 +149,7 @@ jobs: if tt.expectError { require.Error(t, err) if tt.errorContains != "" { - assert.Contains(t, err.Error(), tt.errorContains) + require.ErrorContains(t, err, tt.errorContains) } } else { require.NoError(t, err) @@ -319,7 +319,7 @@ jobs: if tt.expectError { require.Error(t, err) if tt.errorContains != "" { - assert.Contains(t, err.Error(), tt.errorContains) + require.ErrorContains(t, err, tt.errorContains) } } else { require.NoError(t, err) @@ -420,8 +420,8 @@ jobs: // Test with missing required input err = validateWorkflowInputs(markdownPath, []string{}) require.Error(t, err) - assert.Contains(t, err.Error(), "Missing required input(s)") - assert.Contains(t, err.Error(), "issue_url") + require.ErrorContains(t, err, "Missing required input(s)") + require.ErrorContains(t, err, "issue_url") var validationErr *workflow.WorkflowValidationError require.ErrorAs(t, err, &validationErr, "expected WorkflowValidationError for invalid inputs") assert.Contains(t, validationErr.Suggestion, "on:") @@ -435,7 +435,7 @@ jobs: // Test with typo in input name err = validateWorkflowInputs(markdownPath, []string{"issue_ur=https://example.com"}) require.Error(t, err) - assert.Contains(t, err.Error(), "Invalid input name") - assert.Contains(t, err.Error(), "issue_ur", "Error should include invalid input") - assert.Contains(t, err.Error(), "issue_url", "Error should suggest correct input name") + require.ErrorContains(t, err, "Invalid input name") + require.ErrorContains(t, err, "issue_ur", "Error should include invalid input") + require.ErrorContains(t, err, "issue_url", "Error should suggest correct input name") } diff --git a/pkg/cli/setup_command_test.go b/pkg/cli/setup_command_test.go index d0a263461c5..1566da87817 100644 --- a/pkg/cli/setup_command_test.go +++ b/pkg/cli/setup_command_test.go @@ -155,8 +155,8 @@ func TestRunSetupRepositoryCheck_RejectsNonExistentNestedCheckoutPath(t *testing repoExists: func(context.Context, string) (bool, error) { return true, nil }, }) require.Error(t, err) - assert.Contains(t, err.Error(), "is inside a different git checkout rooted at") - assert.Contains(t, err.Error(), parentRepoDir) + require.ErrorContains(t, err, "is inside a different git checkout rooted at") + require.ErrorContains(t, err, parentRepoDir) } func TestCreateSetupRepository_UsesSupportedFlags(t *testing.T) { diff --git a/pkg/cli/shell_completion_test.go b/pkg/cli/shell_completion_test.go index d5e17dffd34..c041cec7d35 100644 --- a/pkg/cli/shell_completion_test.go +++ b/pkg/cli/shell_completion_test.go @@ -229,7 +229,7 @@ func TestUninstallBashCompletionNotFound(t *testing.T) { // Uninstall should fail when no file found err := uninstallBashCompletion(false) require.Error(t, err) - assert.Contains(t, err.Error(), "no bash completion file found") + require.ErrorContains(t, err, "no bash completion file found") } func TestUninstallZshCompletion(t *testing.T) { @@ -271,7 +271,7 @@ func TestUninstallZshCompletionNotFound(t *testing.T) { // Uninstall should fail when no file found err := uninstallZshCompletion(false) require.Error(t, err) - assert.Contains(t, err.Error(), "no zsh completion file found") + require.ErrorContains(t, err, "no zsh completion file found") } func TestUninstallFishCompletion(t *testing.T) { @@ -313,7 +313,7 @@ func TestUninstallFishCompletionNotFound(t *testing.T) { // Uninstall should fail when no file found err := uninstallFishCompletion(false) require.Error(t, err) - assert.Contains(t, err.Error(), "no fish completion file found") + require.ErrorContains(t, err, "no fish completion file found") } func TestUninstallShellCompletion(t *testing.T) { @@ -405,7 +405,7 @@ func TestUninstallShellCompletion(t *testing.T) { if tt.expectError { require.Error(t, err) if tt.errorMessage != "" { - assert.Contains(t, err.Error(), tt.errorMessage) + require.ErrorContains(t, err, tt.errorMessage) } } else { require.NoError(t, err) diff --git a/pkg/cli/status_remote_test.go b/pkg/cli/status_remote_test.go index b218a39510a..792e475d9be 100644 --- a/pkg/cli/status_remote_test.go +++ b/pkg/cli/status_remote_test.go @@ -109,5 +109,5 @@ func TestGetWorkflowStatuses_WithRepoFlag_SkipsLocalFiles(t *testing.T) { func TestGetWorkflowStatuses_LabelFilterWithRepo(t *testing.T) { _, err := GetWorkflowStatuses("", "", "my-label", "owner/repo") require.Error(t, err) - assert.Contains(t, err.Error(), "--label filter is not supported with --repo") + require.ErrorContains(t, err, "--label filter is not supported with --repo") } diff --git a/pkg/cli/tokens_bootstrap_test.go b/pkg/cli/tokens_bootstrap_test.go index 3dcfd105aa9..6c9a9670583 100644 --- a/pkg/cli/tokens_bootstrap_test.go +++ b/pkg/cli/tokens_bootstrap_test.go @@ -146,7 +146,7 @@ func TestCollectRequiredSecretsFromWorkflows_NoWorkflowsDir(t *testing.T) { t.Run("fails when no engine specified", func(t *testing.T) { _, err = getSecretRequirements("") require.Error(t, err, "Should error when no workflows directory exists and no engine specified") - assert.Contains(t, err.Error(), "failed to discover workflows", "Error should indicate workflow discovery failed") + require.ErrorContains(t, err, "failed to discover workflows", "Error should indicate workflow discovery failed") }) t.Run("succeeds when engine specified", func(t *testing.T) { @@ -190,7 +190,7 @@ func TestCollectRequiredSecretsFromWorkflows_EmptyWorkflowsDir(t *testing.T) { t.Run("fails when no engine specified", func(t *testing.T) { _, err = getSecretRequirements("") require.Error(t, err, "Should error when no workflow files found and no engine specified") - assert.Contains(t, err.Error(), "no workflow files found", "Error should indicate no workflow files") + require.ErrorContains(t, err, "no workflow files found", "Error should indicate no workflow files") }) t.Run("succeeds when engine specified", func(t *testing.T) { diff --git a/pkg/cli/trial_dry_run_test.go b/pkg/cli/trial_dry_run_test.go index 633fba47b3a..957536cefd3 100644 --- a/pkg/cli/trial_dry_run_test.go +++ b/pkg/cli/trial_dry_run_test.go @@ -171,7 +171,7 @@ func TestEnsureTrialRepositoryDryRun(t *testing.T) { if tt.expectError { require.Error(t, err, "Expected error for %s", tt.description) if tt.errorContains != "" { - assert.Contains(t, err.Error(), tt.errorContains, "Error should contain expected text") + require.ErrorContains(t, err, tt.errorContains, "Error should contain expected text") } } else { assert.NoError(t, err, "Should not error for %s", tt.description) @@ -373,7 +373,7 @@ func TestDryRunValidationStillOccurs(t *testing.T) { if tt.expectError { require.Error(t, err, "Expected validation error in dry-run mode") - assert.Contains(t, err.Error(), tt.errorContains, "Error should contain expected text") + require.ErrorContains(t, err, tt.errorContains, "Error should contain expected text") } else { assert.NoError(t, err, "Valid input should not error in dry-run mode") } diff --git a/pkg/cli/update_command_test.go b/pkg/cli/update_command_test.go index 7a60dc45628..5cf1eea040a 100644 --- a/pkg/cli/update_command_test.go +++ b/pkg/cli/update_command_test.go @@ -1086,5 +1086,5 @@ func TestRunUpdateWorkflows_SpecificWorkflowNotFound(t *testing.T) { WorkflowNames: []string{"nonexistent"}, }) require.Error(t, err, "Should error when specified workflow not found") - assert.Contains(t, err.Error(), "no workflows found matching the specified names") + require.ErrorContains(t, err, "no workflows found matching the specified names") } diff --git a/pkg/cli/update_extension_check_test.go b/pkg/cli/update_extension_check_test.go index bb8d0bd7a50..0e40033ef5b 100644 --- a/pkg/cli/update_extension_check_test.go +++ b/pkg/cli/update_extension_check_test.go @@ -365,7 +365,7 @@ func TestParseInstalledVersionOutput(t *testing.T) { t.Run("returns error when no version present", func(t *testing.T) { _, err := parseInstalledVersionOutput("gh-aw version unknown") require.Error(t, err) - assert.Contains(t, err.Error(), "could not parse installed gh-aw version") + require.ErrorContains(t, err, "could not parse installed gh-aw version") }) t.Run("uses first version match when multiple exist", func(t *testing.T) { @@ -377,7 +377,7 @@ func TestParseInstalledVersionOutput(t *testing.T) { t.Run("returns error for empty output", func(t *testing.T) { _, err := parseInstalledVersionOutput("") require.Error(t, err) - assert.Contains(t, err.Error(), "could not parse installed gh-aw version") + require.ErrorContains(t, err, "could not parse installed gh-aw version") }) } diff --git a/pkg/cli/update_org_test.go b/pkg/cli/update_org_test.go index 84abe86f12f..58a9fe1c9c2 100644 --- a/pkg/cli/update_org_test.go +++ b/pkg/cli/update_org_test.go @@ -22,7 +22,7 @@ func TestValidateRepoGlobs(t *testing.T) { err := validateRepoGlobs([]string{"["}) require.Error(t, err) - assert.Contains(t, err.Error(), "invalid --repos pattern") + require.ErrorContains(t, err, "invalid --repos pattern") } func TestFilterOrgRepos(t *testing.T) { @@ -80,7 +80,7 @@ func TestRunUpdateForOrgCreateIssueRequiresYesInCI(t *testing.T) { err := runUpdateForOrg(context.Background(), "octo", nil, UpdateWorkflowsOptions{}, false, true, false) require.Error(t, err) - assert.Contains(t, err.Error(), "--yes") + require.ErrorContains(t, err, "--yes") } func TestRunUpdateForOrgCreateIssueSkipsWhenDeclined(t *testing.T) { diff --git a/pkg/cli/update_redirects_test.go b/pkg/cli/update_redirects_test.go index 53ea8b70a12..4dd0160a99e 100644 --- a/pkg/cli/update_redirects_test.go +++ b/pkg/cli/update_redirects_test.go @@ -102,7 +102,7 @@ func TestResolveRedirectedUpdateLocation(t *testing.T) { 0, ) require.Error(t, err, "redirect loop should return an error") - assert.Contains(t, err.Error(), "redirect loop detected", "error should explain redirect loop") + require.ErrorContains(t, err, "redirect loop detected", "error should explain redirect loop") }) t.Run("refuses redirect when no-redirect is enabled", func(t *testing.T) { @@ -129,7 +129,7 @@ func TestResolveRedirectedUpdateLocation(t *testing.T) { 0, ) require.Error(t, err, "redirect should be refused with --no-redirect") - assert.Contains(t, err.Error(), "redirect is disabled by --no-redirect", "error should explain redirect refusal") + require.ErrorContains(t, err, "redirect is disabled by --no-redirect", "error should explain redirect refusal") }) t.Run("resolves default branch via API when source omits ref", func(t *testing.T) { diff --git a/pkg/cli/upgrade_command_test.go b/pkg/cli/upgrade_command_test.go index 377ae90b1db..b27fe8ecd94 100644 --- a/pkg/cli/upgrade_command_test.go +++ b/pkg/cli/upgrade_command_test.go @@ -73,8 +73,8 @@ func TestUpgradeCommandRepoOrgMutualExclusion(t *testing.T) { cmd.SetArgs([]string{"--repo", "owner/repo", "--org", "my-org"}) err := cmd.Execute() require.Error(t, err, "should error when both --repo and --org are specified") - assert.Contains(t, err.Error(), "--repo", "error should mention --repo flag") - assert.Contains(t, err.Error(), "--org", "error should mention --org flag") + require.ErrorContains(t, err, "--repo", "error should mention --repo flag") + require.ErrorContains(t, err, "--org", "error should mention --org flag") } func TestUpgradeCommandFlagRegistration(t *testing.T) { @@ -98,7 +98,7 @@ func TestUpgradeCommandFlagRegistration(t *testing.T) { cmd2.SetArgs([]string{"--repos", "foo-*"}) err := cmd2.Execute() require.Error(t, err, "should error when --repos is specified without --org") - assert.Contains(t, err.Error(), "--repos", "error should mention --repos flag") + require.ErrorContains(t, err, "--repos", "error should mention --repos flag") } // TestUpgradeCommandRepoDispatchNoPR verifies that plain `upgrade --repo` diff --git a/pkg/cli/upgrade_org_test.go b/pkg/cli/upgrade_org_test.go index b7d0912545a..6da27e72b9a 100644 --- a/pkg/cli/upgrade_org_test.go +++ b/pkg/cli/upgrade_org_test.go @@ -77,7 +77,7 @@ func TestRunUpgradeForOrgCreateIssueRequiresYesInCI(t *testing.T) { err := runUpgradeForOrg(context.Background(), "octo", nil, upgradeOptions{ctx: context.Background()}, false, true, false) require.Error(t, err) - assert.Contains(t, err.Error(), "--yes") + require.ErrorContains(t, err, "--yes") } func TestRunUpgradeForOrgCreatePRRequiresYesInCI(t *testing.T) { @@ -89,19 +89,19 @@ func TestRunUpgradeForOrgCreatePRRequiresYesInCI(t *testing.T) { err := runUpgradeForOrg(context.Background(), "octo", nil, upgradeOptions{ctx: context.Background()}, true, false, false) require.Error(t, err) - assert.Contains(t, err.Error(), "--yes") + require.ErrorContains(t, err, "--yes") } func TestRunUpgradeForOrgEmptyOrg(t *testing.T) { err := runUpgradeForOrg(context.Background(), " ", nil, upgradeOptions{ctx: context.Background()}, false, false, false) require.Error(t, err) - assert.Contains(t, err.Error(), "--org cannot be empty") + require.ErrorContains(t, err, "--org cannot be empty") } func TestRunUpgradeForOrgInvalidRepoGlob(t *testing.T) { err := runUpgradeForOrg(context.Background(), "octo", []string{"["}, upgradeOptions{ctx: context.Background()}, false, false, false) require.Error(t, err) - assert.Contains(t, err.Error(), "invalid --repos pattern") + require.ErrorContains(t, err, "invalid --repos pattern") } func TestRunUpgradeForOrgNoReposFound(t *testing.T) { @@ -314,7 +314,7 @@ func TestRunUpgradeCommandCreateIssueRequiresOrg(t *testing.T) { cmd.SetArgs([]string{"--create-issue"}) err := cmd.Execute() require.Error(t, err) - assert.Contains(t, err.Error(), "--create-issue requires --org") + require.ErrorContains(t, err, "--create-issue requires --org") } func TestRunUpgradeCommandCreateIssueAndPRMutuallyExclusive(t *testing.T) { @@ -322,7 +322,7 @@ func TestRunUpgradeCommandCreateIssueAndPRMutuallyExclusive(t *testing.T) { cmd.SetArgs([]string{"--org", "octo", "--create-issue", "--create-pull-request"}) err := cmd.Execute() require.Error(t, err) - assert.Contains(t, err.Error(), "cannot specify both --create-pull-request and --create-issue") + require.ErrorContains(t, err, "cannot specify both --create-pull-request and --create-issue") } func TestRunUpgradeCommandReposRequiresOrg(t *testing.T) { @@ -330,7 +330,7 @@ func TestRunUpgradeCommandReposRequiresOrg(t *testing.T) { cmd.SetArgs([]string{"--repos", "*-svc"}) err := cmd.Execute() require.Error(t, err) - assert.Contains(t, err.Error(), "--repos requires --org") + require.ErrorContains(t, err, "--repos requires --org") } func TestRunUpgradeForOrgSkipsFailedRepos(t *testing.T) { @@ -358,7 +358,7 @@ func TestRunUpgradeForOrgSkipsFailedRepos(t *testing.T) { err := runUpgradeForOrg(context.Background(), "octo", nil, upgradeOptions{ctx: context.Background(), yes: true}, true, false, false) require.Error(t, err) - assert.Contains(t, err.Error(), "failed to upgrade any repository") + require.ErrorContains(t, err, "failed to upgrade any repository") assert.Equal(t, []string{"octo/api", "octo/web"}, called, "should attempt all repos and skip failures") } @@ -393,7 +393,7 @@ func TestRunUpgradeForOrgCreateIssueSkipsFailedRepos(t *testing.T) { err := runUpgradeForOrg(context.Background(), "octo", nil, upgradeOptions{ctx: context.Background(), yes: true}, false, true, false) require.Error(t, err) - assert.Contains(t, err.Error(), "failed to create issues in any repository") + require.ErrorContains(t, err, "failed to create issues in any repository") assert.Equal(t, []string{"octo/api", "octo/web"}, called, "should attempt all repos and skip failures") } diff --git a/pkg/console/confirm_test.go b/pkg/console/confirm_test.go index 8cfe5682974..fda2249aaab 100644 --- a/pkg/console/confirm_test.go +++ b/pkg/console/confirm_test.go @@ -46,7 +46,7 @@ func TestShowTextConfirm(t *testing.T) { if tt.wantErr { require.Error(t, err) if tt.errContains != "" { - assert.Contains(t, err.Error(), tt.errContains) + require.ErrorContains(t, err, tt.errContains) } } else { require.NoError(t, err) diff --git a/pkg/console/input_test.go b/pkg/console/input_test.go index bd887c5c7ff..772561ddd0c 100644 --- a/pkg/console/input_test.go +++ b/pkg/console/input_test.go @@ -5,7 +5,6 @@ package console import ( "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -23,6 +22,6 @@ func TestPromptSecretInput(t *testing.T) { _, err := PromptSecretInput(title, description) // Will error in test environment (no TTY), but that's expected require.Error(t, err, "Should error when not in TTY") - assert.Contains(t, err.Error(), "not a TTY", "Error should mention TTY") + require.ErrorContains(t, err, "not a TTY", "Error should mention TTY") }) } diff --git a/pkg/console/list_test.go b/pkg/console/list_test.go index 9023c1cff52..e480d383f48 100644 --- a/pkg/console/list_test.go +++ b/pkg/console/list_test.go @@ -20,7 +20,7 @@ func TestShowInteractiveList_EmptyItems(t *testing.T) { items := []ListItem{} _, err := ShowInteractiveList("Test", items) require.Error(t, err) - assert.Contains(t, err.Error(), "no items to display") + require.ErrorContains(t, err, "no items to display") } // Note: Full interactive list testing requires TTY and cannot be automated. diff --git a/pkg/fileutil/fileutil_test.go b/pkg/fileutil/fileutil_test.go index 31030edd202..560457cc9d7 100644 --- a/pkg/fileutil/fileutil_test.go +++ b/pkg/fileutil/fileutil_test.go @@ -133,7 +133,7 @@ func TestValidateAbsolutePath(t *testing.T) { if tt.shouldError { require.Error(t, err, "Expected error for path: %s", tt.path) if tt.errorMsg != "" { - assert.Contains(t, err.Error(), tt.errorMsg, "Error message should contain expected text") + require.ErrorContains(t, err, tt.errorMsg, "Error message should contain expected text") } assert.Empty(t, result, "Result should be empty on error") } else { @@ -203,7 +203,7 @@ func TestValidateAbsolutePath_SecurityScenarios(t *testing.T) { t.Run("blocks_"+strings.ReplaceAll(pattern, "/", "_"), func(t *testing.T) { result, err := ValidateAbsolutePath(pattern) require.Error(t, err, "Should reject path traversal pattern: %s", pattern) - assert.Contains(t, err.Error(), "path must be absolute", "Error should mention absolute path requirement") + require.ErrorContains(t, err, "path must be absolute", "Error should mention absolute path requirement") assert.Empty(t, result, "Result should be empty for invalid path") }) } @@ -484,7 +484,7 @@ func TestValidatePathWithinBase(t *testing.T) { err := ValidatePathWithinBase(base, tt.candidate) if tt.shouldErr { require.Error(t, err, "ValidatePathWithinBase should reject path %q relative to %q", tt.candidate, base) - assert.Contains(t, err.Error(), "escapes base directory", "Error should describe the escape") + require.ErrorContains(t, err, "escapes base directory", "Error should describe the escape") } else { require.NoError(t, err, "ValidatePathWithinBase should accept path %q within %q", tt.candidate, base) } @@ -508,7 +508,7 @@ func TestValidatePathWithinBase(t *testing.T) { err = ValidatePathWithinBase(base, linkPath) require.Error(t, err, "ValidatePathWithinBase should reject symlink that points outside base") - assert.Contains(t, err.Error(), "escapes base directory", "Error should describe the symlink escape") + require.ErrorContains(t, err, "escapes base directory", "Error should describe the symlink escape") }) } @@ -539,7 +539,7 @@ func TestExtractFileFromTar_UnsafePaths(t *testing.T) { archive := buildTar(map[string][]byte{"file.txt": []byte("data")}) got, err := ExtractFileFromTar(archive, "/etc/passwd") require.Error(t, err, "Should reject absolute path as search target") - assert.Contains(t, err.Error(), "unsafe path", "Error should mention unsafe path") + require.ErrorContains(t, err, "unsafe path", "Error should mention unsafe path") assert.Nil(t, got, "Result should be nil for unsafe path") }) @@ -547,7 +547,7 @@ func TestExtractFileFromTar_UnsafePaths(t *testing.T) { archive := buildTar(map[string][]byte{"file.txt": []byte("data")}) got, err := ExtractFileFromTar(archive, "../escape.txt") require.Error(t, err, "Should reject .. in search target") - assert.Contains(t, err.Error(), "unsafe path", "Error should mention unsafe path") + require.ErrorContains(t, err, "unsafe path", "Error should mention unsafe path") assert.Nil(t, got, "Result should be nil for unsafe path") }) @@ -575,7 +575,7 @@ func TestExtractFileFromTar_UnsafePaths(t *testing.T) { // a target; the archive entry is just silently skipped. got, err := ExtractFileFromTar(archive, "escape.txt") require.Error(t, err, "File should not be found because dotdot entry was skipped") - assert.Contains(t, err.Error(), "not found", "Error should indicate file not found") + require.ErrorContains(t, err, "not found", "Error should indicate file not found") assert.Nil(t, got) }) } @@ -618,7 +618,7 @@ func TestExtractFileFromTar(t *testing.T) { got, err := ExtractFileFromTar(archive, "missing.txt") require.Error(t, err, "ExtractFileFromTar should return error when file is absent") - assert.Contains(t, err.Error(), "missing.txt", "Error should mention the missing filename") + require.ErrorContains(t, err, "missing.txt", "Error should mention the missing filename") assert.Nil(t, got, "Result should be nil when file is not found") }) diff --git a/pkg/gitutil/gitutil_test.go b/pkg/gitutil/gitutil_test.go index df7ac59aa17..b550439795d 100644 --- a/pkg/gitutil/gitutil_test.go +++ b/pkg/gitutil/gitutil_test.go @@ -327,7 +327,7 @@ func TestFindGitRootFrom(t *testing.T) { _, err := FindGitRootFrom(nonRepoDir) require.Error(t, err, "FindGitRootFrom should return error outside a git repository") - assert.Contains(t, err.Error(), "not in a git repository", "error should mention not in git repository") + require.ErrorContains(t, err, "not in a git repository", "error should mention not in git repository") }) t.Run("returns git root when .git is a worktree marker file", func(t *testing.T) { @@ -364,7 +364,7 @@ func TestFindGitRootFrom(t *testing.T) { _, err := FindGitRootFrom(repoRoot) require.Error(t, err, "FindGitRootFrom should not accept a .git file without gitdir: prefix") - assert.Contains(t, err.Error(), "not in a git repository") + require.ErrorContains(t, err, "not in a git repository") }) t.Run("handles relative path input", func(t *testing.T) { @@ -396,12 +396,12 @@ func TestReadFileFromHEAD(t *testing.T) { outsidePath := filepath.Join(t.TempDir(), "file.yml") _, err = ReadFileFromHEAD(outsidePath, gitRoot) require.Error(t, err, "should fail for a file outside the git root") - assert.Contains(t, err.Error(), "outside the git repository root", "error should mention path is outside repo") + require.ErrorContains(t, err, "outside the git repository root", "error should mention path is outside repo") }) t.Run("returns error for empty gitRoot", func(t *testing.T) { _, err := ReadFileFromHEAD("some/file.yml", "") require.Error(t, err, "should fail when gitRoot is empty") - assert.Contains(t, err.Error(), "gitRoot must not be empty", "error should mention empty gitRoot") + require.ErrorContains(t, err, "gitRoot must not be empty", "error should mention empty gitRoot") }) } diff --git a/pkg/gitutil/spec_test.go b/pkg/gitutil/spec_test.go index df19acf3129..a3b569580fa 100644 --- a/pkg/gitutil/spec_test.go +++ b/pkg/gitutil/spec_test.go @@ -388,7 +388,7 @@ func TestSpec_PublicAPI_ReadFileFromHEAD(t *testing.T) { outsidePath := filepath.Join(root, "..", "outside.txt") _, err := gitutil.ReadFileFromHEAD(outsidePath, root) require.Error(t, err, "ReadFileFromHEAD should reject path-traversal attempts") - assert.Contains(t, err.Error(), "outside the git repository root") + require.ErrorContains(t, err, "outside the git repository root") }) t.Run("returns error when gitRoot is empty", func(t *testing.T) { diff --git a/pkg/parser/import_bfs_test.go b/pkg/parser/import_bfs_test.go index 46501894c46..5ded9f2234d 100644 --- a/pkg/parser/import_bfs_test.go +++ b/pkg/parser/import_bfs_test.go @@ -5,7 +5,6 @@ package parser import ( "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -17,5 +16,5 @@ func TestParseImportSpecsFromArray_RejectsIfField(t *testing.T) { }, }) require.Error(t, err) - assert.Contains(t, err.Error(), "import 'if' is no longer supported") + require.ErrorContains(t, err, "import 'if' is no longer supported") } diff --git a/pkg/parser/import_cache_test.go b/pkg/parser/import_cache_test.go index 4015dba61f1..e95a6825c2b 100644 --- a/pkg/parser/import_cache_test.go +++ b/pkg/parser/import_cache_test.go @@ -286,7 +286,7 @@ func TestValidatePathComponents(t *testing.T) { err := validatePathComponents(tt.owner, tt.repo, tt.path, tt.sha) if tt.shouldErr { require.Error(t, err, "should return error for: %s", tt.name) - assert.Contains(t, err.Error(), tt.errMsg, "error message should mention: %s", tt.errMsg) + require.ErrorContains(t, err, tt.errMsg, "error message should mention: %s", tt.errMsg) } else { require.NoError(t, err, "should not return error for valid components") } @@ -364,7 +364,7 @@ func TestImportCacheSet_Validation(t *testing.T) { _, err := cache.Set(tt.owner, tt.repo, tt.path, tt.sha, tt.content) if tt.shouldErr { require.Error(t, err, "Set should reject: %s", tt.name) - assert.Contains(t, err.Error(), tt.errMsg, "error message should contain %q", tt.errMsg) + require.ErrorContains(t, err, tt.errMsg, "error message should contain %q", tt.errMsg) } else { assert.NoError(t, err, "Set should succeed for: %s", tt.name) } diff --git a/pkg/parser/import_conflict_test.go b/pkg/parser/import_conflict_test.go index af920761019..1061b5c3b61 100644 --- a/pkg/parser/import_conflict_test.go +++ b/pkg/parser/import_conflict_test.go @@ -69,8 +69,8 @@ permissions: _, err := parser.ProcessImportsFromFrontmatterWithSource(frontmatter, tempDir, nil, mainPath, mainContent) require.Error(t, err, "Importing the same file with conflicting 'with' values should error") - assert.Contains(t, err.Error(), "import conflict", "Error should mention import conflict") - assert.Contains(t, err.Error(), "shared.md", "Error should mention the conflicting file") + require.ErrorContains(t, err, "import conflict", "Error should mention import conflict") + require.ErrorContains(t, err, "shared.md", "Error should mention the conflicting file") } // TestImportConflict_SameFileIdenticalWith tests that importing the same file twice diff --git a/pkg/parser/import_field_extractor_test.go b/pkg/parser/import_field_extractor_test.go index fc94d21660f..b1f72eb5690 100644 --- a/pkg/parser/import_field_extractor_test.go +++ b/pkg/parser/import_field_extractor_test.go @@ -286,7 +286,7 @@ imports: _, err = ProcessImportsFromFrontmatterWithSource(result.Frontmatter, tmpDir, nil, "", "") require.Error(t, err, "Should error when two imports define the same env var") - assert.Contains(t, err.Error(), "SHARED_KEY", "Error should mention the conflicting variable name") + require.ErrorContains(t, err, "SHARED_KEY", "Error should mention the conflicting variable name") } // TestExtractAllImportFields_BuiltinCacheHit verifies that extractAllImportFields uses the @@ -399,7 +399,7 @@ func TestValidateImportInputType_Number(t *testing.T) { err := validateImportInputType("retries", "3", "number", paramDef, importPath) require.Error(t, err, "string value should be rejected for number type") - assert.Contains(t, err.Error(), "must be a number", "error should explain expected type") + require.ErrorContains(t, err, "must be a number", "error should explain expected type") }) } diff --git a/pkg/parser/inline_skill_extractor_test.go b/pkg/parser/inline_skill_extractor_test.go index ef2b948a888..57a9ab5365b 100644 --- a/pkg/parser/inline_skill_extractor_test.go +++ b/pkg/parser/inline_skill_extractor_test.go @@ -177,8 +177,8 @@ func TestExtractInlineSkills_DuplicateNameError(t *testing.T) { _, _, err := ExtractInlineSkills(markdown) require.Error(t, err, "duplicate skill name should produce an error") - assert.Contains(t, err.Error(), "duplicate", "error should mention duplicate") - assert.Contains(t, err.Error(), "planner", "error should include the duplicate name") + require.ErrorContains(t, err, "duplicate", "error should mention duplicate") + require.ErrorContains(t, err, "planner", "error should include the duplicate name") } func TestExtractInlineSkills_NameVariants(t *testing.T) { diff --git a/pkg/parser/remote_fetch_integration_test.go b/pkg/parser/remote_fetch_integration_test.go index 9c3cc75a82d..c1597ce4dec 100644 --- a/pkg/parser/remote_fetch_integration_test.go +++ b/pkg/parser/remote_fetch_integration_test.go @@ -227,7 +227,7 @@ func TestResolveRemoteSymlinksNoSymlinks(t *testing.T) { require.Error(t, err, "Expected error when no symlinks found") skipOnAuthError(t, err) - assert.Contains(t, err.Error(), "no symlinks found", "Should indicate no symlinks were found in path") + require.ErrorContains(t, err, "no symlinks found", "Should indicate no symlinks were found in path") } // TestDownloadFileFromGitHubSymlinkRoute verifies that downloading a nonexistent file @@ -240,7 +240,7 @@ func TestDownloadFileFromGitHubSymlinkRoute(t *testing.T) { require.Error(t, err, "Expected error for nonexistent file") skipOnAuthError(t, err) - assert.Contains(t, err.Error(), "failed to fetch file content", "Should return the original fetch failure") + require.ErrorContains(t, err, "failed to fetch file content", "Should return the original fetch failure") } // TestDownloadIncludeFromWorkflowSpecWithCache tests caching behavior diff --git a/pkg/parser/schedule_parser_test.go b/pkg/parser/schedule_parser_test.go index 58ead907040..37320652fa5 100644 --- a/pkg/parser/schedule_parser_test.go +++ b/pkg/parser/schedule_parser_test.go @@ -1081,7 +1081,7 @@ func TestParseSchedule(t *testing.T) { if tt.shouldError { require.Error(t, err, "ParseSchedule(%q) should return an error", tt.input) if tt.errorSubstring != "" { - assert.Contains(t, err.Error(), tt.errorSubstring, + require.ErrorContains(t, err, tt.errorSubstring, "error for %q should contain %q", tt.input, tt.errorSubstring) } return diff --git a/pkg/parser/sub_agent_extractor_test.go b/pkg/parser/sub_agent_extractor_test.go index 53de005a71a..79f0e2d4d97 100644 --- a/pkg/parser/sub_agent_extractor_test.go +++ b/pkg/parser/sub_agent_extractor_test.go @@ -214,8 +214,8 @@ func TestExtractInlineSubAgents_DuplicateNameError(t *testing.T) { _, _, err := ExtractInlineSubAgents(markdown) require.Error(t, err, "duplicate agent name should produce an error") - assert.Contains(t, err.Error(), "duplicate", "error should mention duplicate") - assert.Contains(t, err.Error(), "planner", "error should include the duplicate name") + require.ErrorContains(t, err, "duplicate", "error should mention duplicate") + require.ErrorContains(t, err, "planner", "error should include the duplicate name") } func TestExtractInlineSubAgents_NameVariants(t *testing.T) { diff --git a/pkg/parser/yaml_import_copilot_setup_test.go b/pkg/parser/yaml_import_copilot_setup_test.go index 09e3de88d4a..40b33223545 100644 --- a/pkg/parser/yaml_import_copilot_setup_test.go +++ b/pkg/parser/yaml_import_copilot_setup_test.go @@ -124,7 +124,7 @@ func TestExtractStepsFromCopilotSetup_MissingJob(t *testing.T) { _, err := extractStepsFromCopilotSetup(workflow) require.Error(t, err, "Should error when copilot-setup-steps job is missing") - assert.Contains(t, err.Error(), "copilot-setup-steps job not found", "Error should mention missing job") + require.ErrorContains(t, err, "copilot-setup-steps job not found", "Error should mention missing job") } func TestExtractStepsFromCopilotSetup_NoSteps(t *testing.T) { @@ -140,7 +140,7 @@ func TestExtractStepsFromCopilotSetup_NoSteps(t *testing.T) { _, err := extractStepsFromCopilotSetup(workflow) require.Error(t, err, "Should error when steps are missing") - assert.Contains(t, err.Error(), "no steps found", "Error should mention missing steps") + require.ErrorContains(t, err, "no steps found", "Error should mention missing steps") } func TestExtractStepsFromCopilotSetup_StripsCheckoutStep(t *testing.T) { diff --git a/pkg/parser/yaml_import_test.go b/pkg/parser/yaml_import_test.go index f57ac5f9944..a976d63634d 100644 --- a/pkg/parser/yaml_import_test.go +++ b/pkg/parser/yaml_import_test.go @@ -179,7 +179,7 @@ runs: _, _, err = processYAMLWorkflowImport(actionFile) require.Error(t, err, "Should reject action definition") - assert.Contains(t, err.Error(), "cannot import action definition", "Error should mention action definition") + require.ErrorContains(t, err, "cannot import action definition", "Error should mention action definition") }) t.Run("reject invalid workflow", func(t *testing.T) { @@ -192,7 +192,7 @@ description: This is not a valid workflow` _, _, err = processYAMLWorkflowImport(invalidFile) require.Error(t, err, "Should reject invalid workflow") - assert.Contains(t, err.Error(), "not a valid GitHub Actions workflow", "Error should mention invalid workflow") + require.ErrorContains(t, err, "not a valid GitHub Actions workflow", "Error should mention invalid workflow") }) } @@ -274,6 +274,6 @@ imports: _, err = ProcessImportsFromFrontmatterWithSource(result.Frontmatter, tmpDir, nil, "", "") require.Error(t, err, "Should reject .lock.yml import") - assert.Contains(t, err.Error(), "cannot import .lock.yml files", "Error should mention .lock.yml rejection") - assert.Contains(t, err.Error(), "Import the source .md file instead", "Error should suggest importing .md file") + require.ErrorContains(t, err, "cannot import .lock.yml files", "Error should mention .lock.yml rejection") + require.ErrorContains(t, err, "Import the source .md file instead", "Error should suggest importing .md file") } diff --git a/pkg/stringutil/pat_validation_test.go b/pkg/stringutil/pat_validation_test.go index 4e49a7d2e8a..081cc5d24ab 100644 --- a/pkg/stringutil/pat_validation_test.go +++ b/pkg/stringutil/pat_validation_test.go @@ -117,7 +117,7 @@ func TestValidateCopilotPAT(t *testing.T) { err := ValidateCopilotPAT(tt.token) if tt.expectError { require.Error(t, err, "should return error for invalid token") - assert.Contains(t, err.Error(), tt.errorMsg, "error message should contain expected text") + require.ErrorContains(t, err, tt.errorMsg, "error message should contain expected text") } else { assert.NoError(t, err, "should not return error for valid token") } diff --git a/pkg/workflow/activation_permissions_scope_test.go b/pkg/workflow/activation_permissions_scope_test.go index d50277d4dfb..80c91a5cee3 100644 --- a/pkg/workflow/activation_permissions_scope_test.go +++ b/pkg/workflow/activation_permissions_scope_test.go @@ -328,7 +328,7 @@ engine: copilot compiler := NewCompiler() err = compiler.CompileWorkflow(testFile) require.Error(t, err, "compilation should fail when status-comment object disables all targets") - assert.Contains(t, err.Error(), "status-comment object requires at least one target to be enabled", "error should explain invalid status-comment object configuration") + require.ErrorContains(t, err, "status-comment object requires at least one target to be enabled", "error should explain invalid status-comment object configuration") } func TestActivationPermissionsStatusCommentPullRequestsDisabled(t *testing.T) { diff --git a/pkg/workflow/agent_validation_model_test.go b/pkg/workflow/agent_validation_model_test.go index 5d36ee68ebd..30260fd5d08 100644 --- a/pkg/workflow/agent_validation_model_test.go +++ b/pkg/workflow/agent_validation_model_test.go @@ -36,7 +36,7 @@ func TestValidateUniversalLLMConsumerModel(t *testing.T) { opencodeEngine, ) require.Error(t, err, "Missing model should fail for opencode") - assert.Contains(t, err.Error(), "engine.model is required for engine 'opencode'") + require.ErrorContains(t, err, "engine.model is required for engine 'opencode'") }) t.Run("opencode requires provider/model format", func(t *testing.T) { @@ -50,7 +50,7 @@ func TestValidateUniversalLLMConsumerModel(t *testing.T) { opencodeEngine, ) require.Error(t, err, "Unqualified model should fail for opencode") - assert.Contains(t, err.Error(), "provider/model format") + require.ErrorContains(t, err, "provider/model format") }) t.Run("unsupported provider fails", func(t *testing.T) { @@ -64,7 +64,7 @@ func TestValidateUniversalLLMConsumerModel(t *testing.T) { opencodeEngine, ) require.Error(t, err, "Unsupported provider should fail") - assert.Contains(t, err.Error(), "unsupported provider") + require.ErrorContains(t, err, "unsupported provider") }) t.Run("supported provider passes", func(t *testing.T) { @@ -94,7 +94,7 @@ func TestValidatePiEngineRequirements(t *testing.T) { "github": true, }), NewPiEngine()) require.Error(t, err) - assert.Contains(t, err.Error(), "tools.github.mode: gh-proxy") + require.ErrorContains(t, err, "tools.github.mode: gh-proxy") }) t.Run("pi requires cli-proxy", func(t *testing.T) { @@ -102,7 +102,7 @@ func TestValidatePiEngineRequirements(t *testing.T) { "github": map[string]any{"mode": "gh-proxy"}, }), NewPiEngine()) require.Error(t, err) - assert.Contains(t, err.Error(), "tools.cli-proxy: true") + require.ErrorContains(t, err, "tools.cli-proxy: true") }) t.Run("valid pi tool config passes", func(t *testing.T) { diff --git a/pkg/workflow/agentic_engine_test.go b/pkg/workflow/agentic_engine_test.go index a8bd3d218ab..02c75bf99a9 100644 --- a/pkg/workflow/agentic_engine_test.go +++ b/pkg/workflow/agentic_engine_test.go @@ -113,7 +113,7 @@ func TestEngineRegistry_Register(t *testing.T) { // getDedicatedLLMGatewayPort, triggering the validation path in Register. err := registry.Register(&negativePortEngine{CodingAgentEngine: NewClaudeEngine()}) require.Error(t, err, "registering an engine with dedicatedLLMGatewayPort = -1 should return an error") - assert.Contains(t, err.Error(), "dedicatedLLMGatewayPort must be >= 0", "error message should describe the constraint") + require.ErrorContains(t, err, "dedicatedLLMGatewayPort must be >= 0", "error message should describe the constraint") assert.False(t, registry.IsValidEngine("claude"), "invalid engine should not be registered on error") }) } diff --git a/pkg/workflow/bash_anonymous_validation_test.go b/pkg/workflow/bash_anonymous_validation_test.go index 4b40e3ec7ab..03665de06ff 100644 --- a/pkg/workflow/bash_anonymous_validation_test.go +++ b/pkg/workflow/bash_anonymous_validation_test.go @@ -43,10 +43,10 @@ This is a test workflow with anonymous bash syntax. // Verify that compilation fails with the expected error require.Error(t, err, "Compilation should fail for anonymous bash syntax") - assert.Contains(t, err.Error(), "anonymous syntax 'bash:' is not supported", "Error should mention anonymous syntax") - assert.Contains(t, err.Error(), "bash: true", "Error should suggest bash: true") - assert.Contains(t, err.Error(), "bash: false", "Error should suggest bash: false") - assert.Contains(t, err.Error(), "gh aw fix", "Error should suggest using gh aw fix") + require.ErrorContains(t, err, "anonymous syntax 'bash:' is not supported", "Error should mention anonymous syntax") + require.ErrorContains(t, err, "bash: true", "Error should suggest bash: true") + require.ErrorContains(t, err, "bash: false", "Error should suggest bash: false") + require.ErrorContains(t, err, "gh aw fix", "Error should suggest using gh aw fix") } func TestCompilerAcceptsExplicitBashSyntax(t *testing.T) { diff --git a/pkg/workflow/block_scalar_expression_size_test.go b/pkg/workflow/block_scalar_expression_size_test.go index 4439107cc8a..a819efce2e8 100644 --- a/pkg/workflow/block_scalar_expression_size_test.go +++ b/pkg/workflow/block_scalar_expression_size_test.go @@ -66,8 +66,8 @@ func TestValidateBlockScalarExpressionSizes(t *testing.T) { lines := strings.Split(sb.String(), "\n") err := validateBlockScalarExpressionSizes(lines, smallMax) require.Error(t, err, "large block with expression should fail") - assert.Contains(t, err.Error(), "exceeds maximum allowed size", "error should describe the size issue") - assert.Contains(t, err.Error(), "run", "error should identify the block key") + require.ErrorContains(t, err, "exceeds maximum allowed size", "error should describe the size issue") + require.ErrorContains(t, err, "run", "error should identify the block key") }) t.Run("expression at beginning of large block fails", func(t *testing.T) { @@ -108,7 +108,7 @@ func TestValidateBlockScalarExpressionSizes(t *testing.T) { lines := strings.Split(sb.String(), "\n") err := validateBlockScalarExpressionSizes(lines, smallMax) require.Error(t, err, "folded block (>) with expression exceeding limit should fail") - assert.Contains(t, err.Error(), "exceeds maximum allowed size", "error message should describe the issue") + require.ErrorContains(t, err, "exceeds maximum allowed size", "error message should describe the issue") }) t.Run("MaxExpressionSize used by compiler validation", func(t *testing.T) { diff --git a/pkg/workflow/builtin_job_needs_integration_test.go b/pkg/workflow/builtin_job_needs_integration_test.go index 26131b5a928..f9679cf8037 100644 --- a/pkg/workflow/builtin_job_needs_integration_test.go +++ b/pkg/workflow/builtin_job_needs_integration_test.go @@ -92,5 +92,5 @@ Builtin job needs augmentation validation err := compiler.CompileWorkflow(workflowFile) require.Error(t, err) - assert.Contains(t, err.Error(), `jobs.detection.needs: unknown job "missing_job"`) + require.ErrorContains(t, err, `jobs.detection.needs: unknown job "missing_job"`) } diff --git a/pkg/workflow/cache_id_validation_test.go b/pkg/workflow/cache_id_validation_test.go index 040bc2dd4b8..34f176d34ca 100644 --- a/pkg/workflow/cache_id_validation_test.go +++ b/pkg/workflow/cache_id_validation_test.go @@ -73,7 +73,7 @@ func TestParseCacheMemoryEntry_InvalidID(t *testing.T) { _, err := compiler.extractCacheMemoryConfigFromMap(tools) require.Error(t, err, "Should reject invalid cache ID %q", tt.id) - assert.Contains(t, err.Error(), "invalid cache-memory id", "Error message should identify the problem") + require.ErrorContains(t, err, "invalid cache-memory id", "Error message should identify the problem") }) } } diff --git a/pkg/workflow/cache_integrity_test.go b/pkg/workflow/cache_integrity_test.go index f1d520823bc..d2e3384d067 100644 --- a/pkg/workflow/cache_integrity_test.go +++ b/pkg/workflow/cache_integrity_test.go @@ -709,7 +709,7 @@ func TestCacheMemoryAllowedExtensions_ValidationAndEscaping(t *testing.T) { compiler := NewCompiler() _, err = compiler.extractCacheMemoryConfig(toolsConfig) require.Error(t, err, "Should reject invalid extension at parse time") - assert.Contains(t, err.Error(), "no-leading-dot", "Error should identify the bad value") + require.ErrorContains(t, err, "no-leading-dot", "Error should identify the bad value") }) t.Run("single-quote escaping in emitted YAML", func(t *testing.T) { diff --git a/pkg/workflow/cache_key_validation_test.go b/pkg/workflow/cache_key_validation_test.go index b6aacdd7378..9f7f677d90b 100644 --- a/pkg/workflow/cache_key_validation_test.go +++ b/pkg/workflow/cache_key_validation_test.go @@ -68,7 +68,7 @@ func TestCacheKeyRunIDValidationObject(t *testing.T) { if tt.wantError { require.Error(t, err, "Should return error for key containing run_id") - assert.ErrorContains(t, err, tt.errorText, "Error should contain expected message") + require.ErrorContains(t, err, tt.errorText, "Error should contain expected message") } else { assert.NoError(t, err, "Should not return error for valid key") } @@ -117,7 +117,7 @@ func TestCacheKeyRunIDValidationArray(t *testing.T) { if tt.wantError { require.Error(t, err, "Should return error for key containing run_id") - assert.ErrorContains(t, err, tt.errorText, "Error should contain expected message") + require.ErrorContains(t, err, tt.errorText, "Error should contain expected message") } else { assert.NoError(t, err, "Should not return error for valid key") } diff --git a/pkg/workflow/cache_scope_validation_test.go b/pkg/workflow/cache_scope_validation_test.go index 345bff2a5e8..b6ec48a00c7 100644 --- a/pkg/workflow/cache_scope_validation_test.go +++ b/pkg/workflow/cache_scope_validation_test.go @@ -63,7 +63,7 @@ func TestCacheMemoryScopeValidationObject(t *testing.T) { if tt.wantError { require.Error(t, err, "Should return error for invalid scope") - assert.ErrorContains(t, err, tt.errorText, "Error should contain expected message") + require.ErrorContains(t, err, tt.errorText, "Error should contain expected message") } else { assert.NoError(t, err, "Should not return error for valid scope") } @@ -116,7 +116,7 @@ func TestCacheMemoryScopeValidationArray(t *testing.T) { if tt.wantError { require.Error(t, err, "Should return error for invalid scope") - assert.ErrorContains(t, err, tt.errorText, "Error should contain expected message") + require.ErrorContains(t, err, tt.errorText, "Error should contain expected message") } else { assert.NoError(t, err, "Should not return error for valid scope") } diff --git a/pkg/workflow/call_workflow_compilation_test.go b/pkg/workflow/call_workflow_compilation_test.go index dad931f82ca..47e32665aaf 100644 --- a/pkg/workflow/call_workflow_compilation_test.go +++ b/pkg/workflow/call_workflow_compilation_test.go @@ -565,8 +565,8 @@ safe-outputs: compiler := NewCompiler(WithVersion("1.0.0")) err = compiler.CompileWorkflow(gatewayFile) require.Error(t, err, "Should fail when worker lacks workflow_call trigger") - assert.Contains(t, err.Error(), "workflow_call", "Error should mention workflow_call") - assert.Contains(t, err.Error(), "no-call-worker", "Error should mention the workflow name") + require.ErrorContains(t, err, "workflow_call", "Error should mention workflow_call") + require.ErrorContains(t, err, "no-call-worker", "Error should mention the workflow name") } // TestCallWorkflowCompile_ValidationFails_SelfReference verifies that compilation fails @@ -595,7 +595,7 @@ safe-outputs: compiler := NewCompiler(WithVersion("1.0.0")) err := compiler.CompileWorkflow(gatewayFile) require.Error(t, err, "Should fail for self-reference") - assert.Contains(t, err.Error(), "self-reference", "Error should mention self-reference") + require.ErrorContains(t, err, "self-reference", "Error should mention self-reference") } // TestCallWorkflowCompile_ValidationFails_WorkerNotFound verifies compilation fails @@ -624,8 +624,8 @@ safe-outputs: compiler := NewCompiler(WithVersion("1.0.0")) err := compiler.CompileWorkflow(gatewayFile) require.Error(t, err, "Should fail for missing worker") - assert.Contains(t, err.Error(), "not found", "Error should mention not found") - assert.Contains(t, err.Error(), "nonexistent-worker", "Error should name the missing workflow") + require.ErrorContains(t, err, "not found", "Error should mention not found") + require.ErrorContains(t, err, "nonexistent-worker", "Error should name the missing workflow") } // TestCallWorkflowCompile_ValidationFails_DuplicateWorkflow verifies that compilation @@ -659,8 +659,8 @@ safe-outputs: compiler := NewCompiler(WithVersion("1.0.0")) err := compiler.CompileWorkflow(gatewayFile) require.Error(t, err, "Should fail for duplicate workflow name") - assert.Contains(t, err.Error(), "duplicate", "Error should mention duplicate") - assert.Contains(t, err.Error(), "worker-a", "Error should name the duplicate workflow") + require.ErrorContains(t, err, "duplicate", "Error should mention duplicate") + require.ErrorContains(t, err, "worker-a", "Error should name the duplicate workflow") } // TestCallWorkflowCompile_MDSourceWorker tests that compilation succeeds when the diff --git a/pkg/workflow/call_workflow_validation_test.go b/pkg/workflow/call_workflow_validation_test.go index 307c1bef611..cdccaf57454 100644 --- a/pkg/workflow/call_workflow_validation_test.go +++ b/pkg/workflow/call_workflow_validation_test.go @@ -35,7 +35,7 @@ func TestValidateCallWorkflow_EmptyList(t *testing.T) { err = compiler.validateCallWorkflow(workflowData, gatewayFile) require.Error(t, err, "Validation should fail for empty workflows list") - assert.Contains(t, err.Error(), "must specify at least one workflow", "Should mention the requirement") + require.ErrorContains(t, err, "must specify at least one workflow", "Should mention the requirement") } // TestValidateCallWorkflow_NoConfig tests that nil config passes validation @@ -74,8 +74,8 @@ func TestValidateCallWorkflow_SelfReference(t *testing.T) { err = compiler.validateCallWorkflow(workflowData, gatewayFile) require.Error(t, err, "Self-reference should fail validation") - assert.Contains(t, err.Error(), "self-reference not allowed", "Should mention self-reference") - assert.Contains(t, err.Error(), "gateway", "Should mention the workflow name") + require.ErrorContains(t, err, "self-reference not allowed", "Should mention self-reference") + require.ErrorContains(t, err, "gateway", "Should mention the workflow name") } // TestValidateCallWorkflow_WorkflowNotFound tests that a missing workflow fails validation @@ -102,8 +102,8 @@ func TestValidateCallWorkflow_WorkflowNotFound(t *testing.T) { err = compiler.validateCallWorkflow(workflowData, gatewayFile) require.Error(t, err, "Missing workflow should fail validation") - assert.Contains(t, err.Error(), "not found", "Should mention workflow not found") - assert.Contains(t, err.Error(), "nonexistent-worker", "Should mention the workflow name") + require.ErrorContains(t, err, "not found", "Should mention workflow not found") + require.ErrorContains(t, err, "nonexistent-worker", "Should mention the workflow name") } // TestValidateCallWorkflow_WorkflowWithoutWorkflowCall tests that a workflow missing @@ -148,8 +148,8 @@ jobs: err = compiler.validateCallWorkflow(workflowData, gatewayFile) require.Error(t, err, "Worker without workflow_call should fail validation") - assert.Contains(t, err.Error(), "workflow_call", "Should mention workflow_call trigger") - assert.Contains(t, err.Error(), "worker-a", "Should mention the workflow name") + require.ErrorContains(t, err, "workflow_call", "Should mention workflow_call trigger") + require.ErrorContains(t, err, "worker-a", "Should mention the workflow name") } // TestValidateCallWorkflow_ValidWorkflow tests that a valid worker passes validation diff --git a/pkg/workflow/checkout_manager_test.go b/pkg/workflow/checkout_manager_test.go index 9ccdaaf5233..4b332845982 100644 --- a/pkg/workflow/checkout_manager_test.go +++ b/pkg/workflow/checkout_manager_test.go @@ -427,7 +427,7 @@ func TestParseCheckoutConfigs(t *testing.T) { } _, err := ParseCheckoutConfigs(raw) require.Error(t, err) - assert.Contains(t, err.Error(), "checkout.fetch-depth must be >= 0") + require.ErrorContains(t, err, "checkout.fetch-depth must be >= 0") } }) @@ -516,7 +516,7 @@ func TestParseCheckoutConfigs(t *testing.T) { } _, err := ParseCheckoutConfigs(raw) require.Error(t, err, "safe-output-github-app should be rejected") - assert.Contains(t, err.Error(), "checkout.safe-output-github-app is not supported; use checkout.safe-outputs-github-app") + require.ErrorContains(t, err, "checkout.safe-output-github-app is not supported; use checkout.safe-outputs-github-app") }) t.Run("github-token and github-app are mutually exclusive", func(t *testing.T) { @@ -529,7 +529,7 @@ func TestParseCheckoutConfigs(t *testing.T) { } _, err := ParseCheckoutConfigs(raw) require.Error(t, err, "github-token and github-app together should return error") - assert.Contains(t, err.Error(), "mutually exclusive", "error should mention mutual exclusivity") + require.ErrorContains(t, err, "mutually exclusive", "error should mention mutual exclusivity") }) t.Run("github-app config missing app-id returns error", func(t *testing.T) { @@ -540,7 +540,7 @@ func TestParseCheckoutConfigs(t *testing.T) { } _, err := ParseCheckoutConfigs(raw) require.Error(t, err, "github-app without app-id should return error") - assert.Contains(t, err.Error(), "client-id (or app-id) and private-key") + require.ErrorContains(t, err, "client-id (or app-id) and private-key") }) t.Run("github-app config missing private-key returns error", func(t *testing.T) { @@ -551,7 +551,7 @@ func TestParseCheckoutConfigs(t *testing.T) { } _, err := ParseCheckoutConfigs(raw) require.Error(t, err, "github-app without private-key should return error") - assert.Contains(t, err.Error(), "client-id (or app-id) and private-key") + require.ErrorContains(t, err, "client-id (or app-id) and private-key") }) t.Run("github-app must be an object", func(t *testing.T) { @@ -560,7 +560,7 @@ func TestParseCheckoutConfigs(t *testing.T) { } _, err := ParseCheckoutConfigs(raw) require.Error(t, err, "non-object github-app should return error") - assert.Contains(t, err.Error(), "checkout.github-app must be an object") + require.ErrorContains(t, err, "checkout.github-app must be an object") }) t.Run("array of objects", func(t *testing.T) { @@ -700,7 +700,7 @@ func TestCheckoutCurrentFlag(t *testing.T) { } _, err := ParseCheckoutConfigs(raw) require.Error(t, err, "multiple current: true should return error") - assert.Contains(t, err.Error(), "only one checkout target may have current: true", "error should mention the constraint") + require.ErrorContains(t, err, "only one checkout target may have current: true", "error should mention the constraint") }) t.Run("single current: true in array is valid", func(t *testing.T) { @@ -1477,7 +1477,7 @@ func TestWikiCheckout(t *testing.T) { } _, err := ParseCheckoutConfigs(raw) require.Error(t, err, "non-boolean wiki should return error") - assert.Contains(t, err.Error(), "checkout.wiki must be a boolean", "error message should mention wiki") + require.ErrorContains(t, err, "checkout.wiki must be a boolean", "error message should mention wiki") }) t.Run("parse force-clean-git-credentials true", func(t *testing.T) { @@ -1496,7 +1496,7 @@ func TestWikiCheckout(t *testing.T) { } _, err := ParseCheckoutConfigs(raw) require.Error(t, err, "non-boolean force-clean-git-credentials should return error") - assert.Contains(t, err.Error(), "checkout.force-clean-git-credentials must be a boolean", "error message should mention force-clean-git-credentials") + require.ErrorContains(t, err, "checkout.force-clean-git-credentials must be a boolean", "error message should mention force-clean-git-credentials") }) t.Run("wiki and non-wiki checkouts of same repo and path are not merged", func(t *testing.T) { diff --git a/pkg/workflow/compiler_activation_job_test.go b/pkg/workflow/compiler_activation_job_test.go index f819dc61cf6..77030dd7aeb 100644 --- a/pkg/workflow/compiler_activation_job_test.go +++ b/pkg/workflow/compiler_activation_job_test.go @@ -1081,7 +1081,7 @@ func TestBuildActivationJobWrapsRepositoryStepErrors(t *testing.T) { _, err := compiler.buildActivationJob(data, false, "", "test.lock.yml") require.Error(t, err, "buildActivationJob should return an error for a malformed model") - assert.Contains(t, err.Error(), "failed to add activation repository and output steps:", + require.ErrorContains(t, err, "failed to add activation repository and output steps:", "error should be wrapped with the repository-steps context prefix") } @@ -1111,7 +1111,7 @@ func TestBuildActivationJobWrapsPermissionsErrors(t *testing.T) { _, err := compiler.buildActivationJob(data, false, "", "test.lock.yml") require.Error(t, err, "buildActivationJob should return an error for write gh commands in activation pre-steps") - assert.Contains(t, err.Error(), "failed to build activation permissions:", + require.ErrorContains(t, err, "failed to build activation permissions:", "error should be wrapped with the permissions context prefix") } diff --git a/pkg/workflow/compiler_activation_jobs_test.go b/pkg/workflow/compiler_activation_jobs_test.go index 16c9e13ba5f..649a4632f6b 100644 --- a/pkg/workflow/compiler_activation_jobs_test.go +++ b/pkg/workflow/compiler_activation_jobs_test.go @@ -539,7 +539,7 @@ func TestExtractPreActivationCustomFields_InvalidSteps(t *testing.T) { steps, outputs, err := compiler.extractPreActivationCustomFields(jobs) require.Error(t, err, "Should return error for invalid steps format") - assert.Contains(t, err.Error(), "must be an array", "Error should mention array requirement") + require.ErrorContains(t, err, "must be an array", "Error should mention array requirement") assert.Empty(t, steps, "Should have no steps with invalid format") assert.Empty(t, outputs, "Should have no outputs with invalid format") } diff --git a/pkg/workflow/compiler_custom_job_memory_test.go b/pkg/workflow/compiler_custom_job_memory_test.go index ac699dec829..1023115dbf5 100644 --- a/pkg/workflow/compiler_custom_job_memory_test.go +++ b/pkg/workflow/compiler_custom_job_memory_test.go @@ -314,7 +314,7 @@ jobs: compiler := NewCompiler() err := compiler.CompileWorkflow(testFile) require.Error(t, err, "expected compilation to fail") - assert.Contains(t, err.Error(), "no memory stores are configured in tools") + require.ErrorContains(t, err, "no memory stores are configured in tools") } // TestCustomJobRestoreMemoryOnlyEmitsRestoreSteps verifies that when restore-memory diff --git a/pkg/workflow/compiler_custom_jobs_test.go b/pkg/workflow/compiler_custom_jobs_test.go index 778ebd65078..45e2066a72c 100644 --- a/pkg/workflow/compiler_custom_jobs_test.go +++ b/pkg/workflow/compiler_custom_jobs_test.go @@ -199,7 +199,7 @@ func TestBuildCustomJob_InvalidTimeoutMinutesError(t *testing.T) { ) require.Error(t, err) - assert.Contains(t, err.Error(), "timeout-minutes") + require.ErrorContains(t, err, "timeout-minutes") } func TestBuildCustomJob_UsesReusableWorkflow(t *testing.T) { @@ -333,7 +333,7 @@ func TestConfigureCustomJobSteps_InvalidStepsType(t *testing.T) { err := compiler.configureCustomJobSteps(job, "my-job", configMap, data) require.Error(t, err) - assert.Contains(t, err.Error(), "steps") + require.ErrorContains(t, err, "steps") } func TestConfigureCustomJobSteps_InvalidPreStepsType(t *testing.T) { @@ -347,7 +347,7 @@ func TestConfigureCustomJobSteps_InvalidPreStepsType(t *testing.T) { err := compiler.configureCustomJobSteps(job, "my-job", configMap, data) require.Error(t, err) - assert.Contains(t, err.Error(), "pre-steps") + require.ErrorContains(t, err, "pre-steps") } func TestConfigureCustomJobSteps_InvalidSetupStepsType(t *testing.T) { @@ -361,7 +361,7 @@ func TestConfigureCustomJobSteps_InvalidSetupStepsType(t *testing.T) { err := compiler.configureCustomJobSteps(job, "my-job", configMap, data) require.Error(t, err) - assert.Contains(t, err.Error(), "setup-steps") + require.ErrorContains(t, err, "setup-steps") } // ======================================== @@ -509,7 +509,7 @@ func TestApplyBuiltinJobNeedsAugmentations_UnknownJobError(t *testing.T) { err := compiler.applyBuiltinJobNeedsAugmentations(data) require.Error(t, err) - assert.Contains(t, err.Error(), "unknown job") + require.ErrorContains(t, err, "unknown job") } func TestApplyBuiltinJobNeedsAugmentations_SelfReferenceError(t *testing.T) { @@ -528,7 +528,7 @@ func TestApplyBuiltinJobNeedsAugmentations_SelfReferenceError(t *testing.T) { err := compiler.applyBuiltinJobNeedsAugmentations(data) require.Error(t, err) - assert.Contains(t, err.Error(), "cannot depend on itself") + require.ErrorContains(t, err, "cannot depend on itself") } func TestApplyBuiltinJobNeedsAugmentations_TargetJobNotInManagerError(t *testing.T) { @@ -548,7 +548,7 @@ func TestApplyBuiltinJobNeedsAugmentations_TargetJobNotInManagerError(t *testing err := compiler.applyBuiltinJobNeedsAugmentations(data) require.Error(t, err) - assert.Contains(t, err.Error(), "cannot augment") + require.ErrorContains(t, err, "cannot augment") } func TestApplyBuiltinJobNeedsAugmentations_InvalidConfigNotMap(t *testing.T) { @@ -563,7 +563,7 @@ func TestApplyBuiltinJobNeedsAugmentations_InvalidConfigNotMap(t *testing.T) { err := compiler.applyBuiltinJobNeedsAugmentations(data) require.Error(t, err) - assert.Contains(t, err.Error(), "must be an object") + require.ErrorContains(t, err, "must be an object") } func TestApplyBuiltinJobNeedsAugmentations_HyphenAliasNormalized(t *testing.T) { @@ -819,7 +819,7 @@ func TestExtractCustomJobTimeoutMinutes(t *testing.T) { err := extractCustomJobTimeoutMinutes(job, "test-job", tt.configMap) if tt.expectError { require.Error(t, err) - assert.Contains(t, err.Error(), "timeout-minutes") + require.ErrorContains(t, err, "timeout-minutes") } else { require.NoError(t, err) assert.Equal(t, tt.expectedTimeout, job.TimeoutMinutes) @@ -1170,7 +1170,7 @@ func TestValidateRestrictedBuiltinSetupSteps(t *testing.T) { err := validateRestrictedBuiltinSetupSteps(tt.jobName, tt.hasSetupSteps) if tt.expectError { require.Error(t, err) - assert.Contains(t, err.Error(), "setup-steps") + require.ErrorContains(t, err, "setup-steps") } else { require.NoError(t, err) } @@ -1261,5 +1261,5 @@ func TestConfigureCustomReusableWorkflow_RestoreMemoryNotSupported(t *testing.T) err := configureCustomReusableWorkflow(job, "call-worker", "./.github/workflows/worker.yml", configMap) require.Error(t, err) - assert.Contains(t, err.Error(), "restore-memory") + require.ErrorContains(t, err, "restore-memory") } diff --git a/pkg/workflow/compiler_error_formatting_test.go b/pkg/workflow/compiler_error_formatting_test.go index 96e796fa87a..1bbf75fd723 100644 --- a/pkg/workflow/compiler_error_formatting_test.go +++ b/pkg/workflow/compiler_error_formatting_test.go @@ -129,8 +129,8 @@ func TestFormatCompilerError_ErrorVsWarning(t *testing.T) { require.Error(t, errorErr) require.Error(t, warningErr) - assert.Contains(t, errorErr.Error(), "error", "Error type should be present") - assert.Contains(t, warningErr.Error(), "warning", "Warning type should be present") + require.ErrorContains(t, errorErr, "error", "Error type should be present") + require.ErrorContains(t, warningErr, "warning", "Warning type should be present") // Ensure they produce different outputs assert.NotEqual(t, errorErr.Error(), warningErr.Error(), "Error and warning should have different outputs") @@ -194,8 +194,8 @@ func TestFormatCompilerError_ErrorWrapping(t *testing.T) { require.ErrorIs(t, wrappedErr, underlyingErr, "Should preserve error chain with %w") // Verify formatted message is in the error string - assert.Contains(t, wrappedErr.Error(), "test.md") - assert.Contains(t, wrappedErr.Error(), "validation failed") + require.ErrorContains(t, wrappedErr, "test.md") + require.ErrorContains(t, wrappedErr, "validation failed") // Verify the formatted string does NOT include the cause text (no duplication) assert.NotContains(t, wrappedErr.Error(), "underlying validation error", "Error() should not duplicate cause text") } @@ -228,8 +228,8 @@ func TestFormatCompilerError_NilCause(t *testing.T) { require.Error(t, err) // Verify error message contains expected content - assert.Contains(t, err.Error(), "test.md") - assert.Contains(t, err.Error(), "validation error") + require.ErrorContains(t, err, "test.md") + require.ErrorContains(t, err, "validation error") // Verify it's a new error (not wrapping anything) // This is a validation error, so it should not wrap diff --git a/pkg/workflow/compiler_experiments_test.go b/pkg/workflow/compiler_experiments_test.go index ba7329e3b39..8fdee0e4d93 100644 --- a/pkg/workflow/compiler_experiments_test.go +++ b/pkg/workflow/compiler_experiments_test.go @@ -616,7 +616,7 @@ func TestValidateExperimentMetricReferences(t *testing.T) { return } require.Error(t, err) - assert.Contains(t, err.Error(), tt.wantErr) + require.ErrorContains(t, err, tt.wantErr) }) } } diff --git a/pkg/workflow/compiler_main_job_helpers_test.go b/pkg/workflow/compiler_main_job_helpers_test.go index c6cfa1aa756..fc275938286 100644 --- a/pkg/workflow/compiler_main_job_helpers_test.go +++ b/pkg/workflow/compiler_main_job_helpers_test.go @@ -284,8 +284,8 @@ func TestBuildMainJobPermissions(t *testing.T) { } _, err := c.buildMainJobPermissions(data) require.Error(t, err) - assert.Contains(t, err.Error(), "write operations are not permitted") - assert.Contains(t, err.Error(), "gh issue create") + require.ErrorContains(t, err, "write operations are not permitted") + require.ErrorContains(t, err, "gh issue create") }) t.Run("explicit empty permissions block skips inference", func(t *testing.T) { diff --git a/pkg/workflow/compiler_orchestrator_engine_test.go b/pkg/workflow/compiler_orchestrator_engine_test.go index a94f80cd878..9c964e96c47 100644 --- a/pkg/workflow/compiler_orchestrator_engine_test.go +++ b/pkg/workflow/compiler_orchestrator_engine_test.go @@ -372,7 +372,7 @@ engine: invalid-engine-name result, err := compiler.setupEngineAndImports(frontmatterResult, testFile, content, tmpDir) require.Error(t, err, "Invalid engine should cause error") assert.Nil(t, result) - assert.Contains(t, err.Error(), "invalid-engine-name") + require.ErrorContains(t, err, "invalid-engine-name") } // TestSetupEngineAndImports_StrictModeHandling tests strict mode state management @@ -646,7 +646,7 @@ imports: // Should error due to conflicting engines require.Error(t, err, "Conflicting engines should cause error") assert.Nil(t, result) - assert.Contains(t, err.Error(), "engine") + require.ErrorContains(t, err, "engine") } // TestSetupEngineAndImports_FirewallEnablement tests automatic firewall enablement diff --git a/pkg/workflow/compiler_orchestrator_frontmatter_test.go b/pkg/workflow/compiler_orchestrator_frontmatter_test.go index f090db9a1f3..3855e8985c5 100644 --- a/pkg/workflow/compiler_orchestrator_frontmatter_test.go +++ b/pkg/workflow/compiler_orchestrator_frontmatter_test.go @@ -98,8 +98,8 @@ Content here require.Error(t, err, "Using 'triggers:' instead of 'on:' should cause error") assert.Nil(t, result) - assert.Contains(t, err.Error(), "'triggers:'", "Error should mention the invalid key") - assert.Contains(t, err.Error(), "'on:'", "Error should mention the correct key") + require.ErrorContains(t, err, "'triggers:'", "Error should mention the invalid key") + require.ErrorContains(t, err, "'on:'", "Error should mention the correct key") } // TestParseFrontmatterSection_MissingFrontmatter tests error for no frontmatter @@ -119,7 +119,7 @@ Just markdown content require.Error(t, err, "Missing frontmatter should cause error") assert.Nil(t, result) - assert.Contains(t, err.Error(), "frontmatter") + require.ErrorContains(t, err, "frontmatter") } // TestParseFrontmatterSection_InvalidYAML tests YAML parsing errors @@ -163,7 +163,7 @@ engine: copilot require.Error(t, err, "Main workflow needs markdown content") assert.Nil(t, result) - assert.Contains(t, err.Error(), "markdown content") + require.ErrorContains(t, err, "markdown content") } // TestParseFrontmatterSection_PathTraversal tests path cleaning diff --git a/pkg/workflow/compiler_orchestrator_test.go b/pkg/workflow/compiler_orchestrator_test.go index 227149b2d23..b59b95031ae 100644 --- a/pkg/workflow/compiler_orchestrator_test.go +++ b/pkg/workflow/compiler_orchestrator_test.go @@ -91,7 +91,7 @@ This file has no frontmatter section. require.Error(t, err, "Should error when frontmatter is missing") assert.Nil(t, workflowData) - assert.Contains(t, err.Error(), "frontmatter", "Error should mention frontmatter") + require.ErrorContains(t, err, "frontmatter", "Error should mention frontmatter") } // TestParseWorkflowFile_InvalidYAML tests error handling for invalid YAML frontmatter @@ -156,7 +156,7 @@ engine: copilot require.Error(t, err, "Main workflow without markdown content should error") assert.Nil(t, workflowData) - assert.Contains(t, err.Error(), "markdown content", "Error should mention markdown content") + require.ErrorContains(t, err, "markdown content", "Error should mention markdown content") } // TestParseWorkflowFile_EngineExtraction tests engine config extraction diff --git a/pkg/workflow/compiler_orchestrator_tools_test.go b/pkg/workflow/compiler_orchestrator_tools_test.go index d18232691c4..e2aafaa9384 100644 --- a/pkg/workflow/compiler_orchestrator_tools_test.go +++ b/pkg/workflow/compiler_orchestrator_tools_test.go @@ -342,7 +342,7 @@ tools: // Should error with invalid timeout require.Error(t, err, "Invalid timeout should cause error") assert.Nil(t, result) - assert.Contains(t, err.Error(), "timeout") + require.ErrorContains(t, err, "timeout") } // TestProcessToolsAndMarkdown_MCPValidation tests MCP config validation @@ -716,7 +716,7 @@ engine: copilot // Missing includes may be handled gracefully in some cases // This test verifies the function completes if err != nil { - assert.Contains(t, err.Error(), "nonexistent", "Error should mention missing file") + assert.ErrorContains(t, err, "nonexistent", "Error should mention missing file") } else { assert.NotNil(t, result) } diff --git a/pkg/workflow/compiler_orchestrator_workflow_test.go b/pkg/workflow/compiler_orchestrator_workflow_test.go index 34cf45a2121..5053f8cd5fc 100644 --- a/pkg/workflow/compiler_orchestrator_workflow_test.go +++ b/pkg/workflow/compiler_orchestrator_workflow_test.go @@ -317,7 +317,7 @@ func TestValidateWorkflowEngineSettings_PreservesLegacyErrorOrder(t *testing.T) err := compiler.validateWorkflowEngineSettings("workflow.md", workflowData) require.Error(t, err) - assert.Contains(t, err.Error(), "workflow.md: strict mode: run-install-scripts: true is set") + require.ErrorContains(t, err, "workflow.md: strict mode: run-install-scripts: true is set") assert.NotContains(t, err.Error(), "engine.harness") } @@ -338,7 +338,7 @@ func TestValidateWorkflowEngineSettings_LSPRequiresCopilot(t *testing.T) { err := compiler.validateWorkflowEngineSettings("workflow.md", workflowData) require.Error(t, err) - assert.Contains(t, err.Error(), "workflow.md: lsp is currently only supported for engine: copilot") + require.ErrorContains(t, err, "workflow.md: lsp is currently only supported for engine: copilot") } func TestMergeRawOTLPEndpoints_DedupesAndCountsSources(t *testing.T) { @@ -1460,7 +1460,7 @@ engine: copilot require.Error(t, err, "Should error for %s", tt.name) assert.Nil(t, workflowData) if tt.expectError != "" { - assert.Contains(t, err.Error(), tt.expectError) + require.ErrorContains(t, err, tt.expectError) } }) } @@ -1724,7 +1724,7 @@ engine: invalid-engine-that-does-not-exist require.Error(t, err, "Should error with invalid engine") assert.Nil(t, workflowData) - assert.Contains(t, err.Error(), "invalid-engine-that-does-not-exist") + require.ErrorContains(t, err, "invalid-engine-that-does-not-exist") } // TestParseWorkflowFile_ErrorPropagationFromToolsProcessing tests error propagation from tools phase @@ -1751,7 +1751,7 @@ tools: require.Error(t, err, "Should error with invalid tools timeout") assert.Nil(t, workflowData) - assert.Contains(t, err.Error(), "timeout") + require.ErrorContains(t, err, "timeout") } // TestParseWorkflowFile_ActionCacheAndResolverSetup tests action cache and resolver are properly set @@ -1870,7 +1870,7 @@ func TestProcessAndMergeSteps_InvalidYAML_MergedSteps(t *testing.T) { // Malformed MergedSteps YAML must propagate as an error err := compiler.processAndMergeSteps(frontmatter, workflowData, importsResult) require.Error(t, err, "malformed MergedSteps YAML should return an error") - assert.Contains(t, err.Error(), "failed to parse imported steps") + require.ErrorContains(t, err, "failed to parse imported steps") } // TestProcessAndMergePreSteps_InvalidYAML tests that malformed imported pre-steps YAML returns an error @@ -1891,7 +1891,7 @@ func TestProcessAndMergePreSteps_InvalidYAML(t *testing.T) { err := compiler.processAndMergePreSteps(frontmatter, workflowData, importsResult) require.Error(t, err, "malformed imported pre-steps YAML should return an error") - assert.Contains(t, err.Error(), "failed to parse imported pre-steps") + require.ErrorContains(t, err, "failed to parse imported pre-steps") } // TestProcessAndMergePreAgentSteps_InvalidYAML tests that malformed imported pre-agent-steps YAML returns an error @@ -1912,7 +1912,7 @@ func TestProcessAndMergePreAgentSteps_InvalidYAML(t *testing.T) { err := compiler.processAndMergePreAgentSteps(frontmatter, workflowData, importsResult) require.Error(t, err, "malformed imported pre-agent-steps YAML should return an error") - assert.Contains(t, err.Error(), "failed to parse imported pre-agent-steps") + require.ErrorContains(t, err, "failed to parse imported pre-agent-steps") } // TestProcessAndMergePostSteps_InvalidYAML tests that malformed imported post-steps YAML returns an error @@ -1933,7 +1933,7 @@ func TestProcessAndMergePostSteps_InvalidYAML(t *testing.T) { err := compiler.processAndMergePostSteps(frontmatter, workflowData, importsResult) require.Error(t, err, "malformed imported post-steps YAML should return an error") - assert.Contains(t, err.Error(), "failed to parse imported post-steps") + require.ErrorContains(t, err, "failed to parse imported post-steps") } // TestProcessAndMergeServices_EmptyImportedServices tests handling of empty imported services diff --git a/pkg/workflow/compiler_string_api_test.go b/pkg/workflow/compiler_string_api_test.go index a73001a4fea..fe0d0737c57 100644 --- a/pkg/workflow/compiler_string_api_test.go +++ b/pkg/workflow/compiler_string_api_test.go @@ -47,7 +47,7 @@ No frontmatter here. _, err := compiler.ParseWorkflowString(markdown, "workflow.md") require.Error(t, err) - assert.Contains(t, err.Error(), "frontmatter") + require.ErrorContains(t, err, "frontmatter") } func TestParseWorkflowString_InvalidFrontmatterYAML(t *testing.T) { diff --git a/pkg/workflow/compiler_template_injection_both_paths_test.go b/pkg/workflow/compiler_template_injection_both_paths_test.go index a33a976acb8..fff241728b4 100644 --- a/pkg/workflow/compiler_template_injection_both_paths_test.go +++ b/pkg/workflow/compiler_template_injection_both_paths_test.go @@ -126,7 +126,7 @@ jobs: // Path A: parsedWorkflow != nil (pre-parsed for schema validation). err := compiler.validateTemplateInjection(unsafeYAML, lockFile, markdownPath, parseYAML(t, unsafeYAML)) require.Error(t, err, "should detect template injection with pre-parsed YAML") - assert.Contains(t, err.Error(), "github.event", "error should mention the unsafe context") + require.ErrorContains(t, err, "github.event", "error should mention the unsafe context") }) t.Run("Path A - schema enabled - safe expression passes", func(t *testing.T) { @@ -138,7 +138,7 @@ jobs: // Path B: parsedWorkflow == nil (schema validation skipped). err := compiler.validateTemplateInjection(unsafeYAML, lockFile, markdownPath, nil) require.Error(t, err, "should detect template injection via text scan fallback") - assert.Contains(t, err.Error(), "github.event", "error should mention the unsafe context") + require.ErrorContains(t, err, "github.event", "error should mention the unsafe context") }) t.Run("Path B - schema disabled - safe expression passes", func(t *testing.T) { @@ -149,15 +149,15 @@ jobs: t.Run("Path A - schema enabled - run expression regression detected", func(t *testing.T) { err := compiler.validateTemplateInjection(regressionYAML, lockFile, markdownPath, parseYAML(t, regressionYAML)) require.Error(t, err, "should detect raw GitHub Actions expression in run script") - assert.Contains(t, err.Error(), "compiler regression detected") - assert.Contains(t, err.Error(), "github.token") + require.ErrorContains(t, err, "compiler regression detected") + require.ErrorContains(t, err, "github.token") }) t.Run("Path B - schema disabled - run expression regression detected", func(t *testing.T) { err := compiler.validateTemplateInjection(regressionYAML, lockFile, markdownPath, nil) require.Error(t, err, "should detect raw GitHub Actions expression in run script") - assert.Contains(t, err.Error(), "compiler regression detected") - assert.Contains(t, err.Error(), "github.token") + require.ErrorContains(t, err, "compiler regression detected") + require.ErrorContains(t, err, "github.token") }) t.Run("Path A - schema enabled - allowed generated run expression passes", func(t *testing.T) { @@ -173,8 +173,8 @@ jobs: t.Run("Path B - schema disabled - flow-style regression detected alongside allowed expression", func(t *testing.T) { err := compiler.validateTemplateInjection(flowStyleRegressionYAML, lockFile, markdownPath, nil) require.Error(t, err, "flow-style run expressions should still be detected in the fallback path") - assert.Contains(t, err.Error(), "compiler regression detected") - assert.Contains(t, err.Error(), "github.actor") + require.ErrorContains(t, err, "compiler regression detected") + require.ErrorContains(t, err, "github.actor") }) t.Run("Path A - schema enabled - heredoc expression passes", func(t *testing.T) { @@ -203,8 +203,8 @@ jobs: require.Error(t, errA, "Path A must report an error") require.Error(t, errB, "Path B must report an error") - assert.Contains(t, errA.Error(), "github.event", "Path A error should identify the context") - assert.Contains(t, errB.Error(), "github.event", "Path B error should identify the context") + require.ErrorContains(t, errA, "github.event", "Path A error should identify the context") + require.ErrorContains(t, errB, "github.event", "Path B error should identify the context") }) t.Run("both paths agree on safe YAML", func(t *testing.T) { diff --git a/pkg/workflow/compiler_test.go b/pkg/workflow/compiler_test.go index b28da91e25f..6ab62cdc20f 100644 --- a/pkg/workflow/compiler_test.go +++ b/pkg/workflow/compiler_test.go @@ -174,7 +174,7 @@ Content with and & symbols. } else { require.Error(t, err, "Expected error for %s", tt.name) if tt.errorContains != "" { - assert.Contains(t, err.Error(), tt.errorContains, "Error should contain expected message") + require.ErrorContains(t, err, tt.errorContains, "Error should contain expected message") } } }) @@ -259,7 +259,7 @@ engine: copilot if tt.shouldError { require.Error(t, err, "Expected error for %s", tt.name) if tt.errorContains != "" { - assert.Contains(t, err.Error(), tt.errorContains, "Error should contain expected message") + require.ErrorContains(t, err, tt.errorContains, "Error should contain expected message") } } else { if err != nil { @@ -637,7 +637,7 @@ func TestValidateWorkflowData(t *testing.T) { if tt.shouldError { require.Error(t, err, "Expected validation to fail") if tt.errorContains != "" { - assert.Contains(t, err.Error(), tt.errorContains, "Error should contain expected message") + require.ErrorContains(t, err, tt.errorContains, "Error should contain expected message") } } else { if err != nil { @@ -730,7 +730,7 @@ func TestGenerateAndValidateYAML(t *testing.T) { if tt.shouldError { require.Error(t, err, "Expected YAML generation to fail") if tt.errorContains != "" { - assert.Contains(t, err.Error(), tt.errorContains, "Error should contain expected message") + require.ErrorContains(t, err, tt.errorContains, "Error should contain expected message") } } else { if err != nil { @@ -1048,7 +1048,7 @@ func TestReadLockFileFromHEAD_GitStates(t *testing.T) { _, err := compiler.readLockFileFromHEAD(filepath.Join(os.TempDir(), "nonexistent.lock.yml")) require.Error(t, err, "Missing git root should return an error") - assert.Contains(t, err.Error(), "git root not available", "Error should explain missing git root") + require.ErrorContains(t, err, "git root not available", "Error should explain missing git root") }) t.Run("returns committed content from HEAD", func(t *testing.T) { @@ -1090,6 +1090,6 @@ func TestReadLockFileFromHEAD_GitStates(t *testing.T) { lockFile := filepath.Join(repoDir, "workflow.lock.yml") _, err := compiler.readLockFileFromHEAD(lockFile) require.Error(t, err, "Reading a non-existent lock file from HEAD should fail") - assert.Contains(t, err.Error(), "not found in HEAD commit", "Error should indicate file is absent in HEAD") + require.ErrorContains(t, err, "not found in HEAD commit", "Error should indicate file is absent in HEAD") }) } diff --git a/pkg/workflow/compiler_threat_detection_formal_test.go b/pkg/workflow/compiler_threat_detection_formal_test.go index 0b7a94c5885..29416b4c193 100644 --- a/pkg/workflow/compiler_threat_detection_formal_test.go +++ b/pkg/workflow/compiler_threat_detection_formal_test.go @@ -5,7 +5,6 @@ package workflow import ( "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -17,7 +16,7 @@ func TestFormal_CTR016_NilManifestSkipsEnforcement(t *testing.T) { func TestFormal_CTR016_EmptyManifestRejectsNewSecret(t *testing.T) { err := EnforceSafeUpdate(&GHAWManifest{Version: currentGHAWManifestVersion}, []string{"MY_SECRET"}, nil, "", false, false, false, false) require.Error(t, err) - assert.Contains(t, err.Error(), "MY_SECRET") + require.ErrorContains(t, err, "MY_SECRET") } func TestFormal_CTR016_GitHubTokenExempt_BareForm(t *testing.T) { @@ -45,15 +44,15 @@ func TestFormal_CTR016_NewActionDriftRejected(t *testing.T) { manifest := &GHAWManifest{Version: currentGHAWManifestVersion, Actions: []GHAWManifestAction{{Repo: "actions/checkout", SHA: "abc1234", Version: "v4"}}} err := EnforceSafeUpdate(manifest, nil, []string{"actions/checkout@abc1234 # v4", "evil-org/steal@deadbeef # v1"}, "", false, false, false, false) require.Error(t, err) - assert.Contains(t, err.Error(), "evil-org/steal") + require.ErrorContains(t, err, "evil-org/steal") } func TestFormal_CTR016_RemovedActionDriftRejected(t *testing.T) { manifest := &GHAWManifest{Version: currentGHAWManifestVersion, Actions: []GHAWManifestAction{{Repo: "my-org/approved-action", SHA: "abc1234", Version: "v1"}}} err := EnforceSafeUpdate(manifest, nil, []string{}, "", false, false, false, false) require.Error(t, err) - assert.Contains(t, err.Error(), "Previously-approved action") - assert.Contains(t, err.Error(), "my-org/approved-action") + require.ErrorContains(t, err, "Previously-approved action") + require.ErrorContains(t, err, "my-org/approved-action") } func TestFormal_CTR016_KnownActionPinUpdateAllowed(t *testing.T) { @@ -72,8 +71,8 @@ func TestFormal_CTR016_RedirectChangeRejected(t *testing.T) { manifest := &GHAWManifest{Version: currentGHAWManifestVersion, Redirect: "owner/repo/workflows/old.md@main"} err := EnforceSafeUpdate(manifest, nil, nil, "owner/repo/workflows/new.md@main", false, false, false, false) require.Error(t, err) - assert.Contains(t, err.Error(), "New redirect configured") - assert.Contains(t, err.Error(), "Previously-approved redirect removed") + require.ErrorContains(t, err, "New redirect configured") + require.ErrorContains(t, err, "Previously-approved redirect removed") } func TestFormal_CTR001_WritePermissionsRejected(t *testing.T) { @@ -88,7 +87,7 @@ func TestFormal_CTR001_WritePermissionsRejected(t *testing.T) { perms.Set(scope, PermissionWrite) err := validateDangerousPermissions(&WorkflowData{Permissions: "permissions: {}"}, perms) require.Error(t, err) - assert.Contains(t, err.Error(), "write permissions") + require.ErrorContains(t, err, "write permissions") }) } } @@ -119,7 +118,7 @@ func TestFormal_CTR011_AllowURLsRequiresSSLBump(t *testing.T) { }, }) require.Error(t, err) - assert.Contains(t, err.Error(), "allow-urls requires ssl-bump: true") + require.ErrorContains(t, err, "allow-urls requires ssl-bump: true") } func TestFormal_CTR011_AllowURLsWithSSLBumpAllowed(t *testing.T) { @@ -136,7 +135,7 @@ func TestFormal_CTR011_WildcardOnlyDomainRejected(t *testing.T) { compiler := NewCompiler() err := compiler.validateStrictNetwork(&NetworkPermissions{Allowed: []string{"*"}}) require.Error(t, err) - assert.Contains(t, err.Error(), "wildcard '*' is not allowed") + require.ErrorContains(t, err, "wildcard '*' is not allowed") } func TestFormal_CTR015_WildcardLabelRejected(t *testing.T) { @@ -145,7 +144,7 @@ func TestFormal_CTR015_WildcardLabelRejected(t *testing.T) { CreateIssues: &CreateIssuesConfig{AllowedLabels: []string{"*"}}, }) require.Error(t, err) - assert.Contains(t, err.Error(), "CTR-015") + require.ErrorContains(t, err, "CTR-015") } func TestFormal_CTR015_WildcardLabelRejected_CreateDiscussion(t *testing.T) { @@ -154,7 +153,7 @@ func TestFormal_CTR015_WildcardLabelRejected_CreateDiscussion(t *testing.T) { CreateDiscussions: &CreateDiscussionsConfig{AllowedLabels: []string{"*"}}, }) require.Error(t, err) - assert.Contains(t, err.Error(), "CTR-015") + require.ErrorContains(t, err, "CTR-015") } func TestFormal_CTR015_SpecificLabelsAllowed(t *testing.T) { @@ -176,7 +175,7 @@ func TestFormal_CTR014_StrictModeEnabledRejected(t *testing.T) { compiler.SetStrictMode(true) err := compiler.validateRunInstallScripts(&WorkflowData{RunInstallScripts: true}) require.Error(t, err) - assert.Contains(t, err.Error(), "strict mode") + require.ErrorContains(t, err, "strict mode") } func TestFormal_CTR014_DisabledAlwaysAllowed(t *testing.T) { diff --git a/pkg/workflow/compiler_validators_test.go b/pkg/workflow/compiler_validators_test.go index 8fcfbba3a2e..4a6df8fb044 100644 --- a/pkg/workflow/compiler_validators_test.go +++ b/pkg/workflow/compiler_validators_test.go @@ -58,7 +58,7 @@ func TestValidateExpressions(t *testing.T) { if tt.shouldError { require.Error(t, err, "Expected validateExpressions to return an error") if tt.errorContains != "" { - assert.Contains(t, err.Error(), tt.errorContains, "Error should contain expected message") + require.ErrorContains(t, err, tt.errorContains, "Error should contain expected message") } } else { assert.NoError(t, err, "validateExpressions should not return an error") @@ -128,7 +128,7 @@ func TestValidateFeatureConfig(t *testing.T) { if tt.shouldError { require.Error(t, err, "Expected validateFeatureConfig to return an error") if tt.errorContains != "" { - assert.Contains(t, err.Error(), tt.errorContains, "Error should contain expected message") + require.ErrorContains(t, err, tt.errorContains, "Error should contain expected message") } } else { assert.NoError(t, err, "validateFeatureConfig should not return an error") @@ -342,7 +342,7 @@ func TestValidatePermissions(t *testing.T) { if tt.shouldError { require.Error(t, err, "Expected validatePermissions to return an error") if tt.errorContains != "" { - assert.Contains(t, err.Error(), tt.errorContains, "Error should contain expected message") + require.ErrorContains(t, err, tt.errorContains, "Error should contain expected message") } } else { require.NoError(t, err, "validatePermissions should not return an error") @@ -416,7 +416,7 @@ func TestValidateToolConfiguration(t *testing.T) { if tt.shouldError { require.Error(t, err, "Expected validateToolConfiguration to return an error") if tt.errorContains != "" { - assert.Contains(t, err.Error(), tt.errorContains, "Error should contain expected message") + require.ErrorContains(t, err, tt.errorContains, "Error should contain expected message") } } else { assert.NoError(t, err, "validateToolConfiguration should not return an error") @@ -442,7 +442,7 @@ func TestValidatePermissions_UsesCachedPermissionScopeValidation(t *testing.T) { compiler := NewCompiler() _, err := compiler.validatePermissions(workflowData, markdownPath) require.Error(t, err) - assert.Contains(t, err.Error(), cachedErr.Error()) + require.ErrorContains(t, err, cachedErr.Error()) } func TestValidatePermissions_EmitsCopilotRequestsTipOncePerMarkdownPath(t *testing.T) { @@ -561,7 +561,7 @@ func TestValidateToolConfiguration_EmitsSandboxWarningBeforeThreatDetectionError }) require.Error(t, validateErr) - assert.Contains(t, validateErr.Error(), "threat detection requires sandbox.agent") + require.ErrorContains(t, validateErr, "threat detection requires sandbox.agent") assert.Contains(t, stderr, "Agent sandbox disabled (sandbox.agent: false)") assert.Equal(t, initialWarnings+1, compiler.GetWarningCount()) } diff --git a/pkg/workflow/compiler_yaml_error_wrapping_test.go b/pkg/workflow/compiler_yaml_error_wrapping_test.go index 220d0ff7e93..72e3b8bf7ec 100644 --- a/pkg/workflow/compiler_yaml_error_wrapping_test.go +++ b/pkg/workflow/compiler_yaml_error_wrapping_test.go @@ -67,7 +67,7 @@ jobs: // Error should contain all wrapping messages for _, wantStr := range tt.wantErrContains { - assert.Contains(t, err.Error(), wantStr, + require.ErrorContains(t, err, wantStr, "Error should contain: %s", wantStr) } @@ -133,7 +133,7 @@ jobs: assert.GreaterOrEqual(t, layers, 2, "Error should be wrapped multiple times") // The error message should contain file path (from formatCompilerError) - assert.Contains(t, err.Error(), "test.md", "Error should contain file path") + require.ErrorContains(t, err, "test.md", "Error should contain file path") // The error message should contain context from wrapping errMsg := err.Error() @@ -159,7 +159,7 @@ func TestGenerateYAML_FrontmatterHashFailure(t *testing.T) { _, _, _, err := compiler.generateYAML(data, missingPath) require.Error(t, err, "Expected error when frontmatter hash computation fails") - assert.Contains(t, err.Error(), "could not compute stable frontmatter hash", + require.ErrorContains(t, err, "could not compute stable frontmatter hash", "Error should mention frontmatter hash failure") } @@ -211,7 +211,7 @@ jobs: // All expected strings should be present in the error for _, wantStr := range tt.wantErrContains { - assert.Contains(t, err.Error(), wantStr, + require.ErrorContains(t, err, wantStr, "Error should contain: %s", wantStr) } diff --git a/pkg/workflow/compiler_yaml_main_job_test.go b/pkg/workflow/compiler_yaml_main_job_test.go index f1730be765a..34d1e3cd96b 100644 --- a/pkg/workflow/compiler_yaml_main_job_test.go +++ b/pkg/workflow/compiler_yaml_main_job_test.go @@ -673,7 +673,7 @@ func TestGenerateMainJobSteps(t *testing.T) { if tt.shouldError { require.Error(t, err, "expected error but got none") if tt.errorContains != "" { - assert.Contains(t, err.Error(), tt.errorContains, "error message mismatch") + require.ErrorContains(t, err, tt.errorContains, "error message mismatch") } } else { require.NoError(t, err, "unexpected error: %v", err) diff --git a/pkg/workflow/concurrency_validation_integration_test.go b/pkg/workflow/concurrency_validation_integration_test.go index 4d309cf1228..2a32eacb554 100644 --- a/pkg/workflow/concurrency_validation_integration_test.go +++ b/pkg/workflow/concurrency_validation_integration_test.go @@ -270,7 +270,7 @@ tools: if tt.expectError { assert.Error(t, err, "Expected error for: %s", tt.description) if tt.errorSubstr != "" { - assert.Contains(t, err.Error(), tt.errorSubstr, + assert.ErrorContains(t, err, tt.errorSubstr, "Error should contain expected substring for: %s", tt.description) } } else { diff --git a/pkg/workflow/concurrency_validation_test.go b/pkg/workflow/concurrency_validation_test.go index 7e7fb8b2f20..3ab0b1ee8f4 100644 --- a/pkg/workflow/concurrency_validation_test.go +++ b/pkg/workflow/concurrency_validation_test.go @@ -227,7 +227,7 @@ func TestValidateConcurrencyGroupExpression(t *testing.T) { if tt.wantErr { require.Error(t, err, "Test case: %s - Expected error but got nil", tt.description) if tt.errorInMsg != "" { - assert.Contains(t, err.Error(), tt.errorInMsg, + require.ErrorContains(t, err, tt.errorInMsg, "Error message should contain expected substring for: %s", tt.description) } } else { @@ -286,7 +286,7 @@ func TestValidateBalancedBraces(t *testing.T) { if tt.wantErr { require.Error(t, err, "Expected error for input: %s", tt.input) if tt.errorInMsg != "" { - assert.Contains(t, err.Error(), tt.errorInMsg) + require.ErrorContains(t, err, tt.errorInMsg) } } else { assert.NoError(t, err, "Expected no error for input: %s", tt.input) @@ -358,7 +358,7 @@ func TestValidateExpressionContent(t *testing.T) { if tt.wantErr { require.Error(t, err, "Expected error for expression: %s", tt.expr) if tt.errorInMsg != "" { - assert.Contains(t, err.Error(), tt.errorInMsg) + require.ErrorContains(t, err, tt.errorInMsg) } } else { assert.NoError(t, err, "Expected no error for expression: %s", tt.expr) @@ -431,7 +431,7 @@ func TestValidateBalancedQuotes(t *testing.T) { if tt.wantErr { require.Error(t, err, "Expected error for expression: %s", tt.expr) if tt.errorInMsg != "" { - assert.Contains(t, err.Error(), tt.errorInMsg) + require.ErrorContains(t, err, tt.errorInMsg) } } else { assert.NoError(t, err, "Expected no error for expression: %s", tt.expr) @@ -824,7 +824,7 @@ func TestValidateConcurrencyQueueConfiguration(t *testing.T) { err := validateConcurrencyQueueConfiguration(tt.concurrency) if tt.wantErr { require.Error(t, err, "invalid queue/cancel-in-progress combination should fail validation") - assert.Contains(t, err.Error(), "queue: max cannot be combined with cancel-in-progress: true", "error should explain the queue/cancel-in-progress constraint") + require.ErrorContains(t, err, "queue: max cannot be combined with cancel-in-progress: true", "error should explain the queue/cancel-in-progress constraint") return } assert.NoError(t, err, "valid concurrency configuration should pass queue/cancel-in-progress validation") diff --git a/pkg/workflow/dispatch_repository_test.go b/pkg/workflow/dispatch_repository_test.go index 6d91d289d3b..5c198a3ab75 100644 --- a/pkg/workflow/dispatch_repository_test.go +++ b/pkg/workflow/dispatch_repository_test.go @@ -231,7 +231,7 @@ func TestValidateDispatchRepository_MissingWorkflow(t *testing.T) { err = compiler.validateDispatchRepository(workflowData, workflowPath) require.Error(t, err, "Validation should fail when workflow is missing") - assert.Contains(t, err.Error(), "workflow", "Error should mention workflow field") + require.ErrorContains(t, err, "workflow", "Error should mention workflow field") } // TestValidateDispatchRepository_MissingEventType tests error when event_type field is missing @@ -263,7 +263,7 @@ func TestValidateDispatchRepository_MissingEventType(t *testing.T) { err = compiler.validateDispatchRepository(workflowData, workflowPath) require.Error(t, err, "Validation should fail when event_type is missing") - assert.Contains(t, err.Error(), "event_type", "Error should mention event_type field") + require.ErrorContains(t, err, "event_type", "Error should mention event_type field") } // TestValidateDispatchRepository_MissingRepository tests error when no repository is specified @@ -295,7 +295,7 @@ func TestValidateDispatchRepository_MissingRepository(t *testing.T) { err = compiler.validateDispatchRepository(workflowData, workflowPath) require.Error(t, err, "Validation should fail when no repository is specified") - assert.Contains(t, err.Error(), "repository", "Error should mention repository") + require.ErrorContains(t, err, "repository", "Error should mention repository") } // TestValidateDispatchRepository_AllowedRepositories tests valid config with allowed_repositories @@ -358,7 +358,7 @@ func TestValidateDispatchRepository_InvalidRepoFormat(t *testing.T) { err = compiler.validateDispatchRepository(workflowData, workflowPath) require.Error(t, err, "Validation should fail for invalid repository format") - assert.Contains(t, err.Error(), "invalid", "Error should mention invalid format") + require.ErrorContains(t, err, "invalid", "Error should mention invalid format") } // TestValidateDispatchRepository_GitHubExpression tests that GitHub Actions expressions are accepted @@ -447,7 +447,7 @@ func TestValidateDispatchRepository_EmptyTools(t *testing.T) { err = compiler.validateDispatchRepository(workflowData, workflowPath) require.Error(t, err, "Validation should fail with empty tools map") - assert.Contains(t, err.Error(), "at least one dispatch tool", "Error should mention tools requirement") + require.ErrorContains(t, err, "at least one dispatch tool", "Error should mention tools requirement") } // TestValidateDispatchRepository_NilConfig tests that nil config is OK (no-op) @@ -664,6 +664,6 @@ safe-outputs: err = compiler.CompileWorkflow(workflowFile) require.Error(t, err, "Compilation should fail due to missing workflow field") - assert.Contains(t, err.Error(), "dispatch_repository", "Error should mention dispatch_repository") - assert.Contains(t, err.Error(), "workflow", "Error should mention workflow field") + require.ErrorContains(t, err, "dispatch_repository", "Error should mention dispatch_repository") + require.ErrorContains(t, err, "workflow", "Error should mention workflow field") } diff --git a/pkg/workflow/dispatch_workflow_test.go b/pkg/workflow/dispatch_workflow_test.go index cb889fbcde6..b73412deb16 100644 --- a/pkg/workflow/dispatch_workflow_test.go +++ b/pkg/workflow/dispatch_workflow_test.go @@ -223,8 +223,8 @@ This workflow tries to dispatch to a non-existent workflow. // Validate the workflow - should fail because nonexistent workflow is not found err = compiler.validateDispatchWorkflow(workflowData, dispatcherFile) require.Error(t, err, "Validation should fail - workflow not found") - assert.Contains(t, err.Error(), "not found", "Error should mention workflow not found") - assert.Contains(t, err.Error(), "nonexistent", "Error should mention the workflow name") + require.ErrorContains(t, err, "not found", "Error should mention workflow not found") + require.ErrorContains(t, err, "nonexistent", "Error should mention the workflow name") } // TestDispatchWorkflowWithoutWorkflowDispatchTrigger tests error handling @@ -291,7 +291,7 @@ This workflow tries to dispatch to ci workflow. // Validate the workflow - should fail because ci doesn't support workflow_dispatch err = compiler.validateDispatchWorkflow(workflowData, dispatcherFile) require.Error(t, err, "Validation should fail - workflow doesn't support workflow_dispatch") - assert.Contains(t, err.Error(), "workflow_dispatch", "Error should mention workflow_dispatch") + require.ErrorContains(t, err, "workflow_dispatch", "Error should mention workflow_dispatch") } // TestDispatchWorkflowFileExtensionResolution tests that the correct file extension @@ -451,11 +451,11 @@ No agentic-workflows tool is present. // Check that compilation failed due to validation require.Error(t, err, "Compilation should fail for non-existent workflow") - assert.Contains(t, err.Error(), "dispatch-workflow validation failed", + require.ErrorContains(t, err, "dispatch-workflow validation failed", "Should fail with dispatch-workflow validation error") - assert.Contains(t, err.Error(), "not found", + require.ErrorContains(t, err, "not found", "Error should mention workflow not found") - assert.Contains(t, err.Error(), "nonexistent", + require.ErrorContains(t, err, "nonexistent", "Error should mention the workflow name") } diff --git a/pkg/workflow/dispatch_workflow_validation_test.go b/pkg/workflow/dispatch_workflow_validation_test.go index 4ab97c5f3c2..a889e6e7b63 100644 --- a/pkg/workflow/dispatch_workflow_validation_test.go +++ b/pkg/workflow/dispatch_workflow_validation_test.go @@ -202,7 +202,7 @@ safe-outputs: err = compiler.validateDispatchWorkflow(workflowData, dispatcherFile) require.Error(t, err, "Validation should still run when target-repo references github.repository") - assert.Contains(t, err.Error(), "workflow 'missing-workflow' not found") + require.ErrorContains(t, err, "workflow 'missing-workflow' not found") } func TestDispatchWorkflowValidation_UsesWorkflowDirEnvOverride(t *testing.T) { @@ -483,7 +483,7 @@ safe-outputs: // Validation should fail: .md exists but lacks workflow_dispatch err = compiler.validateDispatchWorkflow(workflowData, dispatcherFile) require.Error(t, err, "Validation should fail when .md target lacks workflow_dispatch") - assert.Contains(t, err.Error(), "does not support workflow_dispatch trigger", "Should explain missing trigger") + require.ErrorContains(t, err, "does not support workflow_dispatch trigger", "Should explain missing trigger") } // TestDispatchWorkflowErrorMessage_MultipleErrors tests that multiple errors diff --git a/pkg/workflow/docker_sbx_test.go b/pkg/workflow/docker_sbx_test.go index 183e9a4c7a2..bd8777d9522 100644 --- a/pkg/workflow/docker_sbx_test.go +++ b/pkg/workflow/docker_sbx_test.go @@ -450,8 +450,8 @@ func TestDockerSbxValidation_ArcDindIncompatible(t *testing.T) { err := validateSandboxConfig(workflowData) require.Error(t, err, "docker-sbx + arc-dind must produce a compile-time error") - assert.Contains(t, err.Error(), "arc-dind", "error must mention arc-dind") - assert.Contains(t, err.Error(), "docker-sbx", "error must mention docker-sbx") + require.ErrorContains(t, err, "arc-dind", "error must mention arc-dind") + require.ErrorContains(t, err, "docker-sbx", "error must mention docker-sbx") } // TestDockerSbxValidation_SudoFalseRejected verifies that docker-sbx without @@ -474,8 +474,8 @@ func TestDockerSbxValidation_SudoFalseRejected(t *testing.T) { err := validateSandboxConfig(workflowData) require.Error(t, err, "docker-sbx without sudo: true must produce a compile-time error") - assert.Contains(t, err.Error(), "sudo: true", "error must mention sudo: true") - assert.Contains(t, err.Error(), "docker-sbx", "error must mention docker-sbx") + require.ErrorContains(t, err, "sudo: true", "error must mention sudo: true") + require.ErrorContains(t, err, "docker-sbx", "error must mention docker-sbx") } // TestDockerSbxValidation_DefaultVersionRejected verifies that docker-sbx is rejected @@ -500,7 +500,7 @@ func TestDockerSbxValidation_DefaultVersionRejected(t *testing.T) { err := validateSandboxConfig(workflowData) require.Error(t, err, "docker-sbx with an AWF version predating containerRuntime support must fail validation") - assert.Contains(t, err.Error(), string(constants.AWFContainerRuntimeMinVersion)) + require.ErrorContains(t, err, string(constants.AWFContainerRuntimeMinVersion)) } // TestDockerSbxValidation_MinVersionSatisfied verifies that docker-sbx passes validation diff --git a/pkg/workflow/docker_validation_graceful_test.go b/pkg/workflow/docker_validation_graceful_test.go index 1cc2335f1cc..c0dbebf53d8 100644 --- a/pkg/workflow/docker_validation_graceful_test.go +++ b/pkg/workflow/docker_validation_graceful_test.go @@ -47,7 +47,7 @@ func TestValidateDockerImage_StillRejectsHyphenWithoutDocker(t *testing.T) { // so it should always reject invalid names regardless of Docker state. err := validateDockerImage("-malicious", false, false) require.Error(t, err, "should reject image names starting with hyphen regardless of Docker availability") - assert.Contains(t, err.Error(), "names must not start with '-'", + require.ErrorContains(t, err, "names must not start with '-'", "error should explain why the name is invalid") } @@ -80,18 +80,18 @@ func TestValidateDockerImage_RequireDockerFailsWhenUnavailable(t *testing.T) { if _, lookErr := exec.LookPath("docker"); lookErr != nil { err := validateDockerImage("ghcr.io/some/image:latest", false, true) require.Error(t, err, "should fail when Docker is not installed and requireDocker is true") - assert.Contains(t, err.Error(), "docker not installed", + require.ErrorContains(t, err, "docker not installed", "error should mention Docker is not installed") - assert.Contains(t, err.Error(), "--validate-images", + require.ErrorContains(t, err, "--validate-images", "error should mention the --validate-images flag") return } if !isDockerDaemonRunning() { err := validateDockerImage("ghcr.io/some/image:latest", false, true) require.Error(t, err, "should fail when Docker daemon is not running and requireDocker is true") - assert.Contains(t, err.Error(), "docker daemon not running", + require.ErrorContains(t, err, "docker daemon not running", "error should mention Docker daemon is not running") - assert.Contains(t, err.Error(), "--validate-images", + require.ErrorContains(t, err, "--validate-images", "error should mention the --validate-images flag") return } diff --git a/pkg/workflow/domains_test.go b/pkg/workflow/domains_test.go index dd65997e824..22d880ab53f 100644 --- a/pkg/workflow/domains_test.go +++ b/pkg/workflow/domains_test.go @@ -1352,6 +1352,6 @@ func TestExtractProviderFromModel(t *testing.T) { t.Run("leading slash returns error", func(t *testing.T) { _, err := extractProviderFromModel("/gpt-4.1") require.Error(t, err, "Leading slash (empty provider prefix) must return an error") - assert.Contains(t, err.Error(), "provider prefix is empty") + require.ErrorContains(t, err, "provider prefix is empty") }) } diff --git a/pkg/workflow/engine_auth_test.go b/pkg/workflow/engine_auth_test.go index a440d17bd1a..e9177a719b2 100644 --- a/pkg/workflow/engine_auth_test.go +++ b/pkg/workflow/engine_auth_test.go @@ -121,8 +121,8 @@ func TestValidateEngineAuthDefinition_MissingTokenURL(t *testing.T) { err := compiler.validateEngineAuthDefinition(config) require.Error(t, err, "missing token-url should produce an error") - assert.Contains(t, err.Error(), "token-url", "error should mention missing token-url field") - assert.Contains(t, err.Error(), "oauth-client-credentials", "error should mention the strategy") + require.ErrorContains(t, err, "token-url", "error should mention missing token-url field") + require.ErrorContains(t, err, "oauth-client-credentials", "error should mention the strategy") } // TestValidateEngineAuthDefinition_MissingClientID verifies that omitting client-id @@ -142,7 +142,7 @@ func TestValidateEngineAuthDefinition_MissingClientID(t *testing.T) { err := compiler.validateEngineAuthDefinition(config) require.Error(t, err, "missing client-id should produce an error") - assert.Contains(t, err.Error(), "client-id", "error should mention missing client-id field") + require.ErrorContains(t, err, "client-id", "error should mention missing client-id field") } // TestValidateEngineAuthDefinition_MissingClientSecret verifies that omitting client-secret @@ -162,7 +162,7 @@ func TestValidateEngineAuthDefinition_MissingClientSecret(t *testing.T) { err := compiler.validateEngineAuthDefinition(config) require.Error(t, err, "missing client-secret should produce an error") - assert.Contains(t, err.Error(), "client-secret", "error should mention missing client-secret field") + require.ErrorContains(t, err, "client-secret", "error should mention missing client-secret field") } // TestValidateEngineAuthDefinition_MissingHeaderNameForOAuth verifies that omitting @@ -182,7 +182,7 @@ func TestValidateEngineAuthDefinition_MissingHeaderNameForOAuth(t *testing.T) { err := compiler.validateEngineAuthDefinition(config) require.Error(t, err, "missing header-name for oauth should produce an error") - assert.Contains(t, err.Error(), "header-name", "error should mention missing header-name field") + require.ErrorContains(t, err, "header-name", "error should mention missing header-name field") } // TestValidateEngineAuthDefinition_MissingHeaderNameForAPIKey verifies that api-key @@ -200,7 +200,7 @@ func TestValidateEngineAuthDefinition_MissingHeaderNameForAPIKey(t *testing.T) { err := compiler.validateEngineAuthDefinition(config) require.Error(t, err, "api-key without header-name should produce an error") - assert.Contains(t, err.Error(), "header-name", "error should mention missing header-name field") + require.ErrorContains(t, err, "header-name", "error should mention missing header-name field") } // TestValidateEngineAuthDefinition_APIKeyRequiresSecret verifies that api-key @@ -219,7 +219,7 @@ func TestValidateEngineAuthDefinition_APIKeyRequiresSecret(t *testing.T) { err := compiler.validateEngineAuthDefinition(config) require.Error(t, err, "api-key without secret should produce an error") - assert.Contains(t, err.Error(), "auth.secret", "error should mention missing secret field") + require.ErrorContains(t, err, "auth.secret", "error should mention missing secret field") } // TestValidateEngineAuthDefinition_BearerRequiresSecret verifies that bearer @@ -237,7 +237,7 @@ func TestValidateEngineAuthDefinition_BearerRequiresSecret(t *testing.T) { err := compiler.validateEngineAuthDefinition(config) require.Error(t, err, "bearer without secret should produce an error") - assert.Contains(t, err.Error(), "auth.secret", "error should mention missing secret field") + require.ErrorContains(t, err, "auth.secret", "error should mention missing secret field") } // TestValidateEngineAuthDefinition_APIKeyValid verifies that a complete api-key definition @@ -289,10 +289,10 @@ func TestValidateEngineAuthDefinition_UnknownStrategy(t *testing.T) { err := compiler.validateEngineAuthDefinition(config) require.Error(t, err, "unknown strategy should produce an error") - assert.Contains(t, err.Error(), "invalid-strategy", "error should mention the unknown strategy") - assert.Contains(t, err.Error(), "api-key", "error should list valid strategies") - assert.Contains(t, err.Error(), "oauth-client-credentials", "error should list valid strategies") - assert.Contains(t, err.Error(), "bearer", "error should list valid strategies") + require.ErrorContains(t, err, "invalid-strategy", "error should mention the unknown strategy") + require.ErrorContains(t, err, "api-key", "error should list valid strategies") + require.ErrorContains(t, err, "oauth-client-credentials", "error should list valid strategies") + require.ErrorContains(t, err, "bearer", "error should list valid strategies") } // TestValidateEngineAuthDefinition_NilAuth verifies that a nil InlineProviderAuth is a no-op. diff --git a/pkg/workflow/engine_definition_loader_test.go b/pkg/workflow/engine_definition_loader_test.go index fdfe6f7aca7..796c8bae68c 100644 --- a/pkg/workflow/engine_definition_loader_test.go +++ b/pkg/workflow/engine_definition_loader_test.go @@ -108,7 +108,7 @@ func TestBuiltinEngineStringFormInjection(t *testing.T) { err := compiler.CompileWorkflow(mainFile) if tt.expectError { require.Error(t, err, "compilation should fail for engine %s (string form)", tt.engineID) - assert.Contains(t, err.Error(), tt.errorContains) + require.ErrorContains(t, err, tt.errorContains) return } require.NoError(t, err, "compilation should succeed for engine %s (string form)", tt.engineID) diff --git a/pkg/workflow/engine_definition_test.go b/pkg/workflow/engine_definition_test.go index b967b0e99e9..1b92a2fcf1e 100644 --- a/pkg/workflow/engine_definition_test.go +++ b/pkg/workflow/engine_definition_test.go @@ -87,13 +87,13 @@ func TestEngineCatalog_Resolve_UnknownEngine(t *testing.T) { _, err := catalog.Resolve("nonexistent-engine", &EngineConfig{ID: "nonexistent-engine"}) require.Error(t, err, "unknown engine should return an error") - assert.Contains(t, err.Error(), "invalid engine", + require.ErrorContains(t, err, "invalid engine", "error should mention 'invalid engine', got: %s", err.Error()) - assert.Contains(t, err.Error(), "nonexistent-engine", + require.ErrorContains(t, err, "nonexistent-engine", "error should mention the unknown engine ID, got: %s", err.Error()) - assert.Contains(t, err.Error(), string(constants.DocsEnginesURL), + require.ErrorContains(t, err, string(constants.DocsEnginesURL), "error should include the engines documentation URL, got: %s", err.Error()) - assert.Contains(t, err.Error(), "engine: copilot", + require.ErrorContains(t, err, "engine: copilot", "error should include an example, got: %s", err.Error()) } diff --git a/pkg/workflow/engine_inline_test.go b/pkg/workflow/engine_inline_test.go index 88dceb78f75..e1b098696c5 100644 --- a/pkg/workflow/engine_inline_test.go +++ b/pkg/workflow/engine_inline_test.go @@ -228,8 +228,8 @@ func TestValidateEngineInlineDefinition_MissingRuntimeID(t *testing.T) { err := c.validateEngineInlineDefinition(config) require.Error(t, err, "missing runtime.id should return an error") - assert.Contains(t, err.Error(), "runtime.id", "error should mention the missing field") - assert.Contains(t, err.Error(), string(constants.DocsEnginesURL), "error should include docs URL") + require.ErrorContains(t, err, "runtime.id", "error should mention the missing field") + require.ErrorContains(t, err, string(constants.DocsEnginesURL), "error should include docs URL") } // TestValidateEngineInlineDefinition_ValidRuntimeID verifies that a valid inline @@ -341,11 +341,11 @@ func TestInlineEngineDefinition_UnknownRuntimeID(t *testing.T) { // validateEngineInlineDefinition should catch the unknown runtime ID with a clear error. err := c.validateEngineInlineDefinition(config) require.Error(t, err, "unknown runtime.id should produce a validation error") - assert.Contains(t, err.Error(), "nonexistent-runtime", + require.ErrorContains(t, err, "nonexistent-runtime", "error should mention the unknown runtime ID") - assert.Contains(t, err.Error(), "runtime.id", + require.ErrorContains(t, err, "runtime.id", "error should mention the 'runtime.id' field") - assert.Contains(t, err.Error(), string(constants.DocsEnginesURL), + require.ErrorContains(t, err, string(constants.DocsEnginesURL), "error should include the docs URL") } diff --git a/pkg/workflow/engine_validation_test.go b/pkg/workflow/engine_validation_test.go index e358f29bd14..5ec4f7a9f8c 100644 --- a/pkg/workflow/engine_validation_test.go +++ b/pkg/workflow/engine_validation_test.go @@ -438,7 +438,7 @@ func TestValidateEngineHarnessScript(t *testing.T) { if tt.expectError { require.Error(t, err, "Expected validation error") if tt.errorSubstr != "" { - assert.Contains(t, err.Error(), tt.errorSubstr, "Expected error substring mismatch") + require.ErrorContains(t, err, tt.errorSubstr, "Expected error substring mismatch") } return } @@ -621,7 +621,7 @@ func TestValidateEngineCopilotSDKDriver_Copilot(t *testing.T) { if tt.expectError { require.Error(t, err, "Expected validation error") if tt.errorSubstr != "" { - assert.Contains(t, err.Error(), tt.errorSubstr, "Expected error substring mismatch") + require.ErrorContains(t, err, tt.errorSubstr, "Expected error substring mismatch") } return } @@ -725,7 +725,7 @@ func TestValidateEngineMCPSessionTimeout(t *testing.T) { if tt.expectError { require.Error(t, err, "Expected validation error") if tt.errorSubstr != "" { - assert.Contains(t, err.Error(), tt.errorSubstr, "Expected error substring mismatch") + require.ErrorContains(t, err, tt.errorSubstr, "Expected error substring mismatch") } return } @@ -845,7 +845,7 @@ func TestValidateEngineMCPToolTimeout(t *testing.T) { if tt.expectError { require.Error(t, err, "Expected validation error") if tt.errorSubstr != "" { - assert.Contains(t, err.Error(), tt.errorSubstr, "Expected error substring mismatch") + require.ErrorContains(t, err, tt.errorSubstr, "Expected error substring mismatch") } return } diff --git a/pkg/workflow/env_secrets_validation_test.go b/pkg/workflow/env_secrets_validation_test.go index 3ded8444a2f..510091ae970 100644 --- a/pkg/workflow/env_secrets_validation_test.go +++ b/pkg/workflow/env_secrets_validation_test.go @@ -226,7 +226,7 @@ func TestValidateEnvSecrets(t *testing.T) { if tt.expectError { require.Error(t, err, "Expected an error but got none") if tt.errorMsg != "" { - assert.Contains(t, err.Error(), tt.errorMsg, "Error message should contain expected text") + require.ErrorContains(t, err, tt.errorMsg, "Error message should contain expected text") } } else { assert.NoError(t, err, "Expected no error but got: %v", err) @@ -707,7 +707,7 @@ func TestValidateEngineEnvSecrets(t *testing.T) { if tt.expectError { require.Error(t, err, "Expected an error but got none") if tt.errorMsg != "" { - assert.Contains(t, err.Error(), tt.errorMsg, "Error message should contain expected text") + require.ErrorContains(t, err, tt.errorMsg, "Error message should contain expected text") } } else { assert.NoError(t, err, "Expected no error but got: %v", err) diff --git a/pkg/workflow/error_helpers_test.go b/pkg/workflow/error_helpers_test.go index 4f92baaeffe..05357559d13 100644 --- a/pkg/workflow/error_helpers_test.go +++ b/pkg/workflow/error_helpers_test.go @@ -16,13 +16,13 @@ func TestValidationError(t *testing.T) { err := NewValidationError("title", "", "cannot be empty", "Provide a non-empty title") require.Error(t, err) - assert.Contains(t, err.Error(), "Validation failed for field 'title'") - assert.Contains(t, err.Error(), "Reason: cannot be empty") - assert.Contains(t, err.Error(), "Suggestion: Provide a non-empty title") + require.ErrorContains(t, err, "Validation failed for field 'title'") + require.ErrorContains(t, err, "Reason: cannot be empty") + require.ErrorContains(t, err, "Suggestion: Provide a non-empty title") // Check timestamp is included - assert.Contains(t, err.Error(), "[") - assert.Contains(t, err.Error(), "T") + require.ErrorContains(t, err, "[") + require.ErrorContains(t, err, "T") }) t.Run("validation error with long value", func(t *testing.T) { @@ -31,7 +31,7 @@ func TestValidationError(t *testing.T) { require.Error(t, err) // Value should be truncated - assert.Contains(t, err.Error(), "...") + require.ErrorContains(t, err, "...") assert.Less(t, len(err.Error()), len(longValue)+200) }) @@ -39,7 +39,7 @@ func TestValidationError(t *testing.T) { err := NewValidationError("labels", "invalid", "not allowed", "") require.Error(t, err) - assert.Contains(t, err.Error(), "Validation failed") + require.ErrorContains(t, err, "Validation failed") assert.NotContains(t, err.Error(), "Suggestion:") }) } @@ -59,9 +59,9 @@ func TestOperationError(t *testing.T) { err := NewOperationError("update", "issue", "123", cause, "Check permissions") require.Error(t, err) - assert.Contains(t, err.Error(), "Failed to update issue #123") - assert.Contains(t, err.Error(), "Underlying error: API error") - assert.Contains(t, err.Error(), "Suggestion: Check permissions") + require.ErrorContains(t, err, "Failed to update issue #123") + require.ErrorContains(t, err, "Underlying error: API error") + require.ErrorContains(t, err, "Suggestion: Check permissions") }) t.Run("operation error without entity ID", func(t *testing.T) { @@ -69,10 +69,10 @@ func TestOperationError(t *testing.T) { err := NewOperationError("create", "PR", "", cause, "") require.Error(t, err) - assert.Contains(t, err.Error(), "Failed to create PR") + require.ErrorContains(t, err, "Failed to create PR") assert.NotContains(t, err.Error(), "#") // Should have default suggestion - assert.Contains(t, err.Error(), "Check that the PR exists") + require.ErrorContains(t, err, "Check that the PR exists") }) t.Run("operation error unwrap", func(t *testing.T) { @@ -87,8 +87,8 @@ func TestOperationError(t *testing.T) { cause := errors.New("failed") err := NewOperationError("operation", "entity", "1", cause, "") - assert.Contains(t, err.Error(), "[") - assert.Contains(t, err.Error(), "T") + require.ErrorContains(t, err, "[") + assert.ErrorContains(t, err, "T") }) } @@ -97,18 +97,18 @@ func TestConfigurationError(t *testing.T) { err := NewConfigurationError("safe-outputs.max", "abc", "must be an integer", "Use a numeric value") require.Error(t, err) - assert.Contains(t, err.Error(), "Configuration error in 'safe-outputs.max'") - assert.Contains(t, err.Error(), "Value: abc") - assert.Contains(t, err.Error(), "Reason: must be an integer") - assert.Contains(t, err.Error(), "Suggestion: Use a numeric value") + require.ErrorContains(t, err, "Configuration error in 'safe-outputs.max'") + require.ErrorContains(t, err, "Value: abc") + require.ErrorContains(t, err, "Reason: must be an integer") + require.ErrorContains(t, err, "Suggestion: Use a numeric value") }) t.Run("configuration error with default suggestion", func(t *testing.T) { err := NewConfigurationError("safe-outputs.target", "invalid", "not a valid target", "") require.Error(t, err) - assert.Contains(t, err.Error(), "Configuration error") - assert.Contains(t, err.Error(), "Check the safe-outputs configuration") + require.ErrorContains(t, err, "Configuration error") + require.ErrorContains(t, err, "Check the safe-outputs configuration") }) t.Run("configuration error with long value", func(t *testing.T) { @@ -117,6 +117,6 @@ func TestConfigurationError(t *testing.T) { require.Error(t, err) // Value should be truncated - assert.Contains(t, err.Error(), "...") + require.ErrorContains(t, err, "...") }) } diff --git a/pkg/workflow/error_wrapping_test.go b/pkg/workflow/error_wrapping_test.go index 55434afb035..30fdea4a380 100644 --- a/pkg/workflow/error_wrapping_test.go +++ b/pkg/workflow/error_wrapping_test.go @@ -300,7 +300,7 @@ func TestHTTPErrorsNotExposed(t *testing.T) { "HTTP internal errors should not be in the error chain") // But the message should still be informative - assert.Contains(t, userErr.Error(), "MCP server") + assert.ErrorContains(t, userErr, "MCP server") }) t.Run("IO errors should be wrapped with context", func(t *testing.T) { diff --git a/pkg/workflow/evals_config_test.go b/pkg/workflow/evals_config_test.go index d1e22148936..5c14fd838d7 100644 --- a/pkg/workflow/evals_config_test.go +++ b/pkg/workflow/evals_config_test.go @@ -103,15 +103,15 @@ func TestParseEvalsFromFrontmatter_QuestionLevelNonStringModel(t *testing.T) { }, }) require.Error(t, err) - assert.Contains(t, err.Error(), "model") - assert.Contains(t, err.Error(), "string") + require.ErrorContains(t, err, "model") + require.ErrorContains(t, err, "string") } func TestParseEvalsFromFrontmatter_InvalidType(t *testing.T) { c := NewCompiler() _, err := c.parseEvalsFromFrontmatter(map[string]any{"evals": "invalid"}) require.Error(t, err) - assert.Contains(t, err.Error(), "evals") + require.ErrorContains(t, err, "evals") } func TestParseEvalsFromFrontmatter_WrongTypeQuestions(t *testing.T) { @@ -122,8 +122,8 @@ func TestParseEvalsFromFrontmatter_WrongTypeQuestions(t *testing.T) { }, }) require.Error(t, err) - assert.Contains(t, err.Error(), "evals.questions") - assert.Contains(t, err.Error(), "list") + require.ErrorContains(t, err, "evals.questions") + require.ErrorContains(t, err, "list") } func TestParseEvalsFromFrontmatter_NonStringModel(t *testing.T) { @@ -137,8 +137,8 @@ func TestParseEvalsFromFrontmatter_NonStringModel(t *testing.T) { }, }) require.Error(t, err) - assert.Contains(t, err.Error(), "evals.model") - assert.Contains(t, err.Error(), "string") + require.ErrorContains(t, err, "evals.model") + require.ErrorContains(t, err, "string") } // --------------------------------------------------------------------------- @@ -148,7 +148,7 @@ func TestParseEvalsFromFrontmatter_NonStringModel(t *testing.T) { func TestValidateEvals_RejectsEmptyQuestions(t *testing.T) { err := validateEvals(&EvalsConfig{Questions: []EvalDefinition{}}) require.Error(t, err) - assert.Contains(t, err.Error(), "at least one question") + require.ErrorContains(t, err, "at least one question") } func TestValidateEvals_RejectsDuplicateID(t *testing.T) { @@ -160,8 +160,8 @@ func TestValidateEvals_RejectsDuplicateID(t *testing.T) { } err := validateEvals(cfg) require.Error(t, err) - assert.Contains(t, err.Error(), "duplicate id") - assert.Contains(t, err.Error(), "builds") + require.ErrorContains(t, err, "duplicate id") + require.ErrorContains(t, err, "builds") } func TestValidateEvals_RejectsEmptyQuestion(t *testing.T) { @@ -173,7 +173,7 @@ func TestValidateEvals_RejectsEmptyQuestion(t *testing.T) { } err := validateEvals(cfg) require.Error(t, err) - assert.Contains(t, err.Error(), "non-empty") + require.ErrorContains(t, err, "non-empty") } func TestValidateEvals_AcceptsValidQuestions(t *testing.T) { @@ -197,19 +197,19 @@ func TestValidateEvals_NilConfig(t *testing.T) { func TestParseEvalDefinition_MissingID(t *testing.T) { _, err := parseEvalDefinition(map[string]any{"question": "Something?"}, 0) require.Error(t, err) - assert.Contains(t, err.Error(), "id") + require.ErrorContains(t, err, "id") } func TestParseEvalDefinition_MissingQuestion(t *testing.T) { _, err := parseEvalDefinition(map[string]any{"id": "q1"}, 0) require.Error(t, err) - assert.Contains(t, err.Error(), "question") + require.ErrorContains(t, err, "question") } func TestParseEvalDefinition_EmptyID(t *testing.T) { _, err := parseEvalDefinition(map[string]any{"id": " ", "question": "Something?"}, 0) require.Error(t, err) - assert.Contains(t, err.Error(), "id") + require.ErrorContains(t, err, "id") } func TestParseEvalDefinition_TrimsWhitespace(t *testing.T) { diff --git a/pkg/workflow/event_validation_test.go b/pkg/workflow/event_validation_test.go index ec2eace75f3..d86f83afb17 100644 --- a/pkg/workflow/event_validation_test.go +++ b/pkg/workflow/event_validation_test.go @@ -113,7 +113,7 @@ func TestValidateEventTypes(t *testing.T) { if tt.wantErr { require.Error(t, err, "ValidateEventTypes should return an error") if tt.errContains != "" { - assert.Contains(t, err.Error(), tt.errContains, + require.ErrorContains(t, err, tt.errContains, "error should contain %q", tt.errContains) } } else { diff --git a/pkg/workflow/expression_coverage_test.go b/pkg/workflow/expression_coverage_test.go index 5067f55102c..f0f1d3c513d 100644 --- a/pkg/workflow/expression_coverage_test.go +++ b/pkg/workflow/expression_coverage_test.go @@ -193,7 +193,7 @@ func TestParseExpressionEmptyString(t *testing.T) { t.Run(tt.name, func(t *testing.T) { _, err := ParseExpression(tt.input) require.Error(t, err, "ParseExpression() with empty/whitespace string should return error") - assert.ErrorContains(t, err, "empty expression", + require.ErrorContains(t, err, "empty expression", "ParseExpression(%q) unexpected error message", tt.input) }) } diff --git a/pkg/workflow/expression_secrets_serialization_validation_test.go b/pkg/workflow/expression_secrets_serialization_validation_test.go index 48acb985d2f..eaae098c95b 100644 --- a/pkg/workflow/expression_secrets_serialization_validation_test.go +++ b/pkg/workflow/expression_secrets_serialization_validation_test.go @@ -214,7 +214,7 @@ func TestValidateSecretsSerializationExpressions(t *testing.T) { if tt.wantError { require.Error(t, err, "expected an error but got none") if tt.errorContains != "" { - assert.Contains(t, err.Error(), tt.errorContains, + require.ErrorContains(t, err, tt.errorContains, "error should contain expected message", ) } @@ -245,7 +245,7 @@ func TestValidateSecretsSerializationViaValidateExpressions(t *testing.T) { err := compiler.validateExpressions(workflowData, "/tmp/test.md") require.Error(t, err, "expected validateExpressions to surface secrets serialization error") - assert.Contains(t, err.Error(), "secrets serialization expression(s) detected") + require.ErrorContains(t, err, "secrets serialization expression(s) detected") } func TestValidateSecretsSerializationNonStrictViaValidateExpressions(t *testing.T) { @@ -276,7 +276,7 @@ func TestValidateSecretsSerializationNonStrictViaValidateExpressions(t *testing. err := compiler.validateExpressions(workflowData, "/tmp/test.md") require.Error(t, err, "dangerous constructor operand should still be rejected") - assert.Contains(t, err.Error(), "constructor") + require.ErrorContains(t, err, "constructor") assert.Equal(t, 1, compiler.GetWarningCount(), "secrets serialization warning should still be emitted") }) } diff --git a/pkg/workflow/forbidden_fields_import_test.go b/pkg/workflow/forbidden_fields_import_test.go index 09e34873561..188a3908dcf 100644 --- a/pkg/workflow/forbidden_fields_import_test.go +++ b/pkg/workflow/forbidden_fields_import_test.go @@ -81,7 +81,7 @@ This workflow imports a shared workflow with forbidden field. // Should get error about forbidden field require.Error(t, err, "Expected error for forbidden field '%s'", field) - assert.Contains(t, err.Error(), "cannot be used in shared workflows", + require.ErrorContains(t, err, "cannot be used in shared workflows", "Error should mention forbidden field, got: %v", err) }) } diff --git a/pkg/workflow/frontmatter_extraction_yaml_test.go b/pkg/workflow/frontmatter_extraction_yaml_test.go index ee7506ae27c..8e541d76db8 100644 --- a/pkg/workflow/frontmatter_extraction_yaml_test.go +++ b/pkg/workflow/frontmatter_extraction_yaml_test.go @@ -198,7 +198,7 @@ func TestExtractIfCondition_InvalidDeploymentStatusStateReturnsError(t *testing. got, err := c.extractIfCondition(frontmatter) require.Error(t, err) assert.Empty(t, got) - assert.ErrorContains(t, err, `invalid on.deployment_status.state value "unknown_state"`) + require.ErrorContains(t, err, `invalid on.deployment_status.state value "unknown_state"`) } func TestExtractWorkflowRunConclusionConditionHelper(t *testing.T) { diff --git a/pkg/workflow/frontmatter_types_test.go b/pkg/workflow/frontmatter_types_test.go index 98fe7e17595..57c6a527d9e 100644 --- a/pkg/workflow/frontmatter_types_test.go +++ b/pkg/workflow/frontmatter_types_test.go @@ -512,7 +512,7 @@ func TestParseFrontmatterConfig(t *testing.T) { "runs-on": tt.runsOn, }) require.Error(t, err) - assert.Contains(t, err.Error(), tt.errContains) + require.ErrorContains(t, err, tt.errContains) }) } }) diff --git a/pkg/workflow/gh_cli_permissions_test.go b/pkg/workflow/gh_cli_permissions_test.go index 304144833bc..eb0c964a5f4 100644 --- a/pkg/workflow/gh_cli_permissions_test.go +++ b/pkg/workflow/gh_cli_permissions_test.go @@ -378,8 +378,8 @@ jobs: compiler := NewCompiler() err = compiler.CompileWorkflow(testFile) require.Error(t, err, "compiler should reject write gh commands in activation pre-steps") - assert.Contains(t, err.Error(), "gh pr comment", "error should mention the offending command") - assert.Contains(t, err.Error(), "write", "error should explain the write-permission restriction") + require.ErrorContains(t, err, "gh pr comment", "error should mention the offending command") + require.ErrorContains(t, err, "write", "error should explain the write-permission restriction") } // TestActivationJobPermissionsWithGhCachePreStep verifies actions: read is added when @@ -543,9 +543,9 @@ Test agent pre-steps write command triggers error. compiler := NewCompiler() err := compiler.CompileWorkflow(testFile) require.Error(t, err, "compiler should error when agent pre-step uses a write gh command") - assert.Contains(t, err.Error(), "agent job uses write gh command(s)") - assert.Contains(t, err.Error(), "gh pr comment") - assert.Contains(t, err.Error(), "safe-outputs") + require.ErrorContains(t, err, "agent job uses write gh command(s)") + require.ErrorContains(t, err, "gh pr comment") + require.ErrorContains(t, err, "safe-outputs") } // TestAgentJobPreStepsInferReadPermission verifies that when an agent job pre-step @@ -868,8 +868,8 @@ Test agent steps write command triggers error. compiler := NewCompiler() err := compiler.CompileWorkflow(testFile) require.Error(t, err, "compiler should error when agent steps use a write gh command") - assert.Contains(t, err.Error(), "agent job uses write gh command(s)") - assert.Contains(t, err.Error(), "gh issue create") + require.ErrorContains(t, err, "agent job uses write gh command(s)") + require.ErrorContains(t, err, "gh issue create") } // TestAgentJobWriteCommandInPostStepsErrors verifies that a write gh command in a top-level @@ -896,8 +896,8 @@ Test agent post-steps write command triggers error. compiler := NewCompiler() err := compiler.CompileWorkflow(testFile) require.Error(t, err, "compiler should error when agent post-steps use a write gh command") - assert.Contains(t, err.Error(), "agent job uses write gh command(s)") - assert.Contains(t, err.Error(), "gh pr close") + require.ErrorContains(t, err, "agent job uses write gh command(s)") + require.ErrorContains(t, err, "gh pr close") } // TestActivationJobStepsNotScanned verifies that jobs.activation.steps is NOT scanned for diff --git a/pkg/workflow/git_tool_validation_integration_test.go b/pkg/workflow/git_tool_validation_integration_test.go index b70eeb9765b..e615c282e66 100644 --- a/pkg/workflow/git_tool_validation_integration_test.go +++ b/pkg/workflow/git_tool_validation_integration_test.go @@ -208,7 +208,7 @@ Test workflow that doesn't use PR features. if tt.expectError { require.Error(t, err, "Expected compilation error") - assert.Contains(t, err.Error(), tt.errorContains, "Error should contain expected message") + require.ErrorContains(t, err, tt.errorContains, "Error should contain expected message") } else { assert.NoError(t, err, "Expected successful compilation") } diff --git a/pkg/workflow/github_cli_test.go b/pkg/workflow/github_cli_test.go index b35856a8ef1..047c74520bf 100644 --- a/pkg/workflow/github_cli_test.go +++ b/pkg/workflow/github_cli_test.go @@ -410,8 +410,8 @@ func TestEnrichGHError(t *testing.T) { _, cmdErr := cmd.Output() require.Error(t, cmdErr, "command should fail") enriched := enrichGHError(cmdErr) - assert.Contains(t, enriched.Error(), "not found", "enriched error should contain stderr output") - assert.Contains(t, enriched.Error(), "exit status 1", "enriched error should still contain original error") + require.ErrorContains(t, enriched, "not found", "enriched error should contain stderr output") + require.ErrorContains(t, enriched, "exit status 1", "enriched error should still contain original error") }) } diff --git a/pkg/workflow/github_mcp_app_token_test.go b/pkg/workflow/github_mcp_app_token_test.go index e4379cd5e74..21aaba6deb8 100644 --- a/pkg/workflow/github_mcp_app_token_test.go +++ b/pkg/workflow/github_mcp_app_token_test.go @@ -150,7 +150,7 @@ Test that setting both app and github-token is an error. // Compile the workflow - should fail because both app and github-token are set err = compiler.CompileWorkflow(testFile) require.Error(t, err, "Expected error when both app and github-token are set") - assert.Contains(t, err.Error(), "'tools.github.github-app' and 'tools.github.github-token' cannot both be set", "Error should mention mutual exclusion") + require.ErrorContains(t, err, "'tools.github.github-app' and 'tools.github.github-token' cannot both be set", "Error should mention mutual exclusion") } // TestGitHubMCPAppTokenWithRemoteMode tests that app token works with remote mode @@ -532,9 +532,9 @@ Test that write is rejected in tools.github.github-app.permissions. err = compiler.CompileWorkflow(testFile) require.Error(t, err, "Compiler should reject write in tools.github.github-app.permissions") - assert.Contains(t, err.Error(), "Invalid permission levels in tools.github.github-app.permissions", "Error should mention invalid permission levels") - assert.Contains(t, err.Error(), `"write" is not allowed`, "Error should mention that write is not allowed") - assert.Contains(t, err.Error(), "members", "Error should mention the offending scope") + require.ErrorContains(t, err, "Invalid permission levels in tools.github.github-app.permissions", "Error should mention invalid permission levels") + require.ErrorContains(t, err, `"write" is not allowed`, "Error should mention that write is not allowed") + require.ErrorContains(t, err, "members", "Error should mention the offending scope") } // TestCheckoutAppTokensMintedInAgentJob verifies that checkout-related GitHub App token diff --git a/pkg/workflow/gvisor_test.go b/pkg/workflow/gvisor_test.go index 81e93698341..77ad3891a14 100644 --- a/pkg/workflow/gvisor_test.go +++ b/pkg/workflow/gvisor_test.go @@ -199,8 +199,8 @@ func TestGVisorValidation_ArcDindIncompatible(t *testing.T) { err := validateSandboxConfig(workflowData) require.Error(t, err, "gVisor + arc-dind must produce a compile-time error") - assert.Contains(t, err.Error(), "arc-dind", "error must mention arc-dind") - assert.Contains(t, err.Error(), "gvisor", "error must mention gvisor") + require.ErrorContains(t, err, "arc-dind", "error must mention arc-dind") + require.ErrorContains(t, err, "gvisor", "error must mention gvisor") } // TestGVisorValidation_SudoFalseAllowed verifies that gVisor + sudo:false (default) is @@ -265,7 +265,7 @@ func TestGVisorStrictModeSudoTrueError(t *testing.T) { err := compiler.validateStrictSandboxCustomization(sandboxConfig) require.Error(t, err, "sudo:true + runtime:gvisor must still produce a strict-mode error") - assert.Contains(t, err.Error(), "sudo", "error must mention sudo") + require.ErrorContains(t, err, "sudo", "error must mention sudo") } // TestGVisorFrontmatterExtraction verifies end-to-end that a workflow with diff --git a/pkg/workflow/imported_steps_validation_test.go b/pkg/workflow/imported_steps_validation_test.go index cf64a6599e3..7430f8cfbcf 100644 --- a/pkg/workflow/imported_steps_validation_test.go +++ b/pkg/workflow/imported_steps_validation_test.go @@ -252,7 +252,7 @@ func TestValidateCheckoutPersistCredentials_FrontmatterSteps(t *testing.T) { if tt.expectError { require.Error(t, err, "Expected an error but got none") - assert.Contains(t, err.Error(), tt.errorMsg, "Error message mismatch") + require.ErrorContains(t, err, tt.errorMsg, "Error message mismatch") } else { assert.NoError(t, err, "Expected no error but got: %v", err) } @@ -332,7 +332,7 @@ func TestValidateCheckoutPersistCredentials_MergedSteps(t *testing.T) { if tt.expectError { require.Error(t, err, "Expected an error but got none") - assert.Contains(t, err.Error(), tt.errorMsg, "Error message mismatch") + require.ErrorContains(t, err, tt.errorMsg, "Error message mismatch") } else { assert.NoError(t, err, "Expected no error but got: %v", err) } @@ -386,7 +386,7 @@ func TestValidateCheckoutPersistCredentials_BothSourcesChecked(t *testing.T) { err := compiler.validateCheckoutPersistCredentials(frontmatter, mergedSteps) require.Error(t, err, "Should error when imported step has insecure checkout") - assert.Contains(t, err.Error(), "'Insecure Imported Checkout'", "Error should reference the insecure imported step") + require.ErrorContains(t, err, "'Insecure Imported Checkout'", "Error should reference the insecure imported step") assert.NotContains(t, err.Error(), "'Safe Checkout'", "Error should not reference the safe step") } @@ -410,8 +410,8 @@ func TestValidateCheckoutPersistCredentials_MultipleOffenders(t *testing.T) { err := compiler.validateCheckoutPersistCredentials(frontmatter, "") require.Error(t, err, "Should error when multiple steps have insecure checkout") - assert.Contains(t, err.Error(), "'First Checkout'", "Error should mention first step") - assert.Contains(t, err.Error(), "'Second Checkout'", "Error should mention second step") + require.ErrorContains(t, err, "'First Checkout'", "Error should mention first step") + require.ErrorContains(t, err, "'Second Checkout'", "Error should mention second step") } // TestStepDisplayName tests the stepDisplayName helper function @@ -490,5 +490,5 @@ func TestValidateCheckoutPersistCredentials_GitLeakErrorMessage(t *testing.T) { strings.Contains(err.Error(), "git token") || strings.Contains(err.Error(), ".git/config"), "Error should mention git token leak", ) - assert.Contains(t, err.Error(), "persist-credentials: false", "Error should mention the fix") + require.ErrorContains(t, err, "persist-credentials: false", "Error should mention the fix") } diff --git a/pkg/workflow/imports_env_test.go b/pkg/workflow/imports_env_test.go index 637edc55dd7..b3927d5efcd 100644 --- a/pkg/workflow/imports_env_test.go +++ b/pkg/workflow/imports_env_test.go @@ -77,7 +77,7 @@ func TestMergeEnvWithInvalidJSON(t *testing.T) { _, err := mergeEnv(topEnv, `{invalid json}`) require.Error(t, err, "mergeEnv should return an error for invalid JSON") - assert.Contains(t, err.Error(), "failed to parse imported env JSON", "Error message should be descriptive") + require.ErrorContains(t, err, "failed to parse imported env JSON", "Error message should be descriptive") } func TestMergeEnvNormalizesImportedWorkflowEnvReferences(t *testing.T) { diff --git a/pkg/workflow/inline_imports_test.go b/pkg/workflow/inline_imports_test.go index 96364357cab..10217d27502 100644 --- a/pkg/workflow/inline_imports_test.go +++ b/pkg/workflow/inline_imports_test.go @@ -80,9 +80,9 @@ Do something. _, err := compiler.ParseWorkflowFile(workflowFile) require.Error(t, err, "should return an error when inlined-imports is used with an agent file") - assert.Contains(t, err.Error(), "inlined-imports cannot be used with agent file imports", + require.ErrorContains(t, err, "inlined-imports cannot be used with agent file imports", "error message should explain the conflict") - assert.Contains(t, err.Error(), "my-agent.md", + require.ErrorContains(t, err, "my-agent.md", "error message should include the agent file path") } diff --git a/pkg/workflow/label_command_test.go b/pkg/workflow/label_command_test.go index fe8b2d62a47..9dbd26b57b1 100644 --- a/pkg/workflow/label_command_test.go +++ b/pkg/workflow/label_command_test.go @@ -460,7 +460,7 @@ This should fail validation. compiler := NewCompiler() err = compiler.CompileWorkflow(workflowPath) require.Error(t, err, "CompileWorkflow() should error when label_command is combined with non-label issues trigger") - assert.Contains(t, err.Error(), "label_command", "error should mention label_command") + require.ErrorContains(t, err, "label_command", "error should mention label_command") } // TestLabelCommandRemoveLabelDisabled verifies that setting remove_label: false in the object form diff --git a/pkg/workflow/labels_validation_test.go b/pkg/workflow/labels_validation_test.go index 86f666841a0..287b54f3307 100644 --- a/pkg/workflow/labels_validation_test.go +++ b/pkg/workflow/labels_validation_test.go @@ -76,7 +76,7 @@ func TestValidateLabels(t *testing.T) { if tt.shouldErr { require.Error(t, err, "Expected validation to fail") if tt.errorMsg != "" { - assert.Contains(t, err.Error(), tt.errorMsg, "Error message should contain expected text") + require.ErrorContains(t, err, tt.errorMsg, "Error message should contain expected text") } } else { assert.NoError(t, err, "Expected validation to pass") diff --git a/pkg/workflow/lock_schema_test.go b/pkg/workflow/lock_schema_test.go index da7b3811ff6..661201baa2e 100644 --- a/pkg/workflow/lock_schema_test.go +++ b/pkg/workflow/lock_schema_test.go @@ -219,7 +219,7 @@ name: test if tt.expectError { require.Error(t, err, "Expected validation error") if tt.errorText != "" { - assert.Contains(t, err.Error(), tt.errorText, "Error message should contain expected text") + require.ErrorContains(t, err, tt.errorText, "Error message should contain expected text") } } else { require.NoError(t, err, "Should not error on compatible schema") diff --git a/pkg/workflow/lsp_validation_test.go b/pkg/workflow/lsp_validation_test.go index 3083eb48548..0cb574a8f9d 100644 --- a/pkg/workflow/lsp_validation_test.go +++ b/pkg/workflow/lsp_validation_test.go @@ -5,7 +5,6 @@ package workflow import ( "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -36,7 +35,7 @@ func TestValidateLSPSupport(t *testing.T) { LSP: validLSP, }) require.Error(t, err) - assert.Contains(t, err.Error(), "only supported for engine: copilot") + require.ErrorContains(t, err, "only supported for engine: copilot") }) t.Run("invalid lsp config fails validation", func(t *testing.T) { @@ -52,6 +51,6 @@ func TestValidateLSPSupport(t *testing.T) { }, }) require.Error(t, err) - assert.Contains(t, err.Error(), "lsp.python.command is required") + require.ErrorContains(t, err, "lsp.python.command is required") }) } diff --git a/pkg/workflow/mcp_config_validation_test.go b/pkg/workflow/mcp_config_validation_test.go index 2ec7d4fd70d..871cf6ebd42 100644 --- a/pkg/workflow/mcp_config_validation_test.go +++ b/pkg/workflow/mcp_config_validation_test.go @@ -129,7 +129,7 @@ func TestValidateMCPMountsSyntax(t *testing.T) { if tt.wantErr { require.Error(t, err, "expected an error") if tt.errMsg != "" { - assert.Contains(t, err.Error(), tt.errMsg, + require.ErrorContains(t, err, tt.errMsg, "error message should contain %q", tt.errMsg) } } else { diff --git a/pkg/workflow/model_alias_validation_test.go b/pkg/workflow/model_alias_validation_test.go index 5eda46963fa..008946d4d15 100644 --- a/pkg/workflow/model_alias_validation_test.go +++ b/pkg/workflow/model_alias_validation_test.go @@ -60,7 +60,7 @@ func TestParseModelIdentifier_T_MAF_005(t *testing.T) { "/fake/path/workflow.md", ) require.Error(t, err, "glob pattern in engine.model should be rejected (V-MAF-004)") - assert.Contains(t, err.Error(), "V-MAF-004", "error should reference V-MAF-004") + require.ErrorContains(t, err, "V-MAF-004", "error should reference V-MAF-004") } // TestParseModelIdentifier_T_MAF_006 – invalid effort value must be rejected. @@ -72,7 +72,7 @@ func TestParseModelIdentifier_T_MAF_006(t *testing.T) { p, _ := ParseModelIdentifier("opus?effort=extreme") err = ValidateKnownParams(p.Params) require.Error(t, err, "effort=extreme should be rejected (V-MAF-002)") - assert.Contains(t, err.Error(), "V-MAF-002", "error should reference V-MAF-002") + require.ErrorContains(t, err, "V-MAF-002", "error should reference V-MAF-002") } // TestParseModelIdentifier_T_MAF_007 – temperature out of range must be rejected. @@ -82,22 +82,22 @@ func TestParseModelIdentifier_T_MAF_007(t *testing.T) { err = ValidateKnownParams(p.Params) require.Error(t, err, "temperature=3.0 should be rejected (V-MAF-003)") - assert.Contains(t, err.Error(), "V-MAF-003", "error should reference V-MAF-003") + require.ErrorContains(t, err, "V-MAF-003", "error should reference V-MAF-003") } // TestParseModelIdentifier_T_MAF_008 – whitespace in identifier must be rejected. func TestParseModelIdentifier_T_MAF_008(t *testing.T) { _, err := ParseModelIdentifier("my model") require.Error(t, err, "whitespace in model identifier should be rejected (V-MAF-006)") - assert.Contains(t, err.Error(), "segment type", "error should name the segment type (V-MAF-006)") + require.ErrorContains(t, err, "segment type", "error should name the segment type (V-MAF-006)") } // TestParseModelIdentifier_T_MAF_009 – colon in identifier must be rejected; error must name the char. func TestParseModelIdentifier_T_MAF_009(t *testing.T) { _, err := ParseModelIdentifier("my:model") require.Error(t, err, "colon in model identifier should be rejected (V-MAF-006)") - assert.Contains(t, err.Error(), ":", "error message must identify the offending character (V-MAF-006)") - assert.Contains(t, err.Error(), "segment type", "error must name the segment type (V-MAF-006)") + require.ErrorContains(t, err, ":", "error message must identify the offending character (V-MAF-006)") + require.ErrorContains(t, err, "segment type", "error must name the segment type (V-MAF-006)") } // ─── Additional syntax tests ────────────────────────────────────────────────── @@ -268,7 +268,7 @@ func TestValidateAliasKey(t *testing.T) { err := validateAliasKey(tt.key, "/fake/path.md") if tt.wantErr { require.Error(t, err, "alias key %q should fail validation (V-MAF-005)", tt.key) - assert.Contains(t, err.Error(), "V-MAF-005", "error should reference V-MAF-005") + require.ErrorContains(t, err, "V-MAF-005", "error should reference V-MAF-005") } else { assert.NoError(t, err, "alias key %q should pass validation", tt.key) } @@ -286,7 +286,7 @@ func TestDetectCircularModelAliases_T_MAF_040(t *testing.T) { } err := detectCircularModelAliases(aliasMap, "/fake/path.md") require.Error(t, err, "2-node cycle a → b → a must be detected (T-MAF-040)") - assert.Contains(t, err.Error(), "V-MAF-010", "error should reference V-MAF-010") + require.ErrorContains(t, err, "V-MAF-010", "error should reference V-MAF-010") } // T-MAF-041: longer 3-node cycle must be detected; error message names all aliases. @@ -299,9 +299,9 @@ func TestDetectCircularModelAliases_T_MAF_041(t *testing.T) { err := detectCircularModelAliases(aliasMap, "/fake/path.md") require.Error(t, err, "3-node cycle a → b → c → a must be detected (T-MAF-041)") // Error message must name all three aliases. - assert.Contains(t, err.Error(), "a", "cycle error should name alias 'a'") - assert.Contains(t, err.Error(), "b", "cycle error should name alias 'b'") - assert.Contains(t, err.Error(), "c", "cycle error should name alias 'c'") + require.ErrorContains(t, err, "a", "cycle error should name alias 'a'") + require.ErrorContains(t, err, "b", "cycle error should name alias 'b'") + require.ErrorContains(t, err, "c", "cycle error should name alias 'c'") } // Acyclic map should not produce an error. @@ -696,8 +696,8 @@ func TestValidateModelAliasMap_EngineModelExpressionForms(t *testing.T) { ) if tt.wantErr { require.Error(t, err, "engine.model=%q should be rejected", tt.engineModel) - assert.Contains(t, err.Error(), "V-MAF-004", "engine.model=%q error should reference V-MAF-004", tt.engineModel) - assert.Contains(t, err.Error(), tt.engineModel, "engine.model error should quote the offending value") + require.ErrorContains(t, err, "V-MAF-004", "engine.model=%q error should reference V-MAF-004", tt.engineModel) + require.ErrorContains(t, err, tt.engineModel, "engine.model error should quote the offending value") } else { assert.NoError(t, err, "engine.model=%q should be accepted", tt.engineModel) } diff --git a/pkg/workflow/network_firewall_validation_test.go b/pkg/workflow/network_firewall_validation_test.go index 0e97e45255d..7313bd4e285 100644 --- a/pkg/workflow/network_firewall_validation_test.go +++ b/pkg/workflow/network_firewall_validation_test.go @@ -22,8 +22,8 @@ func TestValidateNetworkFirewallConfig_AllowURLsRequiresSSLBump(t *testing.T) { err := validateNetworkFirewallConfig(networkPermissions) require.Error(t, err, "Expected validation error when allow-urls is specified without ssl-bump") - assert.Contains(t, err.Error(), "allow-urls requires ssl-bump: true", "Error should mention the ssl-bump requirement") - assert.Contains(t, err.Error(), "network.firewall.allow-urls", "Error should identify the field") + require.ErrorContains(t, err, "allow-urls requires ssl-bump: true", "Error should mention the ssl-bump requirement") + require.ErrorContains(t, err, "network.firewall.allow-urls", "Error should identify the field") }) t.Run("allow-urls with ssl-bump passes validation", func(t *testing.T) { @@ -56,7 +56,7 @@ func TestValidateNetworkFirewallConfig_AllowURLsRequiresSSLBump(t *testing.T) { err := validateNetworkFirewallConfig(networkPermissions) require.Error(t, err, "Expected validation error when multiple allow-urls are specified without ssl-bump") - assert.Contains(t, err.Error(), "allow-urls requires ssl-bump: true", "Error should mention the ssl-bump requirement") + require.ErrorContains(t, err, "allow-urls requires ssl-bump: true", "Error should mention the ssl-bump requirement") }) t.Run("multiple allow-urls with ssl-bump passes validation", func(t *testing.T) { @@ -147,7 +147,7 @@ func TestValidateNetworkFirewallConfig_AllowURLsRequiresSSLBump(t *testing.T) { err := validateNetworkFirewallConfig(networkPermissions) require.Error(t, err, "Expected validation error even when firewall is disabled") - assert.Contains(t, err.Error(), "allow-urls requires ssl-bump: true", "Error should mention the ssl-bump requirement") + require.ErrorContains(t, err, "allow-urls requires ssl-bump: true", "Error should mention the ssl-bump requirement") }) } @@ -181,7 +181,7 @@ func TestValidateNetworkAllowedDomains_EcosystemIdentifiers(t *testing.T) { network := &NetworkPermissions{Allowed: []string{ecosystem}} err := compiler.validateNetworkAllowedDomains(network) require.Error(t, err, "Unknown ecosystem identifier '%s' should fail validation", ecosystem) - assert.Contains(t, err.Error(), "not a valid ecosystem identifier", "Error should indicate invalid ecosystem identifier") + require.ErrorContains(t, err, "not a valid ecosystem identifier", "Error should indicate invalid ecosystem identifier") }) } }) @@ -203,8 +203,8 @@ func TestValidateNetworkAllowedDomains_EcosystemIdentifiers(t *testing.T) { } err := compiler.validateNetworkAllowedDomains(network) require.Error(t, err, "Should fail when invalid ecosystem identifiers are present") - assert.Contains(t, err.Error(), "rustxxxx", "Error should mention the invalid identifier") - assert.Contains(t, err.Error(), "fakeecosystem", "Error should mention the other invalid identifier") + require.ErrorContains(t, err, "rustxxxx", "Error should mention the invalid identifier") + require.ErrorContains(t, err, "fakeecosystem", "Error should mention the other invalid identifier") }) } @@ -231,7 +231,7 @@ func TestValidateNetworkFirewallConfig_Integration(t *testing.T) { err := validateNetworkFirewallConfig(workflowData.NetworkPermissions) require.Error(t, err, "Compiler should reject workflow with allow-urls but no ssl-bump") - assert.Contains(t, err.Error(), "allow-urls requires ssl-bump: true", "Error should explain the requirement") + require.ErrorContains(t, err, "allow-urls requires ssl-bump: true", "Error should explain the requirement") }) t.Run("compiler accepts workflow with allow-urls and ssl-bump", func(t *testing.T) { diff --git a/pkg/workflow/observability_otlp_test.go b/pkg/workflow/observability_otlp_test.go index 40cf59b0c67..ea516a77927 100644 --- a/pkg/workflow/observability_otlp_test.go +++ b/pkg/workflow/observability_otlp_test.go @@ -2058,7 +2058,7 @@ func TestValidateOTLPResourceAttributes(t *testing.T) { return } require.Error(t, err) - assert.Contains(t, err.Error(), tt.errorContains) + require.ErrorContains(t, err, tt.errorContains) }) } } diff --git a/pkg/workflow/on_needs_validation_test.go b/pkg/workflow/on_needs_validation_test.go index 6657b8d7e2f..eb63eaf47ed 100644 --- a/pkg/workflow/on_needs_validation_test.go +++ b/pkg/workflow/on_needs_validation_test.go @@ -5,7 +5,6 @@ package workflow import ( "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -31,7 +30,7 @@ func TestValidateOnNeedsTargets(t *testing.T) { err := validateOnNeedsTargets(data) require.Error(t, err, "expected on.needs validation error") - assert.Contains(t, err.Error(), `built-in job "activation"`, "error should explain invalid built-in target") + require.ErrorContains(t, err, `built-in job "activation"`, "error should explain invalid built-in target") }) t.Run("target depending on activation rejected", func(t *testing.T) { @@ -46,7 +45,7 @@ func TestValidateOnNeedsTargets(t *testing.T) { err := validateOnNeedsTargets(data) require.Error(t, err, "expected on.needs validation error") - assert.Contains(t, err.Error(), "cannot depend on activation/pre_activation", "error should explain cyclic dependency risk") + require.ErrorContains(t, err, "cannot depend on activation/pre_activation", "error should explain cyclic dependency risk") }) } @@ -68,8 +67,8 @@ func TestValidateOnNeedsDependencyChains(t *testing.T) { err := c.validateOnNeeds(data) require.Error(t, err, "expected transitive chain validation error") - assert.Contains(t, err.Error(), `depends on "bootstrap"`, "error should identify problematic transitive dependency") - assert.Contains(t, err.Error(), "implicit needs: activation", "error should explain cycle-prone implicit activation dependency") + require.ErrorContains(t, err, `depends on "bootstrap"`, "error should identify problematic transitive dependency") + require.ErrorContains(t, err, "implicit needs: activation", "error should explain cycle-prone implicit activation dependency") }) t.Run("allows chain when transitive dependency is explicitly in on.needs", func(t *testing.T) { @@ -124,7 +123,7 @@ func TestValidateOnGitHubAppNeedsExpressions(t *testing.T) { err := c.validateOnNeeds(data) require.Error(t, err, "expected on.github-app validation error") - assert.Contains(t, err.Error(), `unknown job "missing_job"`, "error should identify unknown needs job") + require.ErrorContains(t, err, `unknown job "missing_job"`, "error should identify unknown needs job") }) t.Run("error field label uses client-id", func(t *testing.T) { @@ -142,6 +141,6 @@ func TestValidateOnGitHubAppNeedsExpressions(t *testing.T) { err := c.validateOnNeeds(data) require.Error(t, err, "expected on.github-app validation error") - assert.Contains(t, err.Error(), "on.github-app.client-id", "error field should use yaml key client-id") + require.ErrorContains(t, err, "on.github-app.client-id", "error field should use yaml key client-id") }) } diff --git a/pkg/workflow/on_steps_test.go b/pkg/workflow/on_steps_test.go index 5a180199dc7..51e05611b87 100644 --- a/pkg/workflow/on_steps_test.go +++ b/pkg/workflow/on_steps_test.go @@ -630,7 +630,7 @@ func TestExtractOnNeeds(t *testing.T) { needs, err := extractOnNeeds(tt.frontmatter) if tt.expectError { require.Error(t, err, "expected extraction error") - assert.Contains(t, err.Error(), tt.errorContains, "error should contain expected text") + require.ErrorContains(t, err, tt.errorContains, "error should contain expected text") return } @@ -704,7 +704,7 @@ func TestExtractOnRestoreMemory(t *testing.T) { restoreMemory, err := extractOnRestoreMemory(tt.frontmatter) if tt.expectError { require.Error(t, err, "expected extraction error") - assert.Contains(t, err.Error(), tt.errorContains, "error should contain expected text") + require.ErrorContains(t, err, tt.errorContains, "error should contain expected text") return } diff --git a/pkg/workflow/permissions_scope_validation_test.go b/pkg/workflow/permissions_scope_validation_test.go index 8a70cd4c14f..01e2cafbe91 100644 --- a/pkg/workflow/permissions_scope_validation_test.go +++ b/pkg/workflow/permissions_scope_validation_test.go @@ -82,7 +82,7 @@ pull-requests: read`, if tt.wantErr { require.Error(t, err, "ValidatePermissionScopeNames should return an error") if tt.errContains != "" { - assert.Contains(t, err.Error(), tt.errContains, + require.ErrorContains(t, err, tt.errContains, "error should contain %q", tt.errContains) } } else { diff --git a/pkg/workflow/playwright_validation_test.go b/pkg/workflow/playwright_validation_test.go index 0a41219c1d8..b6d9bee762d 100644 --- a/pkg/workflow/playwright_validation_test.go +++ b/pkg/workflow/playwright_validation_test.go @@ -81,7 +81,7 @@ func TestValidatePlaywrightMode(t *testing.T) { if tt.expectError { require.Error(t, err, "expected an error but got none") - assert.Contains(t, err.Error(), tt.errorSubstr, + require.ErrorContains(t, err, tt.errorSubstr, "error %q should contain %q", err.Error(), tt.errorSubstr) } else { assert.NoError(t, err, "expected no error") diff --git a/pkg/workflow/private_to_public_flows_test.go b/pkg/workflow/private_to_public_flows_test.go index 195ac9366f2..e2457e629c5 100644 --- a/pkg/workflow/private_to_public_flows_test.go +++ b/pkg/workflow/private_to_public_flows_test.go @@ -273,7 +273,7 @@ tools: err = compiler.CompileWorkflow(tmpFile.Name()) require.Error(t, err, "strict mode should reject private-to-public-flows: allow") - assert.Contains(t, err.Error(), "private-to-public-flows") + require.ErrorContains(t, err, "private-to-public-flows") }) t.Run("list form is accepted in strict mode", func(t *testing.T) { @@ -370,7 +370,7 @@ func TestValidatePrivateToPublicFlowsServerIDs(t *testing.T) { } err := validatePrivateToPublicFlowsServerIDs(wd) require.Error(t, err, "undeclared server ID should be rejected") - assert.Contains(t, err.Error(), "undeclared-server") + require.ErrorContains(t, err, "undeclared-server") }) t.Run("allow string form skipped", func(t *testing.T) { @@ -407,8 +407,8 @@ func TestValidatePrivateToPublicFlowsStringValue(t *testing.T) { } err := validatePrivateToPublicFlowsStringValue(wd) require.Error(t, err) - assert.Contains(t, err.Error(), "invalid value") - assert.Contains(t, err.Error(), "Allow") + require.ErrorContains(t, err, "invalid value") + require.ErrorContains(t, err, "Allow") }) t.Run("list form skipped", func(t *testing.T) { diff --git a/pkg/workflow/replace_label_formal_test.go b/pkg/workflow/replace_label_formal_test.go index a97450e23e2..5761c48a79a 100644 --- a/pkg/workflow/replace_label_formal_test.go +++ b/pkg/workflow/replace_label_formal_test.go @@ -313,10 +313,10 @@ func runReplaceLabelFixture(t *testing.T, fixtureName string) { if scenario.Expected.ErrorCode != nil { switch *scenario.Expected.ErrorCode { case -32003: - assert.Contains(t, evalErr.Error(), "blocked pattern", + require.ErrorContains(t, evalErr, "blocked pattern", "error_code -32003 requires a blocked-pattern denial (blocklist evaluated first)") case -32002: - assert.Contains(t, evalErr.Error(), "allowed list", + require.ErrorContains(t, evalErr, "allowed list", "error_code -32002 requires an allowed-list denial") } } @@ -394,13 +394,13 @@ func TestFormalReplaceLabelP4_AllowlistEnforcement(t *testing.T) { err = formalValidateSingleLabel("needs-triage", []string{"state-*"}, nil, "label_to_add") require.Error(t, err) - assert.Contains(t, err.Error(), "allowed list") + require.ErrorContains(t, err, "allowed list") } func TestFormalReplaceLabelP5_BlocklistPriority(t *testing.T) { err := formalValidateSingleLabel("state-internal", []string{"state-*"}, []string{"state-in*"}, "label_to_add") require.Error(t, err) - assert.Contains(t, err.Error(), "blocked pattern") + require.ErrorContains(t, err, "blocked pattern") } func TestFormalReplaceLabelP6_RemoveAllowlist(t *testing.T) { @@ -670,5 +670,5 @@ func TestFormalFixtureLoaderRejectsMalformedYAML(t *testing.T) { func TestFormalFixtureLoaderRejectsEmptyScenarios(t *testing.T) { _, err := parseReplaceLabelFixture([]byte("fixture_id: test\nscenarios: []\n")) require.Error(t, err) - assert.Contains(t, err.Error(), "no scenarios") + require.ErrorContains(t, err, "no scenarios") } diff --git a/pkg/workflow/repo_config_test.go b/pkg/workflow/repo_config_test.go index 9398890f4ce..e8a0208f7c7 100644 --- a/pkg/workflow/repo_config_test.go +++ b/pkg/workflow/repo_config_test.go @@ -212,7 +212,7 @@ func TestLoadRepoConfig_DisabledJobsRejectsInvalidOrDuplicateValues(t *testing.T _, err := LoadRepoConfig(dir) require.Error(t, err) - assert.ErrorContains(t, err, tt.contains) + require.ErrorContains(t, err, tt.contains) }) } } @@ -295,7 +295,7 @@ func TestLoadRepoConfig_InvalidUTC(t *testing.T) { _, err := LoadRepoConfig(dir) require.Error(t, err, "invalid timezone should return an error") - assert.Contains(t, err.Error(), "utc must be a numeric UTC offset") + require.ErrorContains(t, err, "utc must be a numeric UTC offset") } // TestFormatRunsOn tests the YAML serialisation of runs-on values. diff --git a/pkg/workflow/repo_memory_test.go b/pkg/workflow/repo_memory_test.go index d75d91d5b9a..9f1e4ef4edf 100644 --- a/pkg/workflow/repo_memory_test.go +++ b/pkg/workflow/repo_memory_test.go @@ -762,7 +762,7 @@ func TestRepoMemoryMaxPatchSizeValidation(t *testing.T) { if tt.wantError { require.Error(t, err, "Should return an error") if err != nil { - assert.Contains(t, err.Error(), tt.errorText, "Error message should match") + require.ErrorContains(t, err, tt.errorText, "Error message should match") } } else { require.NoError(t, err, "Should not return an error") @@ -826,7 +826,7 @@ func TestRepoMemoryMaxPatchSizeValidationArray(t *testing.T) { if tt.wantError { require.Error(t, err, "Should return an error") if err != nil { - assert.Contains(t, err.Error(), tt.errorText, "Error message should match") + require.ErrorContains(t, err, tt.errorText, "Error message should match") } } else { require.NoError(t, err, "Should not return an error") @@ -946,7 +946,7 @@ func TestBranchPrefixValidation(t *testing.T) { err := validateBranchPrefix(tt.prefix) if tt.wantErr { require.Error(t, err, "Expected error for prefix: %s", tt.prefix) - assert.Contains(t, err.Error(), tt.errMsg, "Error message should contain: %s", tt.errMsg) + require.ErrorContains(t, err, tt.errMsg, "Error message should contain: %s", tt.errMsg) } else { require.NoError(t, err, "Expected no error for prefix: %s", tt.prefix) } @@ -1624,7 +1624,7 @@ func TestValidateFileGlobPatterns(t *testing.T) { err := validateFileGlobPatterns(tt.patterns) if tt.wantErr { require.Error(t, err, "Expected error for patterns: %v", tt.patterns) - assert.Contains(t, err.Error(), tt.errMsg, "Error message should contain: %s", tt.errMsg) + require.ErrorContains(t, err, tt.errMsg, "Error message should contain: %s", tt.errMsg) } else { require.NoError(t, err, "Expected no error for patterns: %v", tt.patterns) } @@ -1649,7 +1649,7 @@ func TestFileGlobPatternValidationInConfig(t *testing.T) { compiler := NewCompiler() _, err = compiler.extractRepoMemoryConfig(toolsConfig, "test-workflow") require.Error(t, err) - assert.Contains(t, err.Error(), "must not start with '/'") + require.ErrorContains(t, err, "must not start with '/'") }) t.Run("rejects absolute path in object notation", func(t *testing.T) { @@ -1663,7 +1663,7 @@ func TestFileGlobPatternValidationInConfig(t *testing.T) { compiler := NewCompiler() _, err = compiler.extractRepoMemoryConfig(toolsConfig, "test-workflow") require.Error(t, err) - assert.Contains(t, err.Error(), "must not start with '/'") + require.ErrorContains(t, err, "must not start with '/'") }) t.Run("accepts valid slashless patterns in array notation", func(t *testing.T) { diff --git a/pkg/workflow/resolve_test.go b/pkg/workflow/resolve_test.go index 7e5455cddad..2cc15468886 100644 --- a/pkg/workflow/resolve_test.go +++ b/pkg/workflow/resolve_test.go @@ -165,7 +165,7 @@ func TestResolveWorkflowName_MissingLockFile(t *testing.T) { // Test that it returns an error when lock file is missing _, err := ResolveWorkflowName("incomplete-workflow") require.Error(t, err, "ResolveWorkflowName should return an error when lock file is missing") - assert.Contains(t, err.Error(), "Run 'gh aw compile'", "error should mention compilation when lock file is missing") + require.ErrorContains(t, err, "Run 'gh aw compile'", "error should mention compilation when lock file is missing") } func TestResolveWorkflowName_InvalidYAML(t *testing.T) { @@ -181,7 +181,7 @@ func TestResolveWorkflowName_InvalidYAML(t *testing.T) { // Test that it returns an error when YAML is invalid _, err := ResolveWorkflowName("invalid-yaml") require.Error(t, err, "ResolveWorkflowName should return an error when YAML is invalid") - assert.Contains(t, err.Error(), "failed to parse YAML", "error should mention YAML parsing failure") + require.ErrorContains(t, err, "failed to parse YAML", "error should mention YAML parsing failure") } func TestResolveWorkflowName_MissingNameField(t *testing.T) { @@ -197,7 +197,7 @@ func TestResolveWorkflowName_MissingNameField(t *testing.T) { // Test that it returns an error when name field is missing _, err := ResolveWorkflowName("no-name") require.Error(t, err, "ResolveWorkflowName should return an error when name field is missing") - assert.Contains(t, err.Error(), "workflow name not found", "error should mention missing workflow name") + require.ErrorContains(t, err, "workflow name not found", "error should mention missing workflow name") } func TestResolveWorkflowName_ExistingAgenticWorkflow(t *testing.T) { diff --git a/pkg/workflow/run_install_scripts_validation_test.go b/pkg/workflow/run_install_scripts_validation_test.go index 8115a4c9755..f323c769bdc 100644 --- a/pkg/workflow/run_install_scripts_validation_test.go +++ b/pkg/workflow/run_install_scripts_validation_test.go @@ -151,8 +151,8 @@ func TestValidateRunInstallScripts_StrictModeError(t *testing.T) { err := c.validateRunInstallScripts(workflowData) require.Error(t, err, "Should return error in strict mode") - assert.Contains(t, err.Error(), "strict mode", "Error should mention strict mode") - assert.Contains(t, err.Error(), "supply chain", "Error should mention supply chain risk") + require.ErrorContains(t, err, "strict mode", "Error should mention strict mode") + require.ErrorContains(t, err, "supply chain", "Error should mention supply chain risk") } func TestValidateRunInstallScripts_NotSet(t *testing.T) { diff --git a/pkg/workflow/runner_config_test.go b/pkg/workflow/runner_config_test.go index 16e58802ab8..5f1b63ead3f 100644 --- a/pkg/workflow/runner_config_test.go +++ b/pkg/workflow/runner_config_test.go @@ -51,8 +51,8 @@ func TestValidateRunnerConfig(t *testing.T) { t.Run("unsupported topology returns error", func(t *testing.T) { err := validateRunnerConfig(&RunnerConfig{Topology: "unknown"}) require.Error(t, err) - assert.Contains(t, err.Error(), "unsupported runner.topology") - assert.Contains(t, err.Error(), "unknown") + require.ErrorContains(t, err, "unsupported runner.topology") + require.ErrorContains(t, err, "unknown") }) } diff --git a/pkg/workflow/runner_topology_validation_test.go b/pkg/workflow/runner_topology_validation_test.go index b039e9c3bfe..3d056665a26 100644 --- a/pkg/workflow/runner_topology_validation_test.go +++ b/pkg/workflow/runner_topology_validation_test.go @@ -31,8 +31,8 @@ func TestValidateArcDindRootless(t *testing.T) { } err := validateArcDindRootless(wd) require.Error(t, err) - assert.Contains(t, err.Error(), "arc-dind") - assert.Contains(t, err.Error(), "sudo") + require.ErrorContains(t, err, "arc-dind") + require.ErrorContains(t, err, "sudo") }) t.Run("error when pre-steps use apt-get install", func(t *testing.T) { @@ -42,8 +42,8 @@ func TestValidateArcDindRootless(t *testing.T) { } err := validateArcDindRootless(wd) require.Error(t, err) - assert.Contains(t, err.Error(), "apt-get install") - assert.Contains(t, err.Error(), "pre-steps") + require.ErrorContains(t, err, "apt-get install") + require.ErrorContains(t, err, "pre-steps") }) t.Run("error when post-steps use sudo", func(t *testing.T) { @@ -53,7 +53,7 @@ func TestValidateArcDindRootless(t *testing.T) { } err := validateArcDindRootless(wd) require.Error(t, err) - assert.Contains(t, err.Error(), "post-steps") + require.ErrorContains(t, err, "post-steps") }) t.Run("no error when steps are empty", func(t *testing.T) { @@ -91,8 +91,8 @@ func TestValidateArcDindRootless(t *testing.T) { } err := validateArcDindRootless(wd) require.Error(t, err) - assert.Contains(t, err.Error(), "arc-dind") - assert.Contains(t, err.Error(), string(constants.AWFArcDindMinVersion)) + require.ErrorContains(t, err, "arc-dind") + require.ErrorContains(t, err, string(constants.AWFArcDindMinVersion)) }) t.Run("no error when arc-dind uses minimum required AWF version", func(t *testing.T) { diff --git a/pkg/workflow/runs_on_validation_test.go b/pkg/workflow/runs_on_validation_test.go index 622684aac0e..e61f523c83b 100644 --- a/pkg/workflow/runs_on_validation_test.go +++ b/pkg/workflow/runs_on_validation_test.go @@ -162,7 +162,7 @@ func TestValidateRunsOn(t *testing.T) { if tt.wantErr { require.Error(t, err, "Test: %s - Expected error but got nil", tt.description) if tt.errorInMsg != "" { - assert.Contains(t, err.Error(), tt.errorInMsg, + require.ErrorContains(t, err, tt.errorInMsg, "Error should contain '%s' for: %s", tt.errorInMsg, tt.description) } } else { @@ -274,7 +274,7 @@ func TestValidateRunsOnValue(t *testing.T) { err := validateRunsOnValue(tt.value) if tt.wantErr { require.Error(t, err) - assert.Contains(t, err.Error(), tt.errContain) + require.ErrorContains(t, err, tt.errContain) return } assert.NoError(t, err) diff --git a/pkg/workflow/runtime_import_validation_test.go b/pkg/workflow/runtime_import_validation_test.go index 21a918bc166..0716631aa2a 100644 --- a/pkg/workflow/runtime_import_validation_test.go +++ b/pkg/workflow/runtime_import_validation_test.go @@ -169,7 +169,7 @@ ${{ github.actor if tt.expectError { require.Error(t, err, "Expected an error") if tt.errorText != "" { - assert.Contains(t, err.Error(), tt.errorText, "Error should contain expected text") + require.ErrorContains(t, err, tt.errorText, "Error should contain expected text") } } else { assert.NoError(t, err, "Expected no error") @@ -268,8 +268,8 @@ Please process the issue. // Should fail due to invalid expression in runtime-import file require.Error(t, err, "Compilation should fail due to invalid expression in runtime-import file") - assert.Contains(t, err.Error(), "runtime-import files contain expression errors", "Error should mention runtime-import files") - assert.Contains(t, err.Error(), "secrets.GITHUB_TOKEN", "Error should mention the specific invalid expression") + require.ErrorContains(t, err, "runtime-import files contain expression errors", "Error should mention runtime-import files") + require.ErrorContains(t, err, "secrets.GITHUB_TOKEN", "Error should mention the specific invalid expression") } // TestCompilerIntegration_RuntimeImportValidation_Valid tests successful compilation diff --git a/pkg/workflow/safe_jobs_needs_validation_test.go b/pkg/workflow/safe_jobs_needs_validation_test.go index cda975439d8..a2f4680a0f4 100644 --- a/pkg/workflow/safe_jobs_needs_validation_test.go +++ b/pkg/workflow/safe_jobs_needs_validation_test.go @@ -267,7 +267,7 @@ func TestValidateSafeJobNeeds_ValidTargets(t *testing.T) { if tt.wantErr { require.Error(t, err, "expected validation error") if tt.errContains != "" { - assert.Contains(t, err.Error(), tt.errContains, + require.ErrorContains(t, err, tt.errContains, "error should contain expected substring") } } else { @@ -377,7 +377,7 @@ func TestValidateSafeOutputsNeeds(t *testing.T) { err := validateSafeOutputsNeeds(tt.data) if tt.wantErr { require.Error(t, err, "expected validation error") - assert.Contains(t, err.Error(), tt.errContains, "error should include expected context") + require.ErrorContains(t, err, tt.errContains, "error should include expected context") return } require.NoError(t, err, "expected validation to pass") @@ -412,7 +412,7 @@ func TestDetectSafeJobCycles(t *testing.T) { } err := detectSafeJobCycles(jobs) require.Error(t, err, "expected cycle error") - assert.Contains(t, err.Error(), "cycle detected", "error should mention cycle") + require.ErrorContains(t, err, "cycle detected", "error should mention cycle") }) t.Run("three-node cycle", func(t *testing.T) { @@ -423,7 +423,7 @@ func TestDetectSafeJobCycles(t *testing.T) { } err := detectSafeJobCycles(jobs) require.Error(t, err, "expected cycle error") - assert.Contains(t, err.Error(), "cycle detected", "error should mention cycle") + require.ErrorContains(t, err, "cycle detected", "error should mention cycle") }) t.Run("empty jobs – no error", func(t *testing.T) { diff --git a/pkg/workflow/safe_outputs_allow_workflows_test.go b/pkg/workflow/safe_outputs_allow_workflows_test.go index 123860ac6eb..e241a343082 100644 --- a/pkg/workflow/safe_outputs_allow_workflows_test.go +++ b/pkg/workflow/safe_outputs_allow_workflows_test.go @@ -173,9 +173,9 @@ func TestAllowWorkflowsValidationRequiresGitHubApp(t *testing.T) { err := validateSafeOutputsAllowWorkflows(tt.safeOutputs) if tt.expectError { require.Error(t, err, "Expected validation error") - assert.Contains(t, err.Error(), "allow-workflows", "Error should mention allow-workflows") - assert.Contains(t, err.Error(), "requires a GitHub App", "Error should mention GitHub App requirement") - assert.Contains(t, err.Error(), "github-app:", "Error should include configuration example") + require.ErrorContains(t, err, "allow-workflows", "Error should mention allow-workflows") + require.ErrorContains(t, err, "requires a GitHub App", "Error should mention GitHub App requirement") + require.ErrorContains(t, err, "github-app:", "Error should include configuration example") } else { assert.NoError(t, err, "Expected no validation error") } @@ -332,5 +332,5 @@ Test workflow with allow-workflows but no GitHub App. err = compiler.CompileWorkflow(mdPath) require.Error(t, err, "Compilation should fail without GitHub App") - assert.Contains(t, err.Error(), "allow-workflows", "Error should mention allow-workflows") + require.ErrorContains(t, err, "allow-workflows", "Error should mention allow-workflows") } diff --git a/pkg/workflow/safe_outputs_allowed_labels_validation_test.go b/pkg/workflow/safe_outputs_allowed_labels_validation_test.go index 76b02182343..2a30a0730b6 100644 --- a/pkg/workflow/safe_outputs_allowed_labels_validation_test.go +++ b/pkg/workflow/safe_outputs_allowed_labels_validation_test.go @@ -114,7 +114,7 @@ strict: false if tt.expectError { require.Error(t, compileErr, "CTR-015: expected error for bare \"*\" in allowed-labels") - assert.Contains(t, compileErr.Error(), "CTR-015", + require.ErrorContains(t, compileErr, "CTR-015", "CTR-015: error message should reference the rule ID") } else { assert.NoError(t, compileErr, diff --git a/pkg/workflow/safe_outputs_config_generation_test.go b/pkg/workflow/safe_outputs_config_generation_test.go index d82c04309b1..9988b4198ba 100644 --- a/pkg/workflow/safe_outputs_config_generation_test.go +++ b/pkg/workflow/safe_outputs_config_generation_test.go @@ -122,8 +122,8 @@ func TestGenerateSafeOutputsConfigActionsCollisionReturnsError(t *testing.T) { _, err := generateSafeOutputsConfig(data) require.Error(t, err, "Expected an error when a custom action name collides with a built-in handler key") - assert.Contains(t, err.Error(), "add-labels", "Error should mention the conflicting action name") - assert.Contains(t, err.Error(), "add_labels", "Error should mention the conflicting normalized name") + require.ErrorContains(t, err, "add-labels", "Error should mention the conflicting action name") + require.ErrorContains(t, err, "add_labels", "Error should mention the conflicting normalized name") } // TestGenerateSafeOutputsConfigMissingToolWithIssue tests the missing_tool config. diff --git a/pkg/workflow/safe_outputs_domains_validation_test.go b/pkg/workflow/safe_outputs_domains_validation_test.go index a211fc9ece0..a8ae529fd14 100644 --- a/pkg/workflow/safe_outputs_domains_validation_test.go +++ b/pkg/workflow/safe_outputs_domains_validation_test.go @@ -189,7 +189,7 @@ func TestValidateSafeOutputsAllowedDomains(t *testing.T) { if tt.wantErr { require.Error(t, err, "Expected an error but got none") if tt.errMsg != "" { - assert.Contains(t, err.Error(), tt.errMsg, "Error message should contain expected text") + require.ErrorContains(t, err, tt.errMsg, "Error message should contain expected text") } } else { assert.NoError(t, err, "Expected no error but got: %v", err) @@ -348,7 +348,7 @@ func TestValidateDomainPattern(t *testing.T) { if tt.wantErr { require.Error(t, err, "Expected an error for domain: %s", tt.domain) if tt.errMsg != "" { - assert.Contains(t, err.Error(), tt.errMsg, "Error message should contain expected text") + require.ErrorContains(t, err, tt.errMsg, "Error message should contain expected text") } } else { assert.NoError(t, err, "Expected no error for domain: %s, but got: %v", tt.domain, err) diff --git a/pkg/workflow/safe_outputs_import_test.go b/pkg/workflow/safe_outputs_import_test.go index c21b165d77e..a67c6358bda 100644 --- a/pkg/workflow/safe_outputs_import_test.go +++ b/pkg/workflow/safe_outputs_import_test.go @@ -269,8 +269,8 @@ imports: // Parse the main workflow - should fail with conflict error _, err = compiler.ParseWorkflowFile("main.md") require.Error(t, err, "Expected conflict error") - assert.Contains(t, err.Error(), "safe-outputs conflict") - assert.Contains(t, err.Error(), "create-issue") + require.ErrorContains(t, err, "safe-outputs conflict") + require.ErrorContains(t, err, "create-issue") } // TestSafeOutputsImportNoConflictDifferentTypes tests that importing different safe-output types does not cause a conflict @@ -550,7 +550,7 @@ func TestMergeSafeOutputsUnit(t *testing.T) { if tt.expectError { require.Error(t, err) - assert.Contains(t, err.Error(), tt.errorContains) + require.ErrorContains(t, err, tt.errorContains) return } @@ -1157,8 +1157,8 @@ safe-outputs: // Parse the main workflow - should fail with conflict error _, err = compiler.ParseWorkflowFile("main.md") require.Error(t, err, "Expected conflict error") - assert.Contains(t, err.Error(), "duplicate-job", "Error should mention the conflicting job name") - assert.Contains(t, err.Error(), "conflict", "Error should mention conflict") + require.ErrorContains(t, err, "duplicate-job", "Error should mention the conflicting job name") + require.ErrorContains(t, err, "conflict", "Error should mention conflict") } // TestSafeOutputsImportMessagesFromSharedWorkflow tests that safe-outputs.messages can be imported from shared workflows @@ -1488,7 +1488,7 @@ func TestMergeSafeOutputsErrorPropagation(t *testing.T) { if tt.expectError { require.Error(t, err, "Expected error") - assert.Contains(t, err.Error(), tt.errorContains, "Error message should contain expected text") + require.ErrorContains(t, err, tt.errorContains, "Error message should contain expected text") return } diff --git a/pkg/workflow/safe_outputs_max_validation_test.go b/pkg/workflow/safe_outputs_max_validation_test.go index 1a8b3d12612..1608c72ddac 100644 --- a/pkg/workflow/safe_outputs_max_validation_test.go +++ b/pkg/workflow/safe_outputs_max_validation_test.go @@ -60,8 +60,8 @@ func TestValidateSafeOutputsMax(t *testing.T) { } err := validateSafeOutputsMax(config) require.Error(t, err, "max: 0 should be invalid") - assert.Contains(t, err.Error(), "max must be a positive integer or -1", "error should explain valid values") - assert.Contains(t, err.Error(), "add-comment", "error should mention the field name") + require.ErrorContains(t, err, "max must be a positive integer or -1", "error should explain valid values") + require.ErrorContains(t, err, "add-comment", "error should mention the field name") }) t.Run("max of -2 is invalid", func(t *testing.T) { @@ -72,7 +72,7 @@ func TestValidateSafeOutputsMax(t *testing.T) { } err := validateSafeOutputsMax(config) require.Error(t, err, "max: -2 should be invalid") - assert.Contains(t, err.Error(), "max must be a positive integer or -1", "error should explain valid values") + require.ErrorContains(t, err, "max must be a positive integer or -1", "error should explain valid values") }) t.Run("max as GitHub Actions expression is skipped", func(t *testing.T) { @@ -106,9 +106,9 @@ func TestValidateSafeOutputsMax(t *testing.T) { } err := validateSafeOutputsMax(config) require.Error(t, err, "dispatch_repository max: 0 should be invalid") - assert.Contains(t, err.Error(), "max must be a positive integer or -1", "error should explain valid values") - assert.Contains(t, err.Error(), "my-tool", "error should mention the tool name") - assert.Contains(t, err.Error(), "dispatch_repository", "error should use underscore form") + require.ErrorContains(t, err, "max must be a positive integer or -1", "error should explain valid values") + require.ErrorContains(t, err, "my-tool", "error should mention the tool name") + require.ErrorContains(t, err, "dispatch_repository", "error should use underscore form") }) t.Run("dispatch_repository tool max of -1 is valid (unlimited)", func(t *testing.T) { @@ -161,7 +161,7 @@ func TestValidateSafeOutputsMax(t *testing.T) { } err := validateSafeOutputsMax(config) require.Error(t, err, "config with one invalid max should return error") - assert.Contains(t, err.Error(), "max must be a positive integer or -1", "error should explain valid values") + require.ErrorContains(t, err, "max must be a positive integer or -1", "error should explain valid values") }) } @@ -226,7 +226,7 @@ func TestValidateSafeOutputsMaxIntegration(t *testing.T) { err := validateSafeOutputsMax(config) require.Error(t, err, "max: 0 should fail validation") - assert.Contains(t, err.Error(), "max must be a positive integer or -1", "error message should explain valid values") + require.ErrorContains(t, err, "max must be a positive integer or -1", "error message should explain valid values") }) t.Run("max of -2 is rejected during config extraction via compiler", func(t *testing.T) { @@ -243,7 +243,7 @@ func TestValidateSafeOutputsMaxIntegration(t *testing.T) { err := validateSafeOutputsMax(config) require.Error(t, err, "max: -2 should fail validation") - assert.Contains(t, err.Error(), "max must be a positive integer or -1", "error message should explain valid values") + require.ErrorContains(t, err, "max must be a positive integer or -1", "error message should explain valid values") }) t.Run("max of -1 passes validation (unlimited)", func(t *testing.T) { diff --git a/pkg/workflow/safe_outputs_steps_shell_expansion_validation_test.go b/pkg/workflow/safe_outputs_steps_shell_expansion_validation_test.go index c90696ae6bd..862284639d2 100644 --- a/pkg/workflow/safe_outputs_steps_shell_expansion_validation_test.go +++ b/pkg/workflow/safe_outputs_steps_shell_expansion_validation_test.go @@ -159,9 +159,9 @@ func TestValidateSafeOutputsStepsShellExpansion_DangerousPatterns(t *testing.T) } err := validateSafeOutputsStepsShellExpansion(config) require.Error(t, err, "dangerous pattern should be rejected: %s", tt.name) - assert.Contains(t, err.Error(), tt.wantErrContain, + require.ErrorContains(t, err, tt.wantErrContain, "error message should describe the pattern type") - assert.Contains(t, err.Error(), "safe-outputs.steps[0]", + require.ErrorContains(t, err, "safe-outputs.steps[0]", "error message should include the step index") }) } @@ -181,7 +181,7 @@ func TestValidateRunScriptForShellExpansion(t *testing.T) { t.Run("error includes step index", func(t *testing.T) { err := validateRunScriptForShellExpansion(3, "$(echo bad)") require.Error(t, err, "command substitution should be rejected") - assert.Contains(t, err.Error(), "safe-outputs.steps[3]", + require.ErrorContains(t, err, "safe-outputs.steps[3]", "error should include the step index") }) @@ -189,13 +189,13 @@ func TestValidateRunScriptForShellExpansion(t *testing.T) { err := validateRunScriptForShellExpansion(0, `URL=$(cat /tmp/url.txt)`) require.Error(t, err, "should reject command substitution") // The snippet includes at least the $( opener - assert.Contains(t, err.Error(), "$(", "error should include the offending snippet") + require.ErrorContains(t, err, "$(", "error should include the offending snippet") }) t.Run("error includes remediation guidance", func(t *testing.T) { err := validateRunScriptForShellExpansion(0, "$(echo hi)") require.Error(t, err, "should reject command substitution") - assert.Contains(t, err.Error(), "/tmp/gh-aw/agent/", + require.ErrorContains(t, err, "/tmp/gh-aw/agent/", "error should include remediation guidance about writing to a file") }) diff --git a/pkg/workflow/safe_outputs_validation_merge_pull_request_test.go b/pkg/workflow/safe_outputs_validation_merge_pull_request_test.go index 17112d00052..6ec144b5213 100644 --- a/pkg/workflow/safe_outputs_validation_merge_pull_request_test.go +++ b/pkg/workflow/safe_outputs_validation_merge_pull_request_test.go @@ -43,7 +43,7 @@ func TestValidateSafeOutputsMergePullRequestLabelValidation(t *testing.T) { return } require.Error(t, err, "expected merge-pull-request label validation to fail") - assert.Contains(t, err.Error(), tt.wantErr, "expected validation error to include field-specific message") + require.ErrorContains(t, err, tt.wantErr, "expected validation error to include field-specific message") }) } } @@ -92,7 +92,7 @@ func TestValidateSafeOutputsMergePullRequestAllowedBranchesValidation(t *testing } require.Error(t, err, "expected merge-pull-request allowed-branches validation to fail") - assert.Contains(t, err.Error(), tt.wantErr, "expected field-specific allowed-branches error") + require.ErrorContains(t, err, tt.wantErr, "expected field-specific allowed-branches error") }) } } diff --git a/pkg/workflow/safe_update_enforcement_test.go b/pkg/workflow/safe_update_enforcement_test.go index 0c0011bf84e..20d66c7f66f 100644 --- a/pkg/workflow/safe_update_enforcement_test.go +++ b/pkg/workflow/safe_update_enforcement_test.go @@ -367,7 +367,7 @@ func TestEnforceSafeUpdate(t *testing.T) { if tt.wantErr { require.Error(t, err, "expected safe update enforcement error") for _, msg := range tt.wantErrMsgs { - assert.Contains(t, err.Error(), msg, "error message should contain %q", msg) + require.ErrorContains(t, err, msg, "error message should contain %q", msg) } } else { assert.NoError(t, err, "unexpected safe update enforcement error") diff --git a/pkg/workflow/samples_validation_test.go b/pkg/workflow/samples_validation_test.go index 546e09f9d8f..e20c975582b 100644 --- a/pkg/workflow/samples_validation_test.go +++ b/pkg/workflow/samples_validation_test.go @@ -267,9 +267,9 @@ func TestValidateSafeOutputsSamples_NonExpressionErrorsStillReported(t *testing. } err := validateSafeOutputsSamples(cfg) require.Error(t, err, "missing-title error should still surface even though body is a runtime expression") - assert.Contains(t, err.Error(), "create-issue", "error should reference the failing safe-output key") - assert.Contains(t, err.Error(), "samples[0]", "error should reference the failing sample entry") - assert.Contains(t, err.Error(), "title", "error should still be caused by missing required title") + require.ErrorContains(t, err, "create-issue", "error should reference the failing safe-output key") + require.ErrorContains(t, err, "samples[0]", "error should reference the failing sample entry") + require.ErrorContains(t, err, "title", "error should still be caused by missing required title") } // TestSubstituteRuntimeExpressionsForValidation_LeavesLiteralsUntouched @@ -421,7 +421,7 @@ func TestValidateSamplesForTool_DispatchRepositoryDeferred(t *testing.T) { func TestValidateSamplesForTool_UnknownStillFails(t *testing.T) { err := validateSamplesForTool("tool_that_does_not_exist", []map[string]any{{"x": "y"}}) require.Error(t, err, "expected unknown non-dynamic tool to fail schema lookup") - assert.Contains(t, err.Error(), "no MCP tool schema found") + require.ErrorContains(t, err, "no MCP tool schema found") } // TestPlaceholderForSchema covers the schema-driven placeholder lookup for diff --git a/pkg/workflow/sandbox_agent_disabled_test.go b/pkg/workflow/sandbox_agent_disabled_test.go index f7a1a853bd8..2c285d965ac 100644 --- a/pkg/workflow/sandbox_agent_disabled_test.go +++ b/pkg/workflow/sandbox_agent_disabled_test.go @@ -36,7 +36,7 @@ Test workflow with top-level sandbox: false (no longer supported). err = compiler.CompileWorkflow(workflowPath) require.Error(t, err, "Expected error when using sandbox: false (top-level boolean no longer supported)") - assert.Contains(t, err.Error(), "sandbox", "Error should mention sandbox field") + require.ErrorContains(t, err, "sandbox", "Error should mention sandbox field") }) t.Run("sandbox: true is also rejected", func(t *testing.T) { @@ -150,8 +150,8 @@ Test workflow with agent sandbox disabled in strict mode. err = compiler.CompileWorkflow(workflowPath) require.Error(t, err, "Expected error when sandbox.agent: false in strict mode") - assert.Contains(t, err.Error(), "strict mode") - assert.Contains(t, err.Error(), "sandbox.agent: false") + require.ErrorContains(t, err, "strict mode") + require.ErrorContains(t, err, "sandbox.agent: false") }) t.Run("sandbox.agent: false shows warning at compile time", func(t *testing.T) { diff --git a/pkg/workflow/sandbox_test.go b/pkg/workflow/sandbox_test.go index b4efeb013cc..02968d79138 100644 --- a/pkg/workflow/sandbox_test.go +++ b/pkg/workflow/sandbox_test.go @@ -126,7 +126,7 @@ func TestValidateSandboxConfig(t *testing.T) { if tt.expectError { require.Error(t, err) if tt.errorMsg != "" { - assert.Contains(t, err.Error(), tt.errorMsg) + require.ErrorContains(t, err, tt.errorMsg) } } else { assert.NoError(t, err) diff --git a/pkg/workflow/sandbox_validation_test.go b/pkg/workflow/sandbox_validation_test.go index 143d2ea4c8e..8c997d16eb8 100644 --- a/pkg/workflow/sandbox_validation_test.go +++ b/pkg/workflow/sandbox_validation_test.go @@ -98,51 +98,51 @@ func TestGetSandboxDisableJustification(t *testing.T) { t.Run("boolean true is rejected", func(t *testing.T) { _, err := getSandboxDisableJustification(makeData(true)) require.Error(t, err) - assert.Contains(t, err.Error(), "string", "should explain that a string is required") + require.ErrorContains(t, err, "string", "should explain that a string is required") }) t.Run("boolean false is rejected", func(t *testing.T) { _, err := getSandboxDisableJustification(makeData(false)) require.Error(t, err) - assert.Contains(t, err.Error(), "string", "should explain that a string is required") + require.ErrorContains(t, err, "string", "should explain that a string is required") }) t.Run("empty string is rejected", func(t *testing.T) { _, err := getSandboxDisableJustification(makeData("")) require.Error(t, err) - assert.Contains(t, err.Error(), "20", "should mention minimum length") + require.ErrorContains(t, err, "20", "should mention minimum length") }) t.Run("short string is rejected", func(t *testing.T) { _, err := getSandboxDisableJustification(makeData("too short")) require.Error(t, err) - assert.Contains(t, err.Error(), "20", "should mention minimum length") + require.ErrorContains(t, err, "20", "should mention minimum length") }) t.Run("whitespace-padded short string is rejected", func(t *testing.T) { // 22 spaces - long enough on paper but collapses to empty after TrimSpace _, err := getSandboxDisableJustification(makeData(" ")) require.Error(t, err) - assert.Contains(t, err.Error(), "20", "should mention minimum length") + require.ErrorContains(t, err, "20", "should mention minimum length") }) t.Run("whitespace-padded string where trimmed is below minimum is rejected", func(t *testing.T) { // "short" padded with whitespace to 25 total chars still fails (trimmed is 5) _, err := getSandboxDisableJustification(makeData(" short ")) require.Error(t, err) - assert.Contains(t, err.Error(), "20", "should mention minimum length") + require.ErrorContains(t, err, "20", "should mention minimum length") }) t.Run("GitHub Actions expression is rejected", func(t *testing.T) { _, err := getSandboxDisableJustification(makeData("${{ inputs.reason }}")) require.Error(t, err) - assert.Contains(t, err.Error(), "expressions") + require.ErrorContains(t, err, "expressions") }) t.Run("longer expression with surrounding text is rejected", func(t *testing.T) { _, err := getSandboxDisableJustification(makeData("reason: ${{ inputs.reason }} end")) require.Error(t, err) - assert.Contains(t, err.Error(), "expressions") + require.ErrorContains(t, err, "expressions") }) t.Run("20+ character literal reason passes", func(t *testing.T) { @@ -160,7 +160,7 @@ func TestGetSandboxDisableJustification(t *testing.T) { t.Run("feature missing returns error", func(t *testing.T) { _, err := getSandboxDisableJustification(&WorkflowData{Features: map[string]any{}}) require.Error(t, err) - assert.Contains(t, err.Error(), "missing") + require.ErrorContains(t, err, "missing") }) t.Run("nil features returns error", func(t *testing.T) { diff --git a/pkg/workflow/security_architecture_formal_test.go b/pkg/workflow/security_architecture_formal_test.go index 46b55f837d1..6bcbd58f112 100644 --- a/pkg/workflow/security_architecture_formal_test.go +++ b/pkg/workflow/security_architecture_formal_test.go @@ -178,7 +178,7 @@ func TestFormal_P2_AgentHasNoWritePermissions(t *testing.T) { perms.Set(scope, PermissionWrite) err := validateDangerousPermissions(&WorkflowData{Permissions: "permissions: {}"}, perms) require.Error(t, err, "agent job scope %s:write must be rejected", scope) - assert.Contains(t, err.Error(), "write permissions") + require.ErrorContains(t, err, "write permissions") }) } } @@ -199,7 +199,7 @@ func TestFormal_P3_NetworkDomainAllowlist(t *testing.T) { // A wildcard-only allowlist must be rejected in strict mode (CTR-011). err := compiler.validateStrictNetwork(&NetworkPermissions{Allowed: []string{"*"}}) require.Error(t, err, "wildcard-only allowlist must be rejected in strict mode") - assert.Contains(t, err.Error(), "wildcard") + require.ErrorContains(t, err, "wildcard") // An empty network permission set must not cause a validation error. require.NoError(t, compiler.validateNetworkAllowedDomains(nil), @@ -291,7 +291,7 @@ Simulate a write-permission violation to verify that emit is blocked. yamlOut, err := compiler.CompileToYAML(wd, "workflow.md") require.Error(t, err, "CompileToYAML must return an error when a write-permission violation is present (P6 FailSecure)") assert.Empty(t, yamlOut, "CompileToYAML must return empty YAML — the lock-file must not be emitted — when a security violation is detected") - assert.Contains(t, err.Error(), "write permissions", "error must identify the permission violation") + require.ErrorContains(t, err, "write permissions", "error must identify the permission violation") } // TestFormal_P7_ConformanceLevelMonotonicity (P7 Monotonicity) @@ -411,7 +411,7 @@ Simulate a wildcard network violation. compiler2 := NewCompiler(WithNoEmit(true)) _, strictErr := compiler2.ParseWorkflowString(mdNet, "workflow.md") require.Error(t, strictErr, "wildcard-only network allowlist must be rejected before any YAML is generated (P9)") - assert.Contains(t, strictErr.Error(), "wildcard", "error must identify the wildcard violation") + require.ErrorContains(t, strictErr, "wildcard", "error must identify the wildcard violation") } // TestFormal_P10_WriteTokenIsolatedToSafeOutput (P10 TokenIsolation) diff --git a/pkg/workflow/security_architecture_pm10_formal_test.go b/pkg/workflow/security_architecture_pm10_formal_test.go index f5561796609..b0e42a18508 100644 --- a/pkg/workflow/security_architecture_pm10_formal_test.go +++ b/pkg/workflow/security_architecture_pm10_formal_test.go @@ -189,7 +189,7 @@ func TestFormalStrictMode_WritePermissionsRejected(t *testing.T) { err := validateDangerousPermissions(&WorkflowData{Permissions: "permissions: {}"}, perms) require.Error(t, err) - assert.Contains(t, err.Error(), "write permissions") + require.ErrorContains(t, err, "write permissions") }) } } diff --git a/pkg/workflow/security_architecture_sg_formal_test.go b/pkg/workflow/security_architecture_sg_formal_test.go index 3b2d96aac57..3cb819a1d4f 100644 --- a/pkg/workflow/security_architecture_sg_formal_test.go +++ b/pkg/workflow/security_architecture_sg_formal_test.go @@ -107,7 +107,7 @@ func TestFormalSG02_AgentJobHasNoWritePermissions(t *testing.T) { err := validateDangerousPermissions(&WorkflowData{Permissions: formalEmptyPermissionsYAML}, perms) require.Error(t, err, "SG-02: agent job scope %s:write must be rejected by validateDangerousPermissions", scope) - assert.Contains(t, err.Error(), "write permissions", + require.ErrorContains(t, err, "write permissions", "SG-02: error message must identify the write-permission violation") }) } @@ -293,7 +293,7 @@ SG-07: verify that a write-permission violation blocks lock-file emission. "SG-07: CompileToYAML must return an error when a write-permission violation is present") assert.Empty(t, yamlOut, "SG-07: CompileToYAML must return empty YAML — no lock-file may be emitted on security violation") - assert.Contains(t, err.Error(), "write permissions", + require.ErrorContains(t, err, "write permissions", "SG-07: error must identify the write-permission violation") } diff --git a/pkg/workflow/skills_frontmatter_test.go b/pkg/workflow/skills_frontmatter_test.go index cb9cafcf497..f8e3170fbcb 100644 --- a/pkg/workflow/skills_frontmatter_test.go +++ b/pkg/workflow/skills_frontmatter_test.go @@ -26,7 +26,7 @@ func TestValidateFrontmatterSkills(t *testing.T) { }, }) require.Error(t, err) - require.Contains(t, err.Error(), "40-char-sha") + require.ErrorContains(t, err, "40-char-sha") }) t.Run("rejects 39-char sha", func(t *testing.T) { @@ -55,7 +55,7 @@ func TestValidateFrontmatterSkills(t *testing.T) { }, }) require.Error(t, err) - require.Contains(t, err.Error(), "40-char-sha") + require.ErrorContains(t, err, "40-char-sha") }) t.Run("accepts empty skills array", func(t *testing.T) { @@ -94,7 +94,7 @@ func TestValidateFrontmatterSkills(t *testing.T) { }, }) require.Error(t, err) - require.Contains(t, err.Error(), "skills[0].github-token must be a valid GitHub token expression") + require.ErrorContains(t, err, "skills[0].github-token must be a valid GitHub token expression") }) t.Run("accepts object form with github-app", func(t *testing.T) { @@ -121,7 +121,7 @@ func TestValidateFrontmatterSkills(t *testing.T) { }, }) require.Error(t, err) - require.Contains(t, err.Error(), "skills[0].skill") + require.ErrorContains(t, err, "skills[0].skill") }) t.Run("rejects object form github-app without private-key", func(t *testing.T) { @@ -136,7 +136,7 @@ func TestValidateFrontmatterSkills(t *testing.T) { }, }) require.Error(t, err) - require.Contains(t, err.Error(), "skills[0].github-app") + require.ErrorContains(t, err, "skills[0].github-app") }) t.Run("rejects object form with unknown fields", func(t *testing.T) { @@ -150,7 +150,7 @@ func TestValidateFrontmatterSkills(t *testing.T) { }, }) require.Error(t, err) - require.Contains(t, err.Error(), "skills[0].token is not supported") + require.ErrorContains(t, err, "skills[0].token is not supported") }) t.Run("rejects object form that sets both github-token and github-app", func(t *testing.T) { @@ -167,7 +167,7 @@ func TestValidateFrontmatterSkills(t *testing.T) { }, }) require.Error(t, err) - require.Contains(t, err.Error(), "mutually exclusive") + require.ErrorContains(t, err, "mutually exclusive") }) } diff --git a/pkg/workflow/slash_command_centralized_compile_test.go b/pkg/workflow/slash_command_centralized_compile_test.go index 694197dddae..6f185121241 100644 --- a/pkg/workflow/slash_command_centralized_compile_test.go +++ b/pkg/workflow/slash_command_centralized_compile_test.go @@ -112,7 +112,7 @@ tools: compiler := NewCompiler() err := compiler.CompileWorkflow(markdownPath) require.Error(t, err) - require.Contains(t, err.Error(), "on.workflow_dispatch.inputs.topic.required: true is not allowed when using slash_command") + require.ErrorContains(t, err, "on.workflow_dispatch.inputs.topic.required: true is not allowed when using slash_command") lockPath := stringutil.MarkdownToLockFile(markdownPath) _, statErr := os.Stat(lockPath) @@ -146,7 +146,7 @@ tools: compiler := NewCompiler() err := compiler.CompileWorkflow(markdownPath) require.Error(t, err) - require.Contains(t, err.Error(), "on.workflow_dispatch.inputs.topic.required: true is not allowed when using label_command") + require.ErrorContains(t, err, "on.workflow_dispatch.inputs.topic.required: true is not allowed when using label_command") lockPath := stringutil.MarkdownToLockFile(markdownPath) _, statErr := os.Stat(lockPath) diff --git a/pkg/workflow/step_shell_validator_test.go b/pkg/workflow/step_shell_validator_test.go index c125de8dfe8..0ebf2cc06b7 100644 --- a/pkg/workflow/step_shell_validator_test.go +++ b/pkg/workflow/step_shell_validator_test.go @@ -300,7 +300,7 @@ func TestValidateStepShellScripts(t *testing.T) { if tt.expectError { require.Error(t, err) if tt.errorMsg != "" { - assert.Contains(t, err.Error(), tt.errorMsg) + require.ErrorContains(t, err, tt.errorMsg) } } else { assert.NoError(t, err) diff --git a/pkg/workflow/strict_mode_steps_validation_test.go b/pkg/workflow/strict_mode_steps_validation_test.go index 5fe51155be7..41b8e2a94fc 100644 --- a/pkg/workflow/strict_mode_steps_validation_test.go +++ b/pkg/workflow/strict_mode_steps_validation_test.go @@ -414,7 +414,7 @@ func TestValidateStepsSecrets(t *testing.T) { if tt.expectError { require.Error(t, err, "expected an error but got none") - assert.Contains(t, err.Error(), tt.errorMsg, + require.ErrorContains(t, err, tt.errorMsg, "error %q should contain %q", err.Error(), tt.errorMsg) } else { assert.NoError(t, err, "expected no error") diff --git a/pkg/workflow/strict_mode_update_check_validation_test.go b/pkg/workflow/strict_mode_update_check_validation_test.go index 121b9803b4d..05f23009bda 100644 --- a/pkg/workflow/strict_mode_update_check_validation_test.go +++ b/pkg/workflow/strict_mode_update_check_validation_test.go @@ -72,7 +72,7 @@ func TestValidateUpdateCheck(t *testing.T) { if tt.wantErr { require.Error(t, err, "Expected an error but got none") if tt.errContains != "" { - assert.Contains(t, err.Error(), tt.errContains, + require.ErrorContains(t, err, tt.errContains, "Error should contain %q, got: %s", tt.errContains, err.Error()) } } else { diff --git a/pkg/workflow/template_injection_validation_test.go b/pkg/workflow/template_injection_validation_test.go index 2ff0b800ba9..f1ab3edba45 100644 --- a/pkg/workflow/template_injection_validation_test.go +++ b/pkg/workflow/template_injection_validation_test.go @@ -218,13 +218,13 @@ func TestValidateNoTemplateInjection(t *testing.T) { if tt.shouldError { require.Error(t, err, "Expected validation to fail but it passed") if tt.errorString != "" { - assert.Contains(t, err.Error(), tt.errorString, + require.ErrorContains(t, err, tt.errorString, "Error message should contain expected string") } // Verify error message quality - assert.Contains(t, err.Error(), "template injection", + require.ErrorContains(t, err, "template injection", "Error should mention template injection") - assert.Contains(t, err.Error(), "Safe Pattern", + assert.ErrorContains(t, err, "Safe Pattern", "Error should provide safe pattern example") } else { assert.NoError(t, err, "Expected validation to pass but got error: %v", err) @@ -386,9 +386,9 @@ func TestTemplateInjectionRealWorldPatterns(t *testing.T) { err := validateNoTemplateInjection(yaml) require.Error(t, err, "Should detect unsafe gateway-pid usage in run command") - assert.Contains(t, err.Error(), "steps.*.outputs", + require.ErrorContains(t, err, "steps.*.outputs", "Should identify as steps.outputs context") - assert.Contains(t, err.Error(), "gateway-pid", + require.ErrorContains(t, err, "gateway-pid", "Error should mention the specific expression") }) @@ -1006,7 +1006,7 @@ func TestTemplateInjectionYAMLKeyOrdering(t *testing.T) { if tt.shouldError { require.Error(t, err, tt.description) - assert.Contains(t, err.Error(), "template injection", + require.ErrorContains(t, err, "template injection", "Error should mention template injection") } else { assert.NoError(t, err, tt.description) @@ -1139,7 +1139,7 @@ jobs: if tt.shouldError { require.Error(t, err, tt.description) - assert.Contains(t, err.Error(), "template injection", + require.ErrorContains(t, err, "template injection", "Error should mention template injection") } else { assert.NoError(t, err, tt.description) @@ -1331,7 +1331,7 @@ func TestTemplateInjectionYAMLParsingEdgeCases(t *testing.T) { if tt.shouldError { require.Error(t, err, tt.description) - assert.Contains(t, err.Error(), "template injection", + require.ErrorContains(t, err, "template injection", "Error should mention template injection") } else { assert.NoError(t, err, tt.description) diff --git a/pkg/workflow/time_delta_test.go b/pkg/workflow/time_delta_test.go index 5f78288c382..93d2ef749e7 100644 --- a/pkg/workflow/time_delta_test.go +++ b/pkg/workflow/time_delta_test.go @@ -141,7 +141,7 @@ func TestParseTimeDelta(t *testing.T) { if tt.expectError { require.Error(t, err, "parseTimeDelta(%q) should return an error", tt.input) if tt.errorMsg != "" { - assert.Contains(t, err.Error(), tt.errorMsg, "parseTimeDelta(%q) error message mismatch", tt.input) + require.ErrorContains(t, err, tt.errorMsg, "parseTimeDelta(%q) error message mismatch", tt.input) } } else { require.NoError(t, err, "parseTimeDelta(%q) unexpected error", tt.input) @@ -263,7 +263,7 @@ func TestParseTimeDeltaForStopAfter(t *testing.T) { if tt.expectError { require.Error(t, err, "parseTimeDeltaForStopAfter(%q) should return an error", tt.input) if tt.errorMsg != "" { - assert.Contains(t, err.Error(), tt.errorMsg, "parseTimeDeltaForStopAfter(%q) error message mismatch", tt.input) + require.ErrorContains(t, err, tt.errorMsg, "parseTimeDeltaForStopAfter(%q) error message mismatch", tt.input) } } else { require.NoError(t, err, "parseTimeDeltaForStopAfter(%q) unexpected error", tt.input) @@ -666,7 +666,7 @@ func TestResolveStopTime(t *testing.T) { if tt.expectError { require.Error(t, err, "resolveStopTime(%q) should return an error", tt.stopTime) if tt.errorMsg != "" { - assert.Contains(t, err.Error(), tt.errorMsg, "resolveStopTime(%q) error message mismatch", tt.stopTime) + require.ErrorContains(t, err, tt.errorMsg, "resolveStopTime(%q) error message mismatch", tt.stopTime) } } else { require.NoError(t, err, "resolveStopTime(%q) unexpected error", tt.stopTime) @@ -794,7 +794,7 @@ func TestParseRelativeDate(t *testing.T) { if tt.expectError { require.Error(t, err, "parseRelativeDate(%q) should return an error", tt.input) if tt.errorMsg != "" { - assert.Contains(t, err.Error(), tt.errorMsg, "parseRelativeDate(%q) error message mismatch", tt.input) + require.ErrorContains(t, err, tt.errorMsg, "parseRelativeDate(%q) error message mismatch", tt.input) } return } @@ -924,7 +924,7 @@ func TestResolveRelativeDate(t *testing.T) { if tt.expectError { require.Error(t, err, "ResolveRelativeDate(%q) should return an error", tt.input) if tt.errorMsg != "" { - assert.Contains(t, err.Error(), tt.errorMsg, "ResolveRelativeDate(%q) error message mismatch", tt.input) + require.ErrorContains(t, err, tt.errorMsg, "ResolveRelativeDate(%q) error message mismatch", tt.input) } return } diff --git a/pkg/workflow/tools_validation_test.go b/pkg/workflow/tools_validation_test.go index f6e40876ebf..c13662df3fe 100644 --- a/pkg/workflow/tools_validation_test.go +++ b/pkg/workflow/tools_validation_test.go @@ -66,7 +66,7 @@ func TestValidateBashToolConfig(t *testing.T) { if tt.shouldError { require.Error(t, err, "Expected error for %s", tt.name) if tt.errorMsg != "" { - assert.Contains(t, err.Error(), tt.errorMsg, "Error message should contain expected text") + require.ErrorContains(t, err, tt.errorMsg, "Error message should contain expected text") } } else { assert.NoError(t, err, "Expected no error for %s", tt.name) @@ -137,7 +137,7 @@ func TestNewToolsWithInvalidBash(t *testing.T) { // Validation should catch this err := validateBashToolConfig(tools, "test-workflow") require.Error(t, err, "Expected validation error") - assert.Contains(t, err.Error(), "anonymous syntax", "Error should mention anonymous syntax") + require.ErrorContains(t, err, "anonymous syntax", "Error should mention anonymous syntax") }) t.Run("accepts valid bash configurations", func(t *testing.T) { @@ -233,7 +233,7 @@ func TestValidateGitHubToolConfig(t *testing.T) { if tt.shouldError { require.Error(t, err, "Expected error for %s", tt.name) if tt.errorMsg != "" { - assert.Contains(t, err.Error(), tt.errorMsg, "Error message should contain expected text") + require.ErrorContains(t, err, tt.errorMsg, "Error message should contain expected text") } } else { assert.NoError(t, err, "Expected no error for %s", tt.name) @@ -605,7 +605,7 @@ func TestValidateGitHubGuardPolicy(t *testing.T) { if tt.shouldError { require.Error(t, err, "Expected error for %s", tt.name) if tt.errorMsg != "" { - assert.Contains(t, err.Error(), tt.errorMsg, "Error message should contain expected text") + require.ErrorContains(t, err, tt.errorMsg, "Error message should contain expected text") } } else { assert.NoError(t, err, "Expected no error for %s", tt.name) @@ -740,7 +740,7 @@ func TestValidateReposScopeWithStringSlice(t *testing.T) { if tt.shouldError { require.Error(t, err, "Expected error for %s", tt.name) if tt.errorMsg != "" { - assert.Contains(t, err.Error(), tt.errorMsg, "Error message should contain expected text") + require.ErrorContains(t, err, tt.errorMsg, "Error message should contain expected text") } } else { assert.NoError(t, err, "Expected no error for %s", tt.name) @@ -1036,7 +1036,7 @@ func TestValidateIntegrityReactions(t *testing.T) { if tt.shouldError { require.Error(t, err, "Expected error for: %s", tt.name) if tt.errorContains != "" { - assert.Contains(t, err.Error(), tt.errorContains, "Error should mention: %s", tt.errorContains) + require.ErrorContains(t, err, tt.errorContains, "Error should mention: %s", tt.errorContains) } } else { assert.NoError(t, err, "Expected no error for: %s", tt.name) diff --git a/pkg/workflow/yaml_read_test.go b/pkg/workflow/yaml_read_test.go index e9332d5f158..50628c4b481 100644 --- a/pkg/workflow/yaml_read_test.go +++ b/pkg/workflow/yaml_read_test.go @@ -59,6 +59,6 @@ func TestReadWorkflowYAML(t *testing.T) { workflow, err := readWorkflowYAML(workflowPath) assert.Nil(t, workflow, "Invalid YAML should not return workflow data") require.Error(t, err, "Invalid YAML should return an error") - assert.Contains(t, err.Error(), "failed to parse workflow file", "Should wrap parse error consistently") + require.ErrorContains(t, err, "failed to parse workflow file", "Should wrap parse error consistently") }) }