Conversation
sae/rpc_test.go
Outdated
| t.Run(tc.method, func(t *testing.T) { | ||
| if tc.wantErr != "" { | ||
| err := call(t, nil, tc) // won't unmarshal anything | ||
| require.ErrorContains(t, err, tc.wantErr) |
There was a problem hiding this comment.
I wish we could use require.ErrorIs, but almost all of these errors are from libevm
JonathanOppenheimer
left a comment
JonathanOppenheimer
left a comment
There was a problem hiding this comment.
I am a big fan of this change and will use it in my PRs :)
sae/rpc_test.go
Outdated
| if tc.want == nil { | ||
| require.NoError(t, call(t, nil, tc)) | ||
| return | ||
| } |
There was a problem hiding this comment.
This code doesn't seem to be used in this PR fyi
There was a problem hiding this comment.
Now it is! Kind of, it still doesn't unmarshal anything
sae/rpc_test.go
Outdated
| cmputils.TransactionsByHash(), | ||
| } | ||
|
|
||
| call := func(t *testing.T, store any, tc rpcTest) error { |
There was a problem hiding this comment.
I don't think it's necessary to pull this out and call it multiple times. It's possible to use a black-hole json.RawMessage when want == nil and to do an in-line error check.
About to do the school run and I'll send an example when I'm back.
EDIT: see #159
ARR4N
left a comment
ARR4N
left a comment
There was a problem hiding this comment.
Special-casing tests is generally a sign that either (a) the test cases should be separated into different tables; or (b) the test loop needs refactoring. Packages like errdiff were developed to avoid this, sometimes allowing the testing to keep going (like in #159) but sometimes only requiring the loop to end early when an error is expected.
I like the Also, is it idiomatic to check the return VALUE if an error is returned? I had learned through observation that it doesn't happen often, but that doesn't make it good practice |
5 participants
Credit to @powerslider for the implementation here, but thought we should pull it out from #154, since it can be used elsewhere, and can easily be isolated.