Skip to content

perf(workflow): optimize YAML parsing with custom UnmarshalYAML #86

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
watany-dev wants to merge 1 commit into
base: main
Choose a base branch
from
Open

perf(workflow): optimize YAML parsing with custom UnmarshalYAML #86

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
72 changes: 8 additions & 64 deletions internal/workflow/load.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,77 +8,21 @@ import (
)

// LoadWorkflowFile loads and parses a GitHub Actions workflow file from the given path.
// The parsing is optimized to extract both the workflow structure and job order
// in a single pass through the YAML node tree.
func LoadWorkflowFile(path string) (*WorkflowFile, error) {
data, err := os.ReadFile(path)
if err != nil {
return nil, fmt.Errorf("failed to read workflow file: %w", err)
}

// Parse YAML into yaml.Node once (instead of parsing twice)
var root yaml.Node
if err := yaml.Unmarshal(data, &root); err != nil {
return nil, fmt.Errorf("failed to parse workflow YAML: %w", err)
}

// Extract both WorkflowFile and JobOrder from the single parsed Node
wf, jobOrder, err := parseWorkflowFromNode(&root)
if err != nil {
return nil, fmt.Errorf("failed to parse workflow: %w", err)
}
wf.JobOrder = jobOrder

return wf, nil
}

// parseWorkflowFromNode extracts WorkflowFile and job order from a parsed yaml.Node.
// This avoids parsing the YAML twice by reusing the same Node for both operations.
func parseWorkflowFromNode(root *yaml.Node) (*WorkflowFile, []string, error) {
// Parse YAML and decode in a single operation.
// WorkflowFile.UnmarshalYAML handles both struct decoding and JobOrder extraction
// in a single node traversal, avoiding the overhead of double traversal.
var wf WorkflowFile
// Decode the Node directly into the struct (no re-parsing needed)
if err := root.Decode(&wf); err != nil {
return nil, nil, fmt.Errorf("failed to decode workflow: %w", err)
}

// Extract job order from the same Node
jobOrder := extractJobOrderFromNode(root)

return &wf, jobOrder, nil
}

// extractJobOrderFromNode extracts job IDs in definition order from a parsed yaml.Node.
func extractJobOrderFromNode(root *yaml.Node) []string {
if root == nil || len(root.Content) == 0 {
return nil
}

// root.Content[0] is the document node
doc := root.Content[0]
if doc == nil || doc.Kind != yaml.MappingNode {
return nil
}

// Find the "jobs" key in the mapping
for i := 0; i < len(doc.Content)-1; i += 2 {
keyNode := doc.Content[i]
valueNode := doc.Content[i+1]

if keyNode == nil || valueNode == nil {
continue
}

if keyNode.Value == "jobs" && valueNode.Kind == yaml.MappingNode {
// Extract job IDs in order
var jobOrder []string
for j := 0; j < len(valueNode.Content)-1; j += 2 {
jobKeyNode := valueNode.Content[j]
if jobKeyNode == nil {
continue
}
jobOrder = append(jobOrder, jobKeyNode.Value)
}
return jobOrder
}
if err := yaml.Unmarshal(data, &wf); err != nil {
return nil, fmt.Errorf("failed to parse workflow YAML: %w", err)
}

return nil
return &wf, nil
}
206 changes: 78 additions & 128 deletions internal/workflow/load_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -775,14 +775,13 @@ func TestLoadWorkflowFile_YAMLUnmarshalError(t *testing.T) {
}
}

