Skip to content

Strengthen fix --dir test coverage and migrate fix_dir_flag_test to testify - #53783

Closed
pelikhan with Copilot wants to merge 4 commits into
mainfrom
copilot/testify-expert-improve-test-quality-again
Closed

Strengthen fix --dir test coverage and migrate fix_dir_flag_test to testify#53783
pelikhan with Copilot wants to merge 4 commits into
mainfrom
copilot/testify-expert-improve-test-quality-again

Conversation

Copilot AI commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

pkg/cli/fix_dir_flag_test.go covered core --dir happy paths but missed helper-level unit tests and edge cases, and still used legacy raw assertion patterns. This updates the file to match package testing conventions (assert/require) while expanding coverage for dir resolution and codemod helper behavior.

  • Assertion/style upgrade in fix_dir_flag_test.go

    • Replaced raw t.Fatalf / t.Error checks with require.* for setup/execution gates and assert.* for content checks.
    • Added a focused file-level note clarifying scope (--dir resolution/discovery vs broader codemod behavior tests elsewhere).
  • Reduced duplication with shared helpers + table-driven flow

    • Consolidated repeated setup and verification via helpers (writeFixWorkflow, chdirTemp, assertMigratedTimeoutField).
    • Unified structurally similar dir scenarios into a table-driven TestFixWithDirFlag.
  • Added missing --dir edge-case coverage

    • Nonexistent workflow directory now has explicit test coverage (error expected).
    • Directory containing no markdown workflows now has explicit no-op coverage (no error expected).
    • Added relative and absolute WorkflowDir behavior checks.
  • Added dedicated unit tests for untested helpers in fix_command.go

    • TestResolveWorkflowRoot (including nested .github/workflows, fallback-to-dir behavior, and cleaned/messy paths).
    • TestWasCodemodApplied.
    • TestWasAnyCodemodApplied.
func TestResolveWorkflowRoot(t *testing.T) {
	testCases := []struct {
		name     string
		filePath string
		expected string
	}{
		{
			name:     "nested under github workflows",
			filePath: filepath.Join("repo", ".github", "workflows", "nested", "foo.md"),
			expected: filepath.Join("repo", ".github", "workflows"),
		},
		{
			name:     "no github workflows segment falls back to dir",
			filePath: filepath.Join("repo", "workflows", "foo.md"),
			expected: filepath.Join("repo", "workflows"),
		},
	}
	for _, tc := range testCases {
		t.Run(tc.name, func(t *testing.T) {
			assert.Equal(t, tc.expected, resolveWorkflowRoot(tc.filePath))
		})
	}
}

Copilot AI and others added 2 commits August 18, 2026 18:28
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Improve test quality in fix_dir_flag_test.go Strengthen fix --dir test coverage and migrate fix_dir_flag_test to testify Aug 18, 2026
Copilot AI requested a review from pelikhan August 18, 2026 18:35
@pelikhan
pelikhan marked this pull request as ready for review August 18, 2026 19:26
Copilot AI balanced review requested due to automatic review settings August 18, 2026 19:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Expands fix --dir test coverage and adopts Testify assertions.

Changes:

  • Adds table-driven directory and edge-case tests.
  • Adds unit tests for codemod and workflow-root helpers.
  • Extracts shared test helpers.
Show a summary per file
File Description
pkg/cli/fix_dir_flag_test.go Refactors and expands fix-command tests.

Review details

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Suppressed comments (1)

pkg/cli/fix_dir_flag_test.go:214

  • This second normalization case also uses filepath.Join, which cleans the path before resolveWorkflowRoot receives it. As written, it does not exercise the fallback branch’s normalization behavior.
			filePath: filepath.Join(".", "a", "..", "b", "foo.md"),
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread pkg/cli/fix_dir_flag_test.go Outdated
},
{
name: "messy path under github workflows is normalized",
filePath: filepath.Join(".", "a", "..", "b", ".github", "workflows", "nested", "foo.md"),

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 8038cf0: both messy-path cases now build inputs with a joinUnclean helper that keeps the . and .. segments intact, so removing filepath.Clean from resolveWorkflowRoot fails the test.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Correction: the fix landed in commit 6ce853e, not 8038cf0.

@pelikhan

Copy link
Copy Markdown
Collaborator

@copilot run pr-finisher

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

PR Triage

  • Category: test · Risk: low
  • Score: 33/100 (impact 10, urgency 8, quality 15)
  • Recommended action: auto_merge

Test-only change (assertion style migration + coverage). CI is mostly green (1 cancelled run, likely flaky/rerun-worthy). Safe to auto-merge once CI is fully green.

Generated by 🔧 PR Triage Agent · auto · 58.5 AIC · ⌖ 2.15 AIC · ⊞ 8.3K ·

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[testify-expert] Improve Test Quality: pkg/cli/fix_dir_flag_test.go

3 participants