Initial implementation of the expect package

[jonathansharman]

Dependencies

• Consumed by map@x/expect (diff with dev)
• Consumed by fact@x/expect (diff with dev)

Documentation

• BE Initiative task
• BENG topic 1
• BENG topic 2

Description

Initial implementation of the expect package, which provides a few simple test functions to reduce boilerplate in our unit tests. I've experimented with this in map@x/expect) and fact@x/expect (no plans to merge those branches for the time being).

Nil Errors

Currently in our unit tests we usually store an expected error in our test case struct, and we often omit that field from test cases where the error is expected to be nil, relying on the zero value of error. @wmarshall floated the idea at BENG of making the zero value of ErrorCheck useful (checking that err == nil by default) so that we can continue that pattern. I can think of a couple ways to do this:

1. Change ErrorCheck into a struct containing a function. Give ErrorCheck a method that skips calling this function field if it's nil and instead calls ErrorNil.
2. Keep ErrorCheck a function type but add a method similar to option 1:

   func (ec ErrorCheck) Check(t *testing.T, err error) {
   	t.Helper()
   	if ec == nil {
   		ErrorNil(t, err)
   	} else {
   		ec(t, err)
   	}
   }

3. Keep it as it is, requiring an explicit ErrorNil if you want that check. Worth noting that for test tables that all use expect.ErrorIs, we can keep the error field in the test case struct and continue to (implicitly) pass nil, if we want. (I also don't think it's necessarily bad having to be explicit about expected errors.)

Option 1 is safe but adds a little noise to test bodies since we have to call a method now instead of just using (). Option 2 keeps the shorter syntax available but also makes it easy to do the wrong thing, and it means we have two ways to do essentially the same thing. The existing version has consistent and brief test body syntax but requires initialization. Pick your poison? 🤔

More Specific Equal Checks

Perhaps controversially, I haven't included special functions for checking stub call counts, expected arguments, etc. In my opinion, these don't provide much value over the generic expect.Equal. expect.Equal is a helper (i.e. calls t.Helper), so if the check fails, the testing package will print not only the name of the failed test but also the line number of the call. Checking the line where the check failed should typically give enough context for its purpose.

cmpopts Wrappers

The cmpopts package already provides options for things like approximate comparisons (EquateApprox, EquateApproxTime), etc.

I did include EqualUnordered, which is a kind of wrapper for the SortSlices option because I find that one a little verbose in the common case where we just want to use < for the lessFunc. We should keep an eye out for other commonly used options to see if they're worth baking into expect. That said, the disadvantage of named wrappers is that we can't compose them as easily as cmp.Options.

Test Error Values

Instead of or in addition to ErrTest, I considered a function that's just like errors.New except that it returns the exact same error for the same input. For example, two different calls to expect.NewTestError("foo") would be ==-equal - and would therefore also be errors.Is-equal. (Implementation-wise, we could store the errors in an unexported package-scope map keyed by error message.) This would allow tests to use multiple different errors - as many as necessary - without having to pre-declare them. I think there are few if any real use cases for this though, so I left this out for now.

Versioning

Version 0.1.0. This is somewhat experimental and probably not ready for v1.

[jonathansharman]

Based on https://github.com/nicheinc/pgsql/blob/main/.github/workflows/ci.yml, as is much of the library boilerplate.

[jonathansharman]

None of these badges will work unless we decide to open-source this. (Not sure if that's something we'd want to do, but I also don't see a reason why we couldn't.)

[wmarshall]

I'd be good with feeding the OSS machine

[wmarshall]

I'd be good with feeding the OSS machine

[wmarshall]

Thoughts on an signature/usage pattern more like

func CheckError(t *testing.T, check ErrorCheck, err error) {
	if check == nil {
		check = ErrorNil
	}
	check(t, err)
}

s.t. the zero value for ErrorCheck is equivalent to ErrorNil?

Might save a bunch of boilerplate, but we could add it after the initial release

[wmarshall]

I guess callers could do this as well

[jonathansharman]

I definitely want to minimize boilerplate if possible. (That is this library's raison d'etre after all.) This approach sounds pretty similar to option 2 in the "Nil Errors" part of the PR writeup (the difference being whether the guarded function is standalone or a method on ErrorCheck) - right? I've been leaning slightly towards either option 1 or option 3 to avoid inconsistencies/foot-guns, but I could be swayed in any direction. 🤷‍♂️

[wmarshall]

I definitely missed that section. If you're feeling hesitant, let's keep option 3 - we can always add functionality like this in the future. I don't love option 1 purely on the basis of it feeling clunky to add a custom check.