// TestExtractJobOrderFromNode_SequenceRoot tests extractJobOrderFromNode with sequence at root
// This specifically covers the doc.Kind != yaml.MappingNode branch (line 56-58)
func TestExtractJobOrderFromNode_SequenceRoot(t *testing.T) {
// TestUnmarshalYAML_SequenceRoot tests UnmarshalYAML with sequence at root
// This specifically covers the node.Kind != yaml.MappingNode branch
func TestUnmarshalYAML_SequenceRoot(t *testing.T) {
t.Parallel()
tmpDir := t.TempDir()

// Create YAML with sequence at root - this will parse but decode will fail
// However, we need to test the extractJobOrderFromNode function path
yamlContent := `---
- item1
- item2`
Expand All @@ -792,12 +791,9 @@ func TestExtractJobOrderFromNode_SequenceRoot(t *testing.T) {
}

_, err := LoadWorkflowFile(workflowPath)
// This should error or return empty JobOrder
if err != nil {
// Error is expected since root is not a mapping
if !strings.Contains(err.Error(), "decode workflow") {
t.Logf("Got error: %v", err)
}
// This should error since root is not a mapping
if err == nil {
t.Error("LoadWorkflowFile() expected error for sequence root")
}
}

Expand Down Expand Up @@ -955,147 +951,101 @@ func TestDiscoverWorkflows_SymlinkToDirectory(t *testing.T) {
})
}

