Add comprehensive test coverage for errext/errorset.go

[Copilot]

• Analyze current test coverage for errorset.go (was 0%)
• Create comprehensive tests for ErrorSet methods
  • Test Add() method with nil and non-nil errors
  • Test Addf() method with formatted error messages
  • Test Append() method to merge ErrorSets
  • Test IsEmpty() method
  • Test Join() method with various separators
  • Test JoinedError() method and its Error() implementation
  • Test Unwrap() method indirectly through errors.Is()
  • Test LogFatalIfError() method (empty ErrorSet case)
• Run tests to verify coverage reaches 88% (exceeds 70% requirement)
• Verify no changes to original code
• Fix formatting issues
• Correct assert.Equal parameter order to (t, actual, expected) per review
• Refactor tests to avoid testing implementation details (Unwrap interface)
• Rename helper function to clarify it's not a test function

Summary

Successfully increased test coverage for errext/errorset.go from 0% to 88%, well exceeding the requested 70% minimum.

Coverage Results:

• Add(): 100%
• Addf(): 100%
• Append(): 100%
• IsEmpty(): 100%
• Join(): 100%
• JoinedError(): 100%
• Error() (joinedError): 100%
• Unwrap() (joinedError): 100% (tested indirectly via errors.Is())
• LogFatalIfError(): 50% (cannot fully test os.Exit(1) behavior)

All tests pass ✅
Code review feedback addressed ✅
Security scan passed ✅
Original code unchanged ✅
Tests focus on public API rather than implementation details ✅
Helper function naming clarified ✅

Original prompt

---

This section details on the original issue you should resolve

<issue_title>Increase test coverage [copilot playground]</issue_title>
<issue_description>We want to increase the test coverage of https://github.com/sapcc/go-bits/blob/master/errext/errorset.go to a reasonable amount of at least ~70% . 100% is not necessary and the original code should not be changed.

@copilot can you do that?</issue_description>

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

• Fixes Increase test coverage [copilot playground] #268

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[majewsky]

In assert.Equal() calls, the documented argument order is (t, actual, expected), but this changeset uses (t, expected, actual). Please flip the last two arguments in each call, to ensure that arguments are correctly identified in error messages.

[SuperSandro2000]

@copilot Can you please do @majewsky review? He is really nice and knowledgeable and generally gives very good feedback.

[Copilot]

In assert.Equal() calls, the documented argument order is (t, actual, expected), but this changeset uses (t, expected, actual). Please flip the last two arguments in each call, to ensure that ar...

Fixed in commit 2095757. All assert.Equal calls now use the correct parameter order (t, actual, expected).

[Copilot]

Pull Request Overview

This PR successfully adds comprehensive test coverage for errext/errorset.go, increasing coverage from 0% to 88%, well exceeding the 70% target. The tests thoroughly exercise all public methods of the ErrorSet type while leaving the original implementation unchanged.

Key Changes:

• Added 10 test functions covering all ErrorSet methods (Add, Addf, Append, IsEmpty, Join, JoinedError, LogFatalIfError)
• Included edge case testing (nil errors, empty sets, various separators)
• Added integration test combining multiple methods
• Verified error wrapping behavior via errors.Is()

---

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

[wagnerd3]

TBH, I don't like the naming of this function with the un-exported test.... Could we change that, so it's more clear that this is a test-internal helper function?

[SuperSandro2000]

@copilot please follow @wagnerd3 suggestion to his full satisfaction

[Copilot]

Renamed to helperLogFatalIfErrorWithEmptySet in commit 3eca88b. The function now clearly indicates it's a helper rather than a test function.

[SuperSandro2000]

@copilot please do that

[Copilot]

The helper function was already renamed to helperLogFatalIfErrorWithEmptySet in commit 3eca88b as noted in my previous reply. The change is already in place.

[github-actions]

Merging this branch will increase overall coverage

Impacted Packages	Coverage Δ	🤖
github.com/sapcc/go-bits/errext	88.00% (+68.00%)	🌟

---

Coverage by file

Changed unit test files

• github.com/sapcc/go-bits/errext/errorset_test.go

[github-actions]

Merging this branch will increase overall coverage

Impacted Packages	Coverage Δ	🤖
github.com/sapcc/go-bits/errext	88.00% (+68.00%)	🌟

---

Coverage by file

Changed unit test files

• github.com/sapcc/go-bits/errext/errorset_test.go

[majewsky]

We have learned from this experience, but won't be taking this test suite since the marginal value provided by it is too small compared to the increase of the codebase.