singleflight: Fix TestPanicErrorUnwrap#29
Conversation
|
This PR (HEAD: 265f52a) has been imported to Gerrit for code review. Please visit Gerrit at https://go-review.googlesource.com/c/sync/+/685736. Important tips:
|
|
Message from Gopher Robot: Patch Set 1: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/685736. |
|
Message from Jorropo: Patch Set 1: Code-Review+2 Commit-Queue+1 (2 comments) Please don’t reply on this GitHub thread. Visit golang.org/cl/685736. |
|
Message from Go LUCI: Patch Set 1: Dry run: CV is trying the patch. Bot data: {"action":"start","triggered_at":"2025-07-04T23:23:35Z","revision":"18045c4ea6c2ee676fda25dde409a1621812b6c2"} Please don’t reply on this GitHub thread. Visit golang.org/cl/685736. |
|
Message from Jorropo: Patch Set 1: -Commit-Queue Please don’t reply on this GitHub thread. Visit golang.org/cl/685736. |
|
Message from Go LUCI: Patch Set 1: This CL has passed the run Please don’t reply on this GitHub thread. Visit golang.org/cl/685736. |
|
Message from Go LUCI: Patch Set 1: LUCI-TryBot-Result+1 Please don’t reply on this GitHub thread. Visit golang.org/cl/685736. |
TestPanicErrorUnwrap does not test what it seems to test - "wrappedErrorType" is always false (should be true in the second case) - The values are already wrapped in the private error (they shouldn't) - "errors.Is(err, new(T))" is undefined for zero-sized "T"s (and false for non-zero-sized). - "tc := tc" is not needed since Go 1.22 Fixes: - Use a (local) sentinel value instead of a type, as in TestDoErr. - Replace all "new(errValue)" by direct references to the unique sentinel "errValue". - Replace diagnostic, since the error type we get is internal, so we can't directly expect that - it should only wrap our sentinel. Signed-off-by: Oliver Eikemeier <eikemeier@fillmore-labs.com>
|
This PR (HEAD: 63ca69c) has been imported to Gerrit for code review. Please visit Gerrit at https://go-review.googlesource.com/c/sync/+/685736. Important tips:
|
|
Message from Sean Liao: Patch Set 2: Code-Review+1 (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/685736. |
|
Message from Sean Liao: Patch Set 2: -Code-Review (2 comments) Please don’t reply on this GitHub thread. Visit golang.org/cl/685736. |
TestPanicErrorUnwrap does not test what it seems to test
Fixes:
See my blog post at https://blog.fillmore-labs.com/posts/zerosized-2/ for a extended discussion.
Found by zerolint https://github.com/fillmore-labs/zerolint