Skip to content

[test-improver] Improve tests for mcpresult package#7345

Merged
lpcox merged 1 commit into
mainfrom
test-improve-mcpresult-text-content-124f4428dee60b20
Jun 11, 2026
Merged

[test-improver] Improve tests for mcpresult package#7345
lpcox merged 1 commit into
mainfrom
test-improve-mcpresult-text-content-124f4428dee60b20

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

File Analyzed

  • Test File: internal/mcpresult/text_content_test.go
  • Package: internal/mcpresult

Improvements Made

Better Testing Patterns

  • ✅ Added testify/assert import to text_content_test.go
  • ✅ Replaced manual if got != tt.want { t.Errorf(...) } with assert.Equal(t, tt.want, got) in the table-driven loop
  • ✅ Replaced manual if got != "" { t.Errorf(...) } with assert.Empty(t, got) in TestExtractTextContent_NilMap
  • ✅ Consistent with sibling content_items_test.go in the same package (which already used testify)

Why These Changes?

text_content_test.go was the only file in the mcpresult package not using testify, despite the package having it available (v1.11.1 in go.mod) and the sibling file content_items_test.go already using assert/require. The manual if/t.Errorf checks produce less readable failure output than testify's diff-aware assertions.

The two specific conversions:

  1. assert.Equal(t, tt.want, got) — produces a clear diff on failure with expected/actual values highlighted
  2. assert.Empty(t, got) — more semantically precise than comparing to "", and produces better failure messages

Test Execution

All tests pass with 100% coverage maintained:

ok  github.com/github/gh-aw-mcpg/internal/mcpresult  0.005s  coverage: 100.0% of statements

Generated by Test Improver Workflow
Focuses on better patterns, increased coverage, and more stable tests

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • index.crates.io

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "index.crates.io"

See Network Configuration for more information.

Generated by Test Improver · 687.8 AIC · ⊞ 29.3K ·

Replace manual if/t.Errorf checks with testify assert.Equal and
assert.Empty, consistent with the sibling content_items_test.go in
the same package.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@lpcox

lpcox commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

@copilot review this pr and apply any recommended changes

Copilot AI commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

@copilot review this pr and apply any recommended changes

Reviewed the changes. The two modifications in text_content_test.go are correct and consistent with the rest of the package:

  1. assert.Equal(t, tt.want, got) — replaces the manual if got != tt.want { t.Errorf(...) } pattern
  2. assert.Empty(t, got) — replaces if got != "" { t.Errorf(...) } in TestExtractTextContent_NilMap

All unit tests pass with 100% coverage. No further changes needed.

Copilot finished work on behalf of lpcox June 11, 2026 18:43
Copilot AI requested a review from lpcox June 11, 2026 18:43
@lpcox lpcox merged commit 7f7190a into main Jun 11, 2026
24 of 25 checks passed
@lpcox lpcox deleted the test-improve-mcpresult-text-content-124f4428dee60b20 branch June 11, 2026 18:43
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.

3 participants