Skip to content

Comments

Fix testifylint useless-assert error in TestGetWorkflowStatuses#16263

Closed
Copilot wants to merge 3 commits intosimp2from
copilot/fix-tests-cf6fb216-f3b2-4c77-a3a9-1185cc02a186
Closed

Fix testifylint useless-assert error in TestGetWorkflowStatuses#16263
Copilot wants to merge 3 commits intosimp2from
copilot/fix-tests-cf6fb216-f3b2-4c77-a3a9-1185cc02a186

Conversation

Copy link
Contributor

Copilot AI commented Feb 17, 2026

Linting failure: testifylint flagged a meaningless assertion checking if len(statuses) >= 0, which is always true for slices.

Changes

  • Removed the useless assertion from pkg/cli/add_interactive_workflow_test.go
  • Changed statuses, err := getWorkflowStatuses(...) to _, err := ... since the test only verifies the function doesn't panic

Before

statuses, err := getWorkflowStatuses(tt.pattern, tt.repoOverride, tt.verbose)
if err != nil {
    assert.Error(t, err, "Expected error without gh CLI")
}
if err == nil && statuses != nil {
    assert.GreaterOrEqual(t, len(statuses), 0, "Statuses should be a valid slice")
}

After

_, err := getWorkflowStatuses(tt.pattern, tt.repoOverride, tt.verbose)
if err != nil {
    assert.Error(t, err, "Expected error without gh CLI")
}

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://api.github.com/repos/github/gh-aw/actions/workflows
    • Triggering command: /usr/bin/gh gh workflow list --json name,state,path js/**/*.json' --ignore-path ../../../.prettierignore (http block)
  • https://api.github.com/repos/owner/repo/actions/workflows
    • Triggering command: /usr/bin/gh gh workflow list --json name,state,path --repo owner/repo de/node/bin/git (http block)
    • Triggering command: /usr/bin/gh gh workflow list --json name,state,path --repo owner/repo (http block)

If you need me to access, download, or install something from one of these locations, you can either:

Original prompt

Reference: https://github.com/github/gh-aw/actions/runs/22081416439/job/63807209660#step:7:1

Fix tests


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix tests in the codebase Fix testifylint useless-assert error in TestGetWorkflowStatuses Feb 17, 2026
Copilot AI requested a review from pelikhan February 17, 2026 00:32
@pelikhan pelikhan marked this pull request as ready for review February 17, 2026 00:33
Copilot AI review requested due to automatic review settings February 17, 2026 00:33
Copy link
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 a testifylint lint failure in the CLI tests by removing a meaningless slice-length assertion in TestGetWorkflowStatuses, and includes a small formatting-only update to a Vitest expectation string.

Changes:

  • Remove the always-true len(statuses) >= 0 assertion from TestGetWorkflowStatuses.
  • Replace statuses, err := ... with _, err := ... since the test isn’t validating returned data.
  • Reformat a long toHaveBeenCalledWith(...) string literal in a JS unit test into a single line.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
pkg/cli/add_interactive_workflow_test.go Removes the useless assertion and stops capturing an unused return value (but the updated test logic still contains redundant assertions).
actions/setup/js/log_parser_bootstrap.test.cjs Formatting-only change to the expected failure message string.
Comments suppressed due to low confidence (1)

pkg/cli/add_interactive_workflow_test.go:50

  • assert.Error(t, err, ...) is guarded by if err != nil, so the assertion can never fail and doesn’t add coverage (it may also trigger testifylint’s “useless assert” style checks). Remove this block or replace it with an assertion that can actually fail (e.g., require.NotPanics around the call if that’s the goal).
			if err != nil {
				// Error is acceptable in test environment without gh CLI setup
				assert.Error(t, err, "Expected error without gh CLI")
			}

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

Comment on lines +44 to 47
_, err := getWorkflowStatuses(tt.pattern, tt.repoOverride, tt.verbose)

// Either succeeds or fails gracefully, but shouldn't panic
// Note: statuses may be nil even when err is nil (no workflows found)
if err != nil {
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

This test currently doesn’t actually validate anything: on the success path there are no assertions, and on the error path the if err != nil { assert.Error(t, err, ...) } check is redundant (and may still be flagged by testifylint as a useless assert). If the intent is purely “doesn’t panic”, wrap the call in require.NotPanics and drop the conditional assert entirely (optionally just ignore err or add a deterministic assertion via mocking).

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

Copilot uses AI. Check for mistakes.
@pelikhan pelikhan closed this Feb 17, 2026
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.

2 participants