Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions pkg/workflow/runtime_setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,7 @@ func DeduplicateRuntimeSetupStepsFromCustomSteps(customSteps string, runtimeRequ
// Parse custom steps YAML
var stepsWrapper map[string]any
if err := yaml.Unmarshal([]byte(customSteps), &stepsWrapper); err != nil {
return customSteps, runtimeRequirements, fmt.Errorf("failed to parse custom steps: %w", err)
return customSteps, runtimeRequirements, fmt.Errorf("failed to parse custom workflow steps from frontmatter. Custom steps must be valid GitHub Actions step syntax. Example:\nsteps:\n - name: Setup\n run: echo 'hello'\n - name: Build\n run: make build\nError: %w", err)
}

stepsVal, hasSteps := stepsWrapper["steps"]
Expand Down Expand Up @@ -787,7 +787,7 @@ func DeduplicateRuntimeSetupStepsFromCustomSteps(customSteps string, runtimeRequ
stepsWrapper["steps"] = filteredSteps
deduplicatedYAML, err := yaml.Marshal(stepsWrapper)
if err != nil {
return customSteps, runtimeRequirements, fmt.Errorf("failed to marshal deduplicated steps: %w", err)
return customSteps, runtimeRequirements, fmt.Errorf("failed to marshal deduplicated workflow steps to YAML. Step deduplication removes duplicate runtime setup actions (like actions/setup-node) from custom steps to avoid conflicts when automatic runtime detection adds them. This optimization ensures runtime setup steps appear before custom steps. Error: %w", err)
}

return string(deduplicatedYAML), filteredRequirements, nil
Expand Down
114 changes: 114 additions & 0 deletions pkg/workflow/runtime_setup_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package workflow

import (
"fmt"
"strings"
"testing"
)
Expand Down Expand Up @@ -553,3 +554,116 @@ func TestRuntimeFilteringWithExistingSetupActions(t *testing.T) {
t.Error("Expected go to be detected from go build command (deduplication happens in compiler)")
}
}

// TestRuntimeSetupErrorMessages validates that error messages in runtime_setup.go
// provide clear context, explanation, and examples following the error message guidelines
func TestRuntimeSetupErrorMessages(t *testing.T) {
tests := []struct {
name string
testFunc func() error
shouldContain []string
description string
}{
{
name: "parse custom steps error includes example and explanation",
testFunc: func() error {
// Invalid YAML - tab character which is not allowed in YAML
invalidYAML := "steps:\n\t- name: test\n\t run: echo 'hello'"
// Need at least one runtime requirement to avoid early exit
requirements := []RuntimeRequirement{
{Runtime: findRuntimeByID("node"), Version: "20"},
}
_, _, err := DeduplicateRuntimeSetupStepsFromCustomSteps(invalidYAML, requirements)
return err
},
shouldContain: []string{
"failed to parse custom workflow steps",
"Custom steps must be valid GitHub Actions step syntax",
"Example:",
"steps:",
"- name:",
"run:",
},
description: "Error should explain what custom steps are and show valid example",
},
{
name: "marshal deduplicated steps error includes context about deduplication",
testFunc: func() error {
// This test is harder to trigger since Marshal rarely fails
// We test the error message format by checking it would be generated correctly
// by examining the code path
return nil // Skip actual test since Marshal errors are rare
},
shouldContain: []string{},
description: "Skip - Marshal errors are difficult to trigger in tests",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := tt.testFunc()

// Skip tests that are marked to skip
if len(tt.shouldContain) == 0 {
t.Skip(tt.description)
return
}

if err == nil {
t.Fatal("Expected error but got nil")
}

errMsg := err.Error()

// Check that error contains expected content
for _, content := range tt.shouldContain {
if !strings.Contains(errMsg, content) {
t.Errorf("Error message should contain '%s'\nActual error: %s",
content, errMsg)
}
}

// Check that error is descriptive (not too vague)
if len(errMsg) < 50 {
t.Errorf("Error message should be descriptive (>50 chars)\nActual (%d chars): %s",
len(errMsg), errMsg)
}

// Check that error includes the word "Error:" before the wrapped error
if !strings.Contains(errMsg, "Error:") {
t.Errorf("Error message should include 'Error:' before wrapped error\nActual: %s",
errMsg)
}
})
}
}

// TestDeduplicateErrorMessageFormat tests that the deduplicate function would
// produce a helpful error message if Marshal fails
func TestDeduplicateErrorMessageFormat(t *testing.T) {
// We can't easily trigger a Marshal error, but we can verify the error format
// by checking what the error message would look like
expectedPhrases := []string{
"failed to marshal deduplicated workflow steps",
"Step deduplication removes duplicate runtime setup actions",
"to avoid conflicts",
"Error:",
}

// Create a sample error message as it would appear
sampleError := fmt.Errorf("failed to marshal deduplicated workflow steps to YAML. Step deduplication removes duplicate runtime setup actions (like actions/setup-node) from custom steps to avoid conflicts when automatic runtime detection adds them. This optimization ensures runtime setup steps appear before custom steps. Error: %w", fmt.Errorf("yaml marshal error"))

errMsg := sampleError.Error()

for _, phrase := range expectedPhrases {
if !strings.Contains(errMsg, phrase) {
t.Errorf("Error message should contain '%s'\nActual: %s", phrase, errMsg)
}
}

// Verify the message is descriptive
if len(errMsg) < 100 {
t.Errorf("Error message should be comprehensive (>100 chars)\nActual (%d chars): %s",
len(errMsg), errMsg)
}
}
Loading