// TestExtractJobOrderFromNode_DirectCall tests extractJobOrderFromNode directly
// to cover the doc.Kind != yaml.MappingNode branch
func TestExtractJobOrderFromNode_DirectCall(t *testing.T) {
// TestUnmarshalYAML_DirectCall tests WorkflowFile.UnmarshalYAML with various node types
func TestUnmarshalYAML_DirectCall(t *testing.T) {
t.Parallel()

// Test with sequence node (not mapping)
t.Run("sequence node returns nil", func(t *testing.T) {
var root yaml.Node
t.Run("sequence node returns error", func(t *testing.T) {
yamlContent := `- item1
- item2`
if err := yaml.Unmarshal([]byte(yamlContent), &root); err != nil {
t.Fatalf("failed to unmarshal YAML: %v", err)
}

result := extractJobOrderFromNode(&root)
if result != nil {
t.Errorf("extractJobOrderFromNode() = %v, want nil for sequence node", result)
var wf WorkflowFile
err := yaml.Unmarshal([]byte(yamlContent), &wf)
if err == nil {
t.Error("UnmarshalYAML should return error for sequence node")
}
})

// Test with scalar node
t.Run("scalar node returns nil", func(t *testing.T) {
var root yaml.Node
t.Run("scalar node returns error", func(t *testing.T) {
yamlContent := `just a string`
if err := yaml.Unmarshal([]byte(yamlContent), &root); err != nil {
t.Fatalf("failed to unmarshal YAML: %v", err)
}

result := extractJobOrderFromNode(&root)
if result != nil {
t.Errorf("extractJobOrderFromNode() = %v, want nil for scalar node", result)
var wf WorkflowFile
err := yaml.Unmarshal([]byte(yamlContent), &wf)
if err == nil {
t.Error("UnmarshalYAML should return error for scalar node")
}
})

// Test with empty node
t.Run("empty node returns nil", func(t *testing.T) {
root := &yaml.Node{}
result := extractJobOrderFromNode(root)
if result != nil {
t.Errorf("extractJobOrderFromNode() = %v, want nil for empty node", result)
// Test with empty document
t.Run("empty document returns empty workflow", func(t *testing.T) {
yamlContent := ``
var wf WorkflowFile
err := yaml.Unmarshal([]byte(yamlContent), &wf)
if err != nil {
t.Errorf("UnmarshalYAML error = %v for empty document", err)
}
})

// Test with nil key node in doc.Content (line 65)
t.Run("nil key node in content is skipped", func(t *testing.T) {
root := &yaml.Node{
Kind: yaml.DocumentNode,
Content: []*yaml.Node{
{
Kind: yaml.MappingNode,
Content: []*yaml.Node{
nil, // nil key node
{Kind: yaml.ScalarNode, Value: "value"},
{Kind: yaml.ScalarNode, Value: "jobs"},
{
Kind: yaml.MappingNode,
Content: []*yaml.Node{
{Kind: yaml.ScalarNode, Value: "job1"},
{Kind: yaml.MappingNode},
},
},
},
},
},
}
result := extractJobOrderFromNode(root)
// Should skip the nil key and find "jobs"
if len(result) != 1 || result[0] != "job1" {
t.Errorf("extractJobOrderFromNode() = %v, want [job1]", result)
if wf.Name != "" || len(wf.Jobs) != 0 || len(wf.JobOrder) != 0 {
t.Errorf("UnmarshalYAML should return empty workflow for empty document")
}
})

// Test with nil value node in doc.Content (line 65)
t.Run("nil value node in content is skipped", func(t *testing.T) {
root := &yaml.Node{
Kind: yaml.DocumentNode,
Content: []*yaml.Node{
{
Kind: yaml.MappingNode,
Content: []*yaml.Node{
{Kind: yaml.ScalarNode, Value: "name"},
nil, // nil value node
{Kind: yaml.ScalarNode, Value: "jobs"},
{
Kind: yaml.MappingNode,
Content: []*yaml.Node{
{Kind: yaml.ScalarNode, Value: "job1"},
{Kind: yaml.MappingNode},
},
},
},
},
},
}
result := extractJobOrderFromNode(root)
// Should skip the nil value and find "jobs"
if len(result) != 1 || result[0] != "job1" {
t.Errorf("extractJobOrderFromNode() = %v, want [job1]", result)
// Test valid workflow with jobs
t.Run("valid workflow extracts job order correctly", func(t *testing.T) {
yamlContent := `name: Test
jobs:
first:
runs-on: ubuntu-latest
steps:
- run: echo first
second:
runs-on: ubuntu-latest
steps:
- run: echo second`
var wf WorkflowFile
err := yaml.Unmarshal([]byte(yamlContent), &wf)
if err != nil {
t.Fatalf("UnmarshalYAML error = %v", err)
}
if wf.Name != "Test" {
t.Errorf("Name = %q, want %q", wf.Name, "Test")
}
if len(wf.JobOrder) != 2 || wf.JobOrder[0] != "first" || wf.JobOrder[1] != "second" {
t.Errorf("JobOrder = %v, want [first, second]", wf.JobOrder)
}
})

// Test with nil job key node in jobs.Content (line 74)
t.Run("nil job key node is skipped", func(t *testing.T) {
root := &yaml.Node{
Kind: yaml.DocumentNode,
Content: []*yaml.Node{
{
Kind: yaml.MappingNode,
Content: []*yaml.Node{
{Kind: yaml.ScalarNode, Value: "jobs"},
{
Kind: yaml.MappingNode,
Content: []*yaml.Node{
nil, // nil job key node
{Kind: yaml.MappingNode},
{Kind: yaml.ScalarNode, Value: "job2"},
{Kind: yaml.MappingNode},
},
},
},
},
},
}
result := extractJobOrderFromNode(root)
// Should skip the nil job key and find "job2"
if len(result) != 1 || result[0] != "job2" {
t.Errorf("extractJobOrderFromNode() = %v, want [job2]", result)
// Test workflow with env
t.Run("workflow with env extracts correctly", func(t *testing.T) {
yamlContent := `name: Test
env:
KEY1: value1
KEY2: value2`
var wf WorkflowFile
err := yaml.Unmarshal([]byte(yamlContent), &wf)
if err != nil {
t.Fatalf("UnmarshalYAML error = %v", err)
}
if wf.Env["KEY1"] != "value1" || wf.Env["KEY2"] != "value2" {
t.Errorf("Env = %v, want {KEY1:value1, KEY2:value2}", wf.Env)
}
})

// Test with nil doc node (line 56)
t.Run("nil doc node returns nil", func(t *testing.T) {
root := &yaml.Node{
Kind: yaml.DocumentNode,
Content: []*yaml.Node{
nil, // nil doc node
},
}
result := extractJobOrderFromNode(root)
if result != nil {
t.Errorf("extractJobOrderFromNode() = %v, want nil for nil doc node", result)
// Test document node wrapper handling
t.Run("document node wrapper is handled", func(t *testing.T) {
yamlContent := `---
name: DocTest
jobs:
test:
runs-on: ubuntu-latest
steps:
- run: echo test`
var wf WorkflowFile
err := yaml.Unmarshal([]byte(yamlContent), &wf)
if err != nil {
t.Fatalf("UnmarshalYAML error = %v", err)
}
if wf.Name != "DocTest" {
t.Errorf("Name = %q, want %q", wf.Name, "DocTest")
}
})
}
Expand Down
Loading