Skip to content

[test-improver] Improve tests for strutil package#7011

Merged
lpcox merged 1 commit into
mainfrom
test-improver/strutil-copy-trimmed-map-6b6059110e38ecd8
Jun 5, 2026
Merged

[test-improver] Improve tests for strutil package#7011
lpcox merged 1 commit into
mainfrom
test-improver/strutil-copy-trimmed-map-6b6059110e38ecd8

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot commented Jun 5, 2026

Test Improvements: util_test.go

File Analyzed

  • Test File: internal/strutil/util_test.go
  • Package: internal/strutil
  • Lines of Code: 35 → 123 (+88 lines)

Improvements Made

1. Increased Coverage

  • ✅ Added TestCopyTrimmedStringIntMap — table-driven test with 8 cases for CopyTrimmedStringIntMap, which had 0% coverage within its own package
  • ✅ Added TestCopyTrimmedStringIntMap_DefensiveCopy — verifies that mutating the returned copy does not affect the original map
  • ✅ Added TestCopyTrimmedStringIntMap_OriginalMutationDoesNotAffectCopy — verifies that mutating the original after copying does not affect the returned copy
  • Previous Coverage: 92.0% (CopyTrimmedStringIntMap: 0.0%)
  • New Coverage: 100.0% (CopyTrimmedStringIntMap: 100.0%)
  • Improvement: +8.0%

2. Better Testing Patterns

  • ✅ Table-driven test covering all branches: nil input, empty map, single entry, multiple entries, whitespace trimming (spaces and tabs), zero/negative values
  • ✅ Isolated defensive-copy tests that clearly state their invariant in the test name
  • ✅ Uses require.NotNil before asserting on the copy (fail-fast for nil)
  • ✅ Parallel subtests consistent with the existing TestGetStringFromMap style

3. Cleaner & More Stable Tests

  • ✅ Each subcase is t.Parallel() for faster test execution
  • ✅ Descriptive names follow the project's existing snake_case subtest convention
  • ✅ No timing dependencies or external state

Test Execution

All tests pass with 100% coverage:

ok  github.com/github/gh-aw-mcpg/internal/strutil  0.009s  coverage: 100.0% of statements
github.com/github/gh-aw-mcpg/internal/strutil/copy_trimmed_string_int_map.go:7:  CopyTrimmedStringIntMap  100.0%
github.com/github/gh-aw-mcpg/internal/strutil/util.go:5:                          GetStringFromMap         100.0%
total:                                                                              (statements)             100.0%

Why These Changes?

CopyTrimmedStringIntMap is a utility in the strutil package used by server/guard_init.go to copy tool-call limit maps. The function was tested indirectly via internal/server/copy_tool_call_limits_test.go (which runs in package server), meaning its own package had 0% coverage for this function. Adding direct unit tests in util_test.go ensures the function is tested at the package level, surfaces any future regressions immediately in strutil package test runs, and brings the package to 100% coverage.


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 · sonnet46 2.3M ·

Add comprehensive tests for CopyTrimmedStringIntMap in util_test.go.
The function had 0% coverage within its own package (it was only tested
indirectly from server/copy_tool_call_limits_test.go).

New tests cover:
- nil input returns nil
- empty map returns nil
- single and multiple entries copied correctly
- leading/trailing space trimming from keys
- tab character trimming from keys
- zero and negative limit values preserved
- defensive copy: mutations to copy don't affect original
- defensive copy: mutations to original don't affect copy

Package coverage: 92.0% → 100.0%

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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

Adds direct unit test coverage for CopyTrimmedStringIntMap in internal/strutil, bringing that helper’s coverage into the package’s own test suite (rather than relying on indirect coverage from other packages).

Changes:

  • Added a table-driven test covering CopyTrimmedStringIntMap behavior across nil/empty inputs, key trimming, and value edge cases.
  • Added two focused tests to assert the returned map is a defensive copy (mutations don’t leak across original/copy boundaries).
  • Updated imports to include require for fail-fast assertions in the defensive-copy tests.
Show a summary per file
File Description
internal/strutil/util_test.go Adds comprehensive unit tests for CopyTrimmedStringIntMap, including defensive-copy invariants.

Copilot's findings

Tip

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

  • Files reviewed: 1/1 changed files
  • Comments generated: 0

@lpcox lpcox merged commit 81493a4 into main Jun 5, 2026
23 checks passed
@lpcox lpcox deleted the test-improver/strutil-copy-trimmed-map-6b6059110e38ecd8 branch June 5, 2026 14:38
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.

2 participants