Review use of AssertEquals with nil #3279
Labels
Comments
aarongable
added a commit
that referenced
this issue
Aug 19, 2022
Add a new `test.AssertNil()` helper to facilitate asserting that a given unit test result is a non-boxed nil. Update `test.AssertNotNil()` to use the reflect package's `.IsNil()` method to catch boxed nils. In Go, variables whose type is constrained to be an interface type (e.g. a function parameter which takes an interface, or the return value of a function which returns `error`, itself an interface type) should actually be thought of as a (T, V) tuple, where T is their underlying concrete type and V is their underlying value. Thus, there are two ways for such a variable to be nil-like: it can be truly nil where T=nil and V is uninitialized, or it can be a "boxed nil" where T is a nillable type such as a pointer or a slice and V=nil. Unfortunately, only the former of these is == nil. The latter is the cause of frequent bugs, programmer frustration, a whole entry in the Go FAQ, and considerable design effort to remove from Go 2. Therefore these two test helpers both call `t.Fatal()` when passed a boxed nil. We want to avoid passing around boxed nils whenever possible, and having our tests fail whenever we do is a good way to enforce good nil hygiene. Fixes #3279
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
In https://github.com/letsencrypt/boulder/pull/3278/files, @cpu noticed that
test.AssertEqualsgave incorrect results when checking whether a value was nil. I believe this is due to Go's "boxed nil" semantics. There are 5 other instances of using AssertEquals with nil. We should double-check each of those and fix them if they are affected.The text was updated successfully, but these errors were encountered: