Skip to content

Add test coverage for nil Len() behavior#142

Open
veeceey wants to merge 1 commit intohashicorp:mainfrom
veeceey:fix/issue-54-nil-len-panic
Open

Add test coverage for nil Len() behavior#142
veeceey wants to merge 1 commit intohashicorp:mainfrom
veeceey:fix/issue-54-nil-len-panic

Conversation

@veeceey
Copy link

@veeceey veeceey commented Feb 13, 2026

Summary

The nil check in Len() was added in PR #95 but didn't include test coverage. This PR adds TestLenNil to verify that calling Len() on a nil *Error returns 0 instead of panicking.

Test Results

$ go test ./... -v
...
=== RUN   TestLenNil
--- PASS: TestLenNil (0.00s)
PASS
ok      github.com/hashicorp/go-multierror      0.243s

Verified manually that nil and empty multierror both return 0:

var merr *multierror.Error
fmt.Printf("Len of nil multierror: %d\n", merr.Len())  // Output: 0

merr = &multierror.Error{}
fmt.Printf("Len of empty multierror: %d\n", merr.Len())  // Output: 0

Fixes #54

@veeceey veeceey requested a review from a team as a code owner February 13, 2026 08:48
@ritikrajdev
Copy link
Contributor

Hey @veeceey , Thanks for the contribution. This test case looks good to me but can you please resolve the failing lint CI !

@veeceey
Copy link
Author

veeceey commented Feb 16, 2026

Thanks for the review, @ritikrajdev! I looked into the lint failure and it's not related to my changes — the lint job uses golangci-lint-action without pinning a version, so it pulled golangci-lint v2.9.0 (latest), which passes the -buildvcs flag to go list. That flag was introduced in Go 1.18, but the repo's go.mod specifies Go 1.13, which is what the lint job uses via go-version-file: go.mod.

The lint job last passed on main on Jan 30 when an older golangci-lint version was in use. Since then, the latest version has moved to v2.9.0 and any new PR branch will hit this same failure. This is a pre-existing CI configuration issue — not caused by the test I added.

All actual test jobs (linux and windows across Go 1.13, oldstable, and stable) pass.

@ritikrajdev
Copy link
Contributor

can you please pull the latest changes once!

@veeceey veeceey force-pushed the fix/issue-54-nil-len-panic branch from f15b2a8 to 11135dd Compare February 16, 2026 19:11
@veeceey
Copy link
Author

veeceey commented Feb 18, 2026

Hey @ritikrajdev — I rebased on latest main, but the lint failure will persist since it's a pre-existing CI issue: golangci-lint-action now pulls v2.9.0 which requires Go 1.18+, but the repo's go.mod specifies Go 1.13. This affects all PRs, not just mine.

All actual test jobs (Linux + Windows across Go 1.13, oldstable, and stable) pass cleanly. The fix would be to either pin the golangci-lint version in CI or update the minimum Go version, but that's outside the scope of this PR.

Let me know if there's anything else I can do!

@ritikrajdev
Copy link
Contributor

This test actually looks ok to me, let me quickly fix the CI and merge it!

@ritikrajdev
Copy link
Contributor

Hey the fix is now merged in main, feel please update the branch.

The nil check in Len() was added in PR hashicorp#95 but lacked test coverage.
This adds TestLenNil to verify that calling Len() on a nil *Error
returns 0 instead of panicking.

Fixes hashicorp#54
@veeceey veeceey force-pushed the fix/issue-54-nil-len-panic branch from 11135dd to cb21984 Compare March 13, 2026 04:12
@veeceey
Copy link
Author

veeceey commented Mar 13, 2026

rebased onto latest main - should be good to go now!

@veeceey
Copy link
Author

veeceey commented Mar 25, 2026

Already rebased on latest main — should be good to go now. The CI fix from #147 is included.

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.

Len() with nil multierror should return 0 instead of panic

2 participants