Skip to content

Remove zombie skipped test functions from pkg/cli/ test files#19312

Merged
pelikhan merged 2 commits intomainfrom
copilot/remove-zombie-skipped-tests
Mar 3, 2026
Merged

Remove zombie skipped test functions from pkg/cli/ test files#19312
pelikhan merged 2 commits intomainfrom
copilot/remove-zombie-skipped-tests

Conversation

Copy link
Contributor

Copilot AI commented Mar 3, 2026

7 test functions across 4 pkg/cli/ files were permanently skipped with the require() migration message, providing no test coverage while adding noise.

Changes

  • audit_test.go — removed TestAuditParseFlagBehavior
  • logs_firewall_parse_test.go — removed TestParseFirewallLogs, TestParseFirewallLogsInWorkflowLogsSubdir, TestParseFirewallLogsEmptyDirectory
  • logs_parse_test.go — removed TestParseAgentLog, TestParseAgentLogWithAgentOutputDir
  • mcp_inspect_safe_inputs_integration_test.go — deleted (contained only the skipped test)

All active test functions are untouched.

Original prompt

This section details on the original issue you should resolve

<issue_title>[plan] Remove zombie skipped test functions from pkg/cli/ test files</issue_title>
<issue_description>## Objective

Remove permanently-skipped test functions from four pkg/cli/ test files. Unlike the pkg/workflow/ zombie files where entire files can be deleted, these files contain a mix of active and skipped functions — so only the skipped functions should be removed, not the entire files.

Context

Discussion #19223 identified 7 permanently-skipped test functions across 4 files in pkg/cli/. All skips use the require() migration message.

Files to Edit

File Skipped Functions to Remove
pkg/cli/audit_test.go 1
pkg/cli/logs_firewall_parse_test.go 3
pkg/cli/logs_parse_test.go 2
pkg/cli/mcp_inspect_safe_inputs_integration_test.go 1

Approach

For each file:

  1. Open the file and identify all test functions that contain t.Skip(...) with the message "scripts now use require() pattern" (or similar require() migration messages)
  2. Remove those test functions entirely (including any helper functions that exist solely to support the skipped tests)
  3. Do NOT remove functions that are either:
    • Not skipped
    • Skipped for a different reason (e.g., // FIXME skips)

Search pattern to find skipped functions:

grep -n "t.Skip" pkg/cli/audit_test.go pkg/cli/logs_firewall_parse_test.go pkg/cli/logs_parse_test.go pkg/cli/mcp_inspect_safe_inputs_integration_test.go

Validation

go build ./...
go vet ./...
make fmt
make lint
make test-unit

Acceptance Criteria

  • All require()-migration-skipped test functions removed from the 4 files
  • No non-skipped test functions accidentally removed
  • Files still compile and pass go vet
  • make test-unit passes with no regressions
  • make lint passes (no unused helper functions left behind)

Generated by Plan Command for issue #discussion #19223

  • expires on Mar 4, 2026, 2:57 PM UTC

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Remove zombie skipped test functions from pkg/cli test files Remove zombie skipped test functions from pkg/cli/ test files Mar 3, 2026
@pelikhan pelikhan marked this pull request as ready for review March 3, 2026 02:26
Copilot AI review requested due to automatic review settings March 3, 2026 02:26
@pelikhan pelikhan merged commit 58b4760 into main Mar 3, 2026
@pelikhan pelikhan deleted the copilot/remove-zombie-skipped-tests branch March 3, 2026 02:27
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

This PR removes 7 permanently-skipped test functions (all marked as skipped with the require() migration message) from 4 files in pkg/cli/. One file that contained only the skipped test is fully deleted; the other three files have only the skipped functions removed, leaving all active test functions untouched.

Changes:

  • Removed 7 permanently-skipped (no-op) test functions across 4 test files
  • Deleted mcp_inspect_safe_inputs_integration_test.go entirely (it contained only the one skipped test)
  • Cleaned up zombie tests in audit_test.go, logs_firewall_parse_test.go, and logs_parse_test.go

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
pkg/cli/mcp_inspect_safe_inputs_integration_test.go Entire file deleted; contained only TestSafeInputsMCPServerCompilation which was permanently skipped
pkg/cli/audit_test.go Removed TestAuditParseFlagBehavior, a permanently-skipped no-op test
pkg/cli/logs_firewall_parse_test.go Removed TestParseFirewallLogs, TestParseFirewallLogsInWorkflowLogsSubdir, and TestParseFirewallLogsEmptyDirectory — all permanently skipped
pkg/cli/logs_parse_test.go Removed TestParseAgentLog and TestParseAgentLogWithAgentOutputDir — both permanently skipped

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

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.

[plan] Remove zombie skipped test functions from pkg/cli/ test files

3 participants