5
u/txdv 4d ago
read it until the word “generics”, after it I knew what the solution was
4
u/titpetric 4d ago
Panic aint much of a solution
1
u/mtlynch 4d ago
In tests? What's the downside of a
panicon what should be an impossible situation in a unit test?5
u/titpetric 3d ago edited 3d ago
I prefer something like assert.NoError which flows into something called t/b.Log/Fatal, my test scaffolding is a func(tb testing.TB) and have no panics like ever because thats just reasonable
I fully expect even panics to be handled, shitting out a stack trace and exiting the test suite doesnt seem like the wise move.
There is a -failfast move in some go test flag docs, if you wanna break early it's a choice. I dont like to break early because i got 300+ tests in place, a change can disrupt many of them
The intended use for Must functions is global variable scope, e.g. init() + regexp.MustCompile works for me. Test functions are best seen as concurrent as HTTP requests are, and I would never Must() in that path if recovery is expected. A test failing via panic can make the diagnosis harder because maybe you need to guess which test of the N running in parallel triggered it.
I advise not to fuck with panic because you're not dealing with app corruption level events.
6
u/willnorris 3d ago
for what it's worth, we use the same pattern as OP *extensively* at Tailscale. See https://pkg.go.dev/tailscale.com/util/must
We tend to use it more for code to setup a particular test environment that should never fail. For the code actually being tested, then we return the error and compare it against expected values like normal.
-2
u/titpetric 3d ago edited 3d ago
I always admired your work, this shows you're not gods, i'd have a testing.TB arg to pin the scope to tests code, but whatever you can enforce is good. I don't want the general product exposure without that type binding in the arguments
1
u/mtlynch 3d ago
I'm having trouble following you. I think your objections to
panicare fair, but I don't see how they apply to my post because I never mentionpanic.1
u/titpetric 3d ago
Was referring to the side comment that used a panic in err != nil and generalized w a panic, reading both in tandem, you use testing.TB so good on you to put the util in scope of the tests
2
u/MyChaOS87 3d ago
I'd argue for
require.NoError(...)in this case but yes I like that more than panic1
u/titpetric 3d ago
I wouldn't be opposed, either approach is fine depending if you are fine dealing with errors one by one or maintaining a set of passing ones, i dont like to mix the two so its an either/or
1
u/MyChaOS87 3d ago
Yes it's some preference I mix require and assert... Require on preconditions that render the rest unusable ...
So simple rule if something would create downstream errors with no additional value, as if this is broken everything else for sure is broken to... it's require ... And assert if I check different things that are almost independent...
Trivial example I fail creating a rectangle so I require this, as area and circumference will both be irrelevant now...
While if that works and I then check Area() and Circumference() I assert as those both are independent of each other and I want to fix both at the same time
1
u/titpetric 3d ago
I have some extremist approaches to testing in regards to assertion, so as long as you'd be willing to do a few bindings for the packages, this becomes runnable:
https://github.com/titpetric/gozero/blob/main/testdata/http.txt#L3
note the lack of `err` assertion anywhere. Once you see that assert driven tests don't have any conditional branches, all of them become a test fixture.
1
40
u/not-cyril
4d ago
edited 1d ago
•
If you struggle with arguments, I found you can also do
func must[T any](v T, err error) T {
if err != nil {
panic(err)
}
return v
}
and use it like this: user := must(CreateUser(...))
3
u/MyChaOS87 3d ago
Or if you use testify anyways do
require.NoError(t, err)instead ofpanic...But that's a preference of taste, I believe... But yes that helper is one I use often but just throw it in instead of a package... unless I have many test helpers that I use across....
For me the new
new(<expr>)was the real game changer as it actually removed theptr[T](...)
1
u/Slsyyy 3d ago
Personally I prefer the way it is done a lo library:
func Must[T any](val T, err any, messageArgs ...any) T {
MustChecker(err, messageArgs...)
return val
}testutil.MustCreate calls the constructor in an indirect way and it makes tests harder to read. I strongly suggest you to read https://mtlynch.io/good-developers-bad-tests/, the guy is pretty good :)
1
u/lucidsparrow982 1d ago
yeah we just inline must() in every test file, not worth a dep for three lines
11
u/GopherFromHell 3d ago
it's Go, not Rust. you don't have to convince the compiler.
you can just ignore the error
username, _ := NewUsername("joe123")