diff --git a/pkg/workflow/expression_coverage_test.go b/pkg/workflow/expression_coverage_test.go index 3f5ab4ba113..5067f55102c 100644 --- a/pkg/workflow/expression_coverage_test.go +++ b/pkg/workflow/expression_coverage_test.go @@ -4,15 +4,16 @@ package workflow import ( "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // TestBuildFromAllowedForksEmptyList tests BuildFromAllowedForks with empty list func TestBuildFromAllowedForksEmptyList(t *testing.T) { result := BuildFromAllowedForks([]string{}) expected := "github.event.pull_request.head.repo.id == github.repository_id" - if result.Render() != expected { - t.Errorf("BuildFromAllowedForks([]) = %v, expected %v", result.Render(), expected) - } + assert.Equal(t, expected, result.Render(), "BuildFromAllowedForks([]) rendered output mismatch") } // TestBuildFromAllowedForksSingleCondition tests BuildFromAllowedForks with single pattern @@ -22,9 +23,8 @@ func TestBuildFromAllowedForksSingleCondition(t *testing.T) { result := BuildFromAllowedForks([]string{}) rendered := result.Render() // Should not have DisjunctionNode wrapping for single condition - if rendered != "github.event.pull_request.head.repo.id == github.repository_id" { - t.Errorf("BuildFromAllowedForks with empty list should return single condition without OR") - } + assert.Equal(t, "github.event.pull_request.head.repo.id == github.repository_id", rendered, + "BuildFromAllowedForks with empty list should return single condition without OR") } // TestBuildFromAllowedForksGlobPattern tests BuildFromAllowedForks with glob pattern @@ -32,15 +32,12 @@ func TestBuildFromAllowedForksGlobPattern(t *testing.T) { result := BuildFromAllowedForks([]string{"myorg/*"}) rendered := result.Render() // Should include both the default condition AND the glob pattern with OR - if !containsSubstring(rendered, "github.event.pull_request.head.repo.id == github.repository_id") { - t.Errorf("BuildFromAllowedForks should include default condition") - } - if !containsSubstring(rendered, "startsWith(github.event.pull_request.head.repo.full_name, 'myorg/')") { - t.Errorf("BuildFromAllowedForks should include glob pattern condition") - } - if !containsSubstring(rendered, "||") { - t.Errorf("BuildFromAllowedForks with multiple conditions should use OR") - } + assert.Contains(t, rendered, "github.event.pull_request.head.repo.id == github.repository_id", + "BuildFromAllowedForks should include default condition") + assert.Contains(t, rendered, "startsWith(github.event.pull_request.head.repo.full_name, 'myorg/')", + "BuildFromAllowedForks should include glob pattern condition") + assert.Contains(t, rendered, "||", + "BuildFromAllowedForks with multiple conditions should use OR") } // TestBuildFromAllowedForksExactMatch tests BuildFromAllowedForks with exact match @@ -48,15 +45,12 @@ func TestBuildFromAllowedForksExactMatch(t *testing.T) { result := BuildFromAllowedForks([]string{"myorg/myrepo"}) rendered := result.Render() // Should include both the default condition AND the exact match with OR - if !containsSubstring(rendered, "github.event.pull_request.head.repo.id == github.repository_id") { - t.Errorf("BuildFromAllowedForks should include default condition") - } - if !containsSubstring(rendered, "github.event.pull_request.head.repo.full_name == 'myorg/myrepo'") { - t.Errorf("BuildFromAllowedForks should include exact match condition") - } - if !containsSubstring(rendered, "||") { - t.Errorf("BuildFromAllowedForks with multiple conditions should use OR") - } + assert.Contains(t, rendered, "github.event.pull_request.head.repo.id == github.repository_id", + "BuildFromAllowedForks should include default condition") + assert.Contains(t, rendered, "github.event.pull_request.head.repo.full_name == 'myorg/myrepo'", + "BuildFromAllowedForks should include exact match condition") + assert.Contains(t, rendered, "||", + "BuildFromAllowedForks with multiple conditions should use OR") } // TestBuildFromAllowedForksMixedPatterns tests BuildFromAllowedForks with mixed patterns @@ -65,9 +59,8 @@ func TestBuildFromAllowedForksMixedPatterns(t *testing.T) { rendered := result.Render() // Should include default condition - if !containsSubstring(rendered, "github.event.pull_request.head.repo.id == github.repository_id") { - t.Errorf("BuildFromAllowedForks should include default condition") - } + assert.Contains(t, rendered, "github.event.pull_request.head.repo.id == github.repository_id", + "BuildFromAllowedForks should include default condition") // Should include all patterns expectedPatterns := []string{ @@ -78,15 +71,11 @@ func TestBuildFromAllowedForksMixedPatterns(t *testing.T) { } for _, pattern := range expectedPatterns { - if !containsSubstring(rendered, pattern) { - t.Errorf("BuildFromAllowedForks should include pattern: %s", pattern) - } + assert.Contains(t, rendered, pattern, "BuildFromAllowedForks should include pattern") } // Should use DisjunctionNode with OR operators - if !containsSubstring(rendered, "||") { - t.Errorf("BuildFromAllowedForks with multiple conditions should use OR") - } + assert.Contains(t, rendered, "||", "BuildFromAllowedForks with multiple conditions should use OR") } // TestVisitExpressionTreeWithDifferentNodeTypes tests VisitExpressionTree with various node types @@ -153,13 +142,8 @@ func TestVisitExpressionTreeWithDifferentNodeTypes(t *testing.T) { return nil }) - if err != nil { - t.Errorf("VisitExpressionTree() unexpected error: %v", err) - } - - if count != tt.expectedCount { - t.Errorf("VisitExpressionTree() visited %d nodes, expected %d: %s", count, tt.expectedCount, tt.description) - } + require.NoError(t, err, "VisitExpressionTree() unexpected error: %s", tt.description) + assert.Equal(t, tt.expectedCount, count, "VisitExpressionTree() visited node count mismatch: %s", tt.description) }) } } @@ -172,12 +156,8 @@ func TestExpressionParserCurrentWithEmptyTokens(t *testing.T) { } result := parser.current() - if result.kind != tokenEOF { - t.Errorf("current() with empty tokens should return EOF token, got %v", result.kind) - } - if result.pos != -1 { - t.Errorf("current() with empty tokens should return pos -1, got %d", result.pos) - } + assert.Equal(t, tokenEOF, result.kind, "current() with empty tokens should return EOF token") + assert.Equal(t, -1, result.pos, "current() with empty tokens should return pos -1") } // TestExpressionParserCurrentBeyondLength tests the current() method when pos >= len(tokens) @@ -190,9 +170,7 @@ func TestExpressionParserCurrentBeyondLength(t *testing.T) { } result := parser.current() - if result.kind != tokenEOF { - t.Errorf("current() with pos beyond length should return EOF token, got %v", result.kind) - } + assert.Equal(t, tokenEOF, result.kind, "current() with pos beyond length should return EOF token") } // TestParseExpressionEmptyString tests ParseExpression with empty string @@ -214,12 +192,9 @@ func TestParseExpressionEmptyString(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { _, err := ParseExpression(tt.input) - if err == nil { - t.Error("ParseExpression() with empty/whitespace string should return error") - } - if err.Error() != "empty expression" { - t.Errorf("ParseExpression() error = %v, expected 'empty expression'", err) - } + require.Error(t, err, "ParseExpression() with empty/whitespace string should return error") + assert.ErrorContains(t, err, "empty expression", + "ParseExpression(%q) unexpected error message", tt.input) }) } } diff --git a/pkg/workflow/time_delta_test.go b/pkg/workflow/time_delta_test.go index a408f7640d0..5f78288c382 100644 --- a/pkg/workflow/time_delta_test.go +++ b/pkg/workflow/time_delta_test.go @@ -5,6 +5,9 @@ package workflow import ( "testing" "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestParseTimeDelta(t *testing.T) { @@ -136,26 +139,14 @@ func TestParseTimeDelta(t *testing.T) { result, err := parseTimeDelta(tt.input) if tt.expectError { - if err == nil { - t.Errorf("parseTimeDelta(%q) expected error but got none", tt.input) - return - } - if tt.errorMsg != "" && !containsString(err.Error(), tt.errorMsg) { - t.Errorf("parseTimeDelta(%q) error = %v, want to contain %v", tt.input, err.Error(), tt.errorMsg) + 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) } } else { - if err != nil { - t.Errorf("parseTimeDelta(%q) unexpected error: %v", tt.input, err) - return - } - if result == nil { - t.Errorf("parseTimeDelta(%q) returned nil result", tt.input) - return - } - if result.Days != tt.expected.Days || result.Hours != tt.expected.Hours || result.Minutes != tt.expected.Minutes { - t.Errorf("parseTimeDelta(%q) = {Days: %d, Hours: %d, Minutes: %d}, want {Days: %d, Hours: %d, Minutes: %d}", - tt.input, result.Days, result.Hours, result.Minutes, tt.expected.Days, tt.expected.Hours, tt.expected.Minutes) - } + require.NoError(t, err, "parseTimeDelta(%q) unexpected error", tt.input) + require.NotNil(t, result, "parseTimeDelta(%q) returned nil result", tt.input) + assert.Equal(t, tt.expected, result, "parseTimeDelta(%q) result mismatch", tt.input) } }) } @@ -270,29 +261,14 @@ func TestParseTimeDeltaForStopAfter(t *testing.T) { result, err := parseTimeDeltaForStopAfter(tt.input) if tt.expectError { - if err == nil { - t.Errorf("parseTimeDeltaForStopAfter(%q) expected error but got none", tt.input) - return - } - if tt.errorMsg != "" && !containsString(err.Error(), tt.errorMsg) { - t.Errorf("parseTimeDeltaForStopAfter(%q) error = %v, want to contain %v", tt.input, err.Error(), tt.errorMsg) + 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) } } else { - if err != nil { - t.Errorf("parseTimeDeltaForStopAfter(%q) unexpected error: %v", tt.input, err) - return - } - if result == nil { - t.Errorf("parseTimeDeltaForStopAfter(%q) returned nil result", tt.input) - return - } - if result.Days != tt.expected.Days || result.Hours != tt.expected.Hours || - result.Minutes != tt.expected.Minutes || result.Weeks != tt.expected.Weeks || - result.Months != tt.expected.Months { - t.Errorf("parseTimeDeltaForStopAfter(%q) = {Months: %d, Weeks: %d, Days: %d, Hours: %d, Minutes: %d}, want {Months: %d, Weeks: %d, Days: %d, Hours: %d, Minutes: %d}", - tt.input, result.Months, result.Weeks, result.Days, result.Hours, result.Minutes, - tt.expected.Months, tt.expected.Weeks, tt.expected.Days, tt.expected.Hours, tt.expected.Minutes) - } + require.NoError(t, err, "parseTimeDeltaForStopAfter(%q) unexpected error", tt.input) + require.NotNil(t, result, "parseTimeDeltaForStopAfter(%q) returned nil result", tt.input) + assert.Equal(t, tt.expected, result, "parseTimeDeltaForStopAfter(%q) result mismatch", tt.input) } }) } @@ -339,9 +315,7 @@ func TestTimeDeltaString(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { result := tt.delta.String() - if result != tt.expected { - t.Errorf("TimeDelta.String() = %v, want %v", result, tt.expected) - } + assert.Equal(t, tt.expected, result, "TimeDelta.String() mismatch for delta %+v", tt.delta) }) } } @@ -382,9 +356,7 @@ func TestIsRelativeStopTime(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { result := isRelativeStopTime(tt.input) - if result != tt.expected { - t.Errorf("isRelativeStopTime(%q) = %v, want %v", tt.input, result, tt.expected) - } + assert.Equal(t, tt.expected, result, "isRelativeStopTime(%q) mismatch", tt.input) }) } } @@ -552,33 +524,19 @@ func TestParseAbsoluteDateTime(t *testing.T) { result, err := parseAbsoluteDateTime(tt.input) if tt.expectedErr { - if err == nil { - t.Errorf("parseAbsoluteDateTime(%q) expected error but got none", tt.input) - } + require.Error(t, err, "parseAbsoluteDateTime(%q) should return an error", tt.input) return } - if err != nil { - t.Errorf("parseAbsoluteDateTime(%q) unexpected error: %v", tt.input, err) - return - } + require.NoError(t, err, "parseAbsoluteDateTime(%q) unexpected error", tt.input) // Parse the result to verify it's correct parsed, err := time.Parse("2006-01-02 15:04:05", result) - if err != nil { - t.Errorf("parseAbsoluteDateTime(%q) result %q is not a valid timestamp: %v", tt.input, result, err) - return - } + require.NoError(t, err, "parseAbsoluteDateTime(%q) result %q is not a valid timestamp", tt.input, result) - if parsed.Day() != tt.expectedDay { - t.Errorf("parseAbsoluteDateTime(%q) day = %d, want %d", tt.input, parsed.Day(), tt.expectedDay) - } - if parsed.Month() != tt.expectedMonth { - t.Errorf("parseAbsoluteDateTime(%q) month = %v, want %v", tt.input, parsed.Month(), tt.expectedMonth) - } - if parsed.Year() != tt.expectedYear { - t.Errorf("parseAbsoluteDateTime(%q) year = %d, want %d", tt.input, parsed.Year(), tt.expectedYear) - } + assert.Equal(t, tt.expectedDay, parsed.Day(), "parseAbsoluteDateTime(%q) day mismatch", tt.input) + assert.Equal(t, tt.expectedMonth, parsed.Month(), "parseAbsoluteDateTime(%q) month mismatch", tt.input) + assert.Equal(t, tt.expectedYear, parsed.Year(), "parseAbsoluteDateTime(%q) year mismatch", tt.input) }) } } @@ -706,40 +664,18 @@ func TestResolveStopTime(t *testing.T) { result, err := resolveStopTime(tt.stopTime, tt.compileTime) if tt.expectError { - if err == nil { - t.Errorf("resolveStopTime(%q, %v) expected error but got none", tt.stopTime, tt.compileTime) - return - } - if tt.errorMsg != "" && !containsString(err.Error(), tt.errorMsg) { - t.Errorf("resolveStopTime(%q, %v) error = %v, want to contain %v", tt.stopTime, tt.compileTime, err.Error(), tt.errorMsg) + 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) } } else { - if err != nil { - t.Errorf("resolveStopTime(%q, %v) unexpected error: %v", tt.stopTime, tt.compileTime, err) - return - } - if result != tt.expected { - t.Errorf("resolveStopTime(%q, %v) = %v, want %v", tt.stopTime, tt.compileTime, result, tt.expected) - } + require.NoError(t, err, "resolveStopTime(%q) unexpected error", tt.stopTime) + assert.Equal(t, tt.expected, result, "resolveStopTime(%q) result mismatch", tt.stopTime) } }) } } -// Helper function to check if a string contains a substring -func containsString(s, substr string) bool { - return len(s) >= len(substr) && (s == substr || containsSubstring(s, substr)) -} - -func containsSubstring(s, substr string) bool { - for i := 0; i <= len(s)-len(substr); i++ { - if s[i:i+len(substr)] == substr { - return true - } - } - return false -} - func TestIsRelativeDate(t *testing.T) { tests := []struct { name string @@ -781,9 +717,7 @@ func TestIsRelativeDate(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { result := isRelativeDate(tt.input) - if result != tt.expected { - t.Errorf("isRelativeDate(%q) = %v, want %v", tt.input, result, tt.expected) - } + assert.Equal(t, tt.expected, result, "isRelativeDate(%q) mismatch", tt.input) }) } } @@ -858,40 +792,23 @@ func TestParseRelativeDate(t *testing.T) { delta, isNeg, err := parseRelativeDate(tt.input) if tt.expectError { - if err == nil { - t.Errorf("parseRelativeDate(%q) expected error but got none", tt.input) - return - } - if tt.errorMsg != "" && !containsString(err.Error(), tt.errorMsg) { - t.Errorf("parseRelativeDate(%q) error = %v, want to contain %v", tt.input, err.Error(), tt.errorMsg) + 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) } return } - if err != nil { - t.Errorf("parseRelativeDate(%q) unexpected error: %v", tt.input, err) - return - } + require.NoError(t, err, "parseRelativeDate(%q) unexpected error", tt.input) if !tt.expectDelta { - if delta != nil { - t.Errorf("parseRelativeDate(%q) expected no delta but got %v", tt.input, delta) - } - return - } - - if delta == nil { - t.Errorf("parseRelativeDate(%q) expected delta but got nil", tt.input) + assert.Nil(t, delta, "parseRelativeDate(%q) expected no delta but got one", tt.input) return } - if isNeg != tt.expectNeg { - t.Errorf("parseRelativeDate(%q) isNegative = %v, want %v", tt.input, isNeg, tt.expectNeg) - } - - if *delta != *tt.expectedDelta { - t.Errorf("parseRelativeDate(%q) delta = %v, want %v", tt.input, delta, tt.expectedDelta) - } + require.NotNil(t, delta, "parseRelativeDate(%q) expected delta but got nil", tt.input) + assert.Equal(t, tt.expectNeg, isNeg, "parseRelativeDate(%q) isNegative mismatch", tt.input) + assert.Equal(t, tt.expectedDelta, delta, "parseRelativeDate(%q) delta mismatch", tt.input) }) } } @@ -1005,24 +922,15 @@ func TestResolveRelativeDate(t *testing.T) { result, err := ResolveRelativeDate(tt.input, tt.baseTime) if tt.expectError { - if err == nil { - t.Errorf("ResolveRelativeDate(%q) expected error but got none", tt.input) - return - } - if tt.errorMsg != "" && !containsString(err.Error(), tt.errorMsg) { - t.Errorf("ResolveRelativeDate(%q) error = %v, want to contain %v", tt.input, err.Error(), tt.errorMsg) + 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) } return } - if err != nil { - t.Errorf("ResolveRelativeDate(%q) unexpected error: %v", tt.input, err) - return - } - - if result != tt.expected { - t.Errorf("ResolveRelativeDate(%q) = %q, want %q", tt.input, result, tt.expected) - } + require.NoError(t, err, "ResolveRelativeDate(%q) unexpected error", tt.input) + assert.Equal(t, tt.expected, result, "ResolveRelativeDate(%q) result mismatch", tt.input) }) } } @@ -1178,9 +1086,7 @@ func TestParseRelativeTimeSpec(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { result := parseRelativeTimeSpec(tt.input) - if result != tt.expected { - t.Errorf("parseRelativeTimeSpec(%q) = %d, expected %d", tt.input, result, tt.expected) - } + assert.Equal(t, tt.expected, result, "parseRelativeTimeSpec(%q) mismatch", tt.input) }) } } @@ -1276,9 +1182,7 @@ func TestParseExpiresFromConfig(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { result := parseExpiresFromConfig(tt.config) - if result != tt.expected { - t.Errorf("parseExpiresFromConfig(%v) = %d, expected %d", tt.config, result, tt.expected) - } + assert.Equal(t, tt.expected, result, "parseExpiresFromConfig(%v) mismatch", tt.config) }) } }