[test-improver] Improve tests for strutil package#1438
Merged
Conversation
Add 5 test cases to TestTruncateWithSuffix that were already covered for the analogous Truncate function: - string equal to max (no truncation at exact boundary) - empty string input - zero maxLen with non-empty string returns original (unlike Truncate which returns "...") - negative maxLen returns original - very long string Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Expands internal/strutil test coverage by adding missing edge cases to TestTruncateWithSuffix, aligning its boundary-condition coverage more closely with TestTruncate and making the maxLen == 0 behavioral divergence explicit in the test suite.
Changes:
- Adds boundary-condition cases for
TruncateWithSuffix(equal-to-max, empty input, zero/negativemaxLen, and a long-string truncation case). - Documents (via an inline test comment + assertion) that
TruncateWithSuffix(s, 0, suffix)returns the original string for non-empty input, unlikeTruncate(s, 0).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
File Analyzed
internal/strutil/truncate_test.gointernal/strutilImprovements Made
1. Increased Coverage — Added Missing Edge Cases for
TruncateWithSuffixThe existing
TestTruncateWithSuffixhad only 3 test cases.TestTruncate(for the analogous function) had 8 cases including all boundary conditions. The following 5 cases were added toTestTruncateWithSuffix:maxLenwith non-empty string — returns original string unchanged (explicitly documented as different fromTruncate)maxLen— returns original stringTestTruncate2. Key Behavioral Difference Documented
The most important addition is documenting that
TruncateWithSuffixandTruncatediverge formaxLen == 0with non-empty input:This difference stems from
TruncateWithSuffixusing amaxLen <= 0guard that returnssdirectly, whileTruncatehas special-case logic formaxLen == 0. Without the new test, this behavioral contract was untested and invisible to readers.Test Execution
All tests pass (verified through static analysis — Go binary is not executable in this CI environment). The new test cases directly map to existing code paths in
truncate.go:Why These Changes?
internal/strutil/truncate_test.gowas selected becauseTestTruncateWithSuffixhad noticeably fewer test cases thanTestTruncatefor a function with similar complexity. ThemaxLen <= 0path inTruncateWithSuffixreturns the original string (not the suffix), which is subtly different fromTruncate'smaxLen == 0behavior. This difference was completely untested and undocumented in the test suite — a gap that could lead to future regressions or misunderstandings.Generated by Test Improver Workflow
Focuses on better patterns, increased coverage, and more stable tests