Skip to content

fix: testifylint expected/actual order and invalid steps syntax in auto-triage-issues workflow#26290

Merged
pelikhan merged 1 commit intomainfrom
copilot/fix-build-lint-go
Apr 14, 2026
Merged

fix: testifylint expected/actual order and invalid steps syntax in auto-triage-issues workflow#26290
pelikhan merged 1 commit intomainfrom
copilot/fix-build-lint-go

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 14, 2026

Two CI failures: lint-go due to reversed assert.Equal arguments, and build due to invalid frontmatter syntax in auto-triage-issues.md that produced non-schema-compliant YAML.

pkg/stringutil/spec_test.go — testifylint

All 4 PATType constant assertions had (actual, expected) order. Fixed to (expected, actual):

// Before
assert.Equal(t, PATType("fine-grained"), PATTypeFineGrained, "...")

// After
assert.Equal(t, PATTypeFineGrained, PATType("fine-grained"), "...")

.github/workflows/auto-triage-issues.md — invalid steps syntax

The steps frontmatter used a pre-agent: subkey (map) instead of a flat step list. The compiler passed the map through addCustomStepsAsIs, which emitted pre-agent: as an inline property of the preceding step — failing GitHub Actions schema validation.

# Before (invalid — map syntax)
steps:
  pre-agent:
    - name: Fetch unlabeled issues

# After (valid — flat list)
steps:
  - name: Fetch unlabeled issues

…ix steps syntax in auto-triage-issues.md

Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d42b4d7e-97b3-40a4-acb3-90eca6f7d205

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
@pelikhan pelikhan marked this pull request as ready for review April 14, 2026 21:15
Copilot AI review requested due to automatic review settings April 14, 2026 21:15
@pelikhan pelikhan merged commit bc2e4ca into main Apr 14, 2026
19 checks passed
@pelikhan pelikhan deleted the copilot/fix-build-lint-go branch April 14, 2026 21:16
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

Fixes two CI failures by correcting assertion argument ordering for testifylint and repairing invalid steps frontmatter syntax so the generated GitHub Actions workflow YAML is schema-compliant.

Changes:

  • Reordered assert.Equal arguments in PATType constant spec tests to satisfy testifylint.
  • Converted steps frontmatter in auto-triage-issues.md from an invalid map form to a valid flat list.
  • Updated the compiled workflow lockfile to reflect the corrected frontmatter.
Show a summary per file
File Description
pkg/stringutil/spec_test.go Adjusts assert.Equal argument order in PATType constant spec assertions to address testifylint.
.github/workflows/auto-triage-issues.md Fixes frontmatter steps structure to a valid YAML list so compilation produces schema-valid workflow YAML.
.github/workflows/auto-triage-issues.lock.yml Regenerates compiled workflow output to match the updated markdown frontmatter.

Copilot's findings

Tip

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

Comments suppressed due to low confidence (1)

pkg/stringutil/spec_test.go:400

  • Same expected/actual readability point for these assertions as well (documented value as expected; constant value as actual) to keep failure diffs easy to interpret.
	assert.Equal(t, PATTypeOAuth, PATType("oauth"),
		"PATTypeOAuth should have documented value 'oauth'")
	assert.Equal(t, PATTypeUnknown, PATType("unknown"),
		"PATTypeUnknown should have documented value 'unknown'")
  • Files reviewed: 3/3 changed files
  • Comments generated: 1

Comment on lines +393 to 396
assert.Equal(t, PATTypeFineGrained, PATType("fine-grained"),
"PATTypeFineGrained should have documented value 'fine-grained'")
assert.Equal(t, PATType("classic"), PATTypeClassic,
assert.Equal(t, PATTypeClassic, PATType("classic"),
"PATTypeClassic should have documented value 'classic'")
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

These assertions now treat the constant under test as “expected” and the documented literal as “actual”, which makes assertion failure output less intuitive given the message (“…should have documented value…”). If testifylint was complaining about the type-conversion expression, an alternative is to keep the documented value as expected (string literal) and compare it to the constant’s string value, so failures read naturally.

This issue also appears on line 397 of the same file.

See below for a potential fix:

	assert.Equal(t, "fine-grained", string(PATTypeFineGrained),
		"PATTypeFineGrained should have documented value 'fine-grained'")
	assert.Equal(t, "classic", string(PATTypeClassic),
		"PATTypeClassic should have documented value 'classic'")
	assert.Equal(t, "oauth", string(PATTypeOAuth),
		"PATTypeOAuth should have documented value 'oauth'")
	assert.Equal(t, "unknown", string(PATTypeUnknown),

Copilot uses AI. Check for mistakes.
@github-actions github-actions bot mentioned this pull request Apr 14, 2026
@github-actions
Copy link
Copy Markdown
Contributor

🧪 Test Quality Sentinel Report

Test Quality Score: 85/100

Excellent test quality

Metric Value
New/modified tests analyzed 1
✅ Design tests (behavioral contracts) 1 (100%)
⚠️ Implementation tests (low value) 0 (0%)
Tests with error/edge cases 0 (0%) — N/A for constant verification
Duplicate test clusters 0
Test inflation detected No
🚨 Coding-guideline violations None

Test Classification Details

Test File Classification Issues Detected
TestSpec_Constants_PATType pkg/stringutil/spec_test.go:392 ✅ Design None — correct expected, actual order after fix

What Changed

The only test change in this PR is a testifylint correction in TestSpec_Constants_PATType: the four assert.Equal calls had their expected and actual arguments swapped. The fix brings them into compliance with testify's convention (assert.Equal(t, expected, actual, msg)), which matters for readable failure output.

Before (incorrect order):

assert.Equal(t, PATType("fine-grained"), PATTypeFineGrained, "...")
//              ^^^ actual              ^^^ expected  — wrong order

After (correct order):

assert.Equal(t, PATTypeFineGrained, PATType("fine-grained"), "...")
//              ^^^ expected        ^^^ actual               — correct

All 4 assertions have descriptive message strings ✅. Build tag //go:build !integration is present on line 1 ✅. No mock libraries used ✅.

Why 85 and not 100? The edge case / error path component scores 0 because TestSpec_Constants_PATType only verifies constant values (no error branches to test). This is appropriate for a constant-contract test and is not a deficiency — the score cap reflects the rubric arithmetic, not a real quality gap.


Language Support

Tests analyzed:

  • 🐹 Go (*_test.go): 1 test — unit (//go:build !integration)
  • 🟨 JavaScript (*.test.cjs, *.test.js): 0 tests

Verdict

Check passed. 0% of new tests are implementation tests (threshold: 30%). The change is a correct, guideline-compliant testifylint fix with no behavioral regressions.


📖 Understanding Test Classifications

Design Tests (High Value) verify what the system does:

  • Assert on observable outputs, return values, or state changes
  • Cover error paths and boundary conditions
  • Would catch a behavioral regression if deleted
  • Remain valid even after internal refactoring

Implementation Tests (Low Value) verify how the system does it:

  • Assert on internal function calls (mocking internals)
  • Only test the happy path with typical inputs
  • Break during legitimate refactoring even when behavior is correct
  • Give false assurance: they pass even when the system is wrong

Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.

🧪 Test quality analysis by Test Quality Sentinel · ● 703.5K ·

Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

✅ Test Quality Sentinel: 85/100. Test quality is acceptable — 0% of new tests are implementation tests (threshold: 30%). The change correctly fixes testifylint expected/actual argument order in TestSpec_Constants_PATType with no guideline violations.

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.

3 participants