Skip to content

Strengthen frontmatter YAML helper tests with table-driven coverage and injection guards#36031

Merged
pelikhan merged 4 commits into
mainfrom
copilot/testify-expert-improve-test-quality-another-one
May 31, 2026
Merged

Strengthen frontmatter YAML helper tests with table-driven coverage and injection guards#36031
pelikhan merged 4 commits into
mainfrom
copilot/testify-expert-improve-test-quality-another-one

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 31, 2026

pkg/workflow/frontmatter_extraction_yaml_test.go previously covered only one helper with a single t.Fatal check. This update expands coverage across core pure helpers in frontmatter_extraction_yaml.go, standardizes on testify assertions with messages, and adds explicit regression checks for expression-injection-shaped inputs.

  • isGitHubAppNestedField: full table-driven coverage

    • Replaced t.Fatal path with assert.Equal/assert-style checks.
    • Added cases for all supported named fields (app-id, client-id, private-key, ignore-if-missing, owner, repositories), repository list items (- ...), and non-matching/near-miss keys.
  • Workflow run conclusion validation hardening

    • Added TestIsValidWorkflowRunConclusion covering all allowed conclusions.
    • Added negative cases for unknown values and injection-attempt strings.
  • indentYAMLLines behavior contract

    • Added table-driven tests for empty input, single-line input, multi-line indentation, and blank-line preservation semantics.
  • Condition-extraction helper coverage

    • Added TestExtractDeploymentStatusStateCondition for missing config, single/multi-state inputs, and mixed-type arrays (string filtering behavior).
    • Added TestExtractWorkflowRunConclusionConditionHelper for missing config, valid single/multi conclusions, and invalid/injection conclusion rejection via error path.
tests := []struct {
    name       string
    conclusion string
    want       bool
}{
    {name: "success", conclusion: "success", want: true},
    {name: "timed_out", conclusion: "timed_out", want: true},
    {name: "expression injection attempt", conclusion: "success' || contains(github.event.comment.body, 'x') || '", want: false},
}
for _, tt := range tests {
    t.Run(tt.name, func(t *testing.T) {
        assert.Equal(t, tt.want, isValidWorkflowRunConclusion(tt.conclusion),
            "isValidWorkflowRunConclusion(%q) should return %t", tt.conclusion, tt.want)
    })
}

Copilot AI and others added 2 commits May 31, 2026 01:31
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Copilot AI changed the title [WIP] Improve test quality in frontmatter extraction tests Strengthen frontmatter YAML helper tests with table-driven coverage and injection guards May 31, 2026
Copilot AI requested a review from gh-aw-bot May 31, 2026 01:32
@pelikhan pelikhan marked this pull request as ready for review May 31, 2026 01:41
Copilot AI review requested due to automatic review settings May 31, 2026 01:41
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Expands the previously thin frontmatter_extraction_yaml_test.go with table-driven tests for several pure helpers in frontmatter_extraction_yaml.go, standardizes on testify assertions, and adds regression cases for expression-injection-shaped inputs (notably for workflow_run.conclusion).

Changes:

  • Adds table-driven coverage for isGitHubAppNestedField, isValidWorkflowRunConclusion, and indentYAMLLines.
  • Adds TestExtractDeploymentStatusStateCondition and TestExtractWorkflowRunConclusionConditionHelper, including injection-attempt rejection.
  • Switches to testify assert style with descriptive messages, replacing the prior single t.Fatal check.
Show a summary per file
File Description
pkg/workflow/frontmatter_extraction_yaml_test.go Greatly expanded tests for frontmatter YAML helpers; contains a stray ) that breaks compilation.
.github/workflows/daily-model-inventory.lock.yml Regenerated lock metadata (body_hash bump only); no functional change.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 2/2 changed files
  • Comments generated: 1

}
assert.Equal(t, tt.want, got, "extractWorkflowRunConclusionCondition should return expected expression for %q", tt.name)
})
})
@pelikhan
Copy link
Copy Markdown
Collaborator

@copilot review all comments

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 31, 2026

@copilot review all comments

Addressed in b8c6983. I reviewed the actionable feedback, fixed the malformed loop closures in pkg/workflow/frontmatter_extraction_yaml_test.go, and verified with go test ./pkg/workflow -count=1.

Copilot AI requested a review from pelikhan May 31, 2026 01:59
@pelikhan pelikhan merged commit f5be76a into main May 31, 2026
@pelikhan pelikhan deleted the copilot/testify-expert-improve-test-quality-another-one branch May 31, 2026 02:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[testify-expert] Improve Test Quality: pkg/workflow/frontmatter_extraction_yaml_test.go

4 participants