Conversation
libevm/ethtest/logger_test.go
Outdated
| l.Info("Hello", "who", "world") | ||
| l.Warn("Smoke") | ||
| l.Error("Fire") | ||
| // Crit will call os.Exit(1) so we don't test it. |
There was a problem hiding this comment.
Not saying this is necessarily better, but I did find a janky way to test this:
func (r *tbRecorder) Fatalf(format string, a ...any) {
panic(fmt.Sprintf(format, a...))
}
// in test
require.Panics(t, func() { l.Crit("Explosion") })By panicking, we avoid calling os.Exit, but we can ensure that it's called. This might be awful though, so don't feel the need to include this. Just being creative
There was a problem hiding this comment.
If panic is okay behavior, I'm fine with this. I'll implement and let someone else chime in.
Co-authored-by: Arran Schlosberg <519948+ARR4N@users.noreply.github.com> Signed-off-by: Jonathan Oppenheimer <147infiniti@gmail.com>
libevm/ethtest/logger_test.go
Outdated
| doLogging := func(t *testing.T, l log.Logger) { | ||
| t.Helper() | ||
| l.Debug("Hi") | ||
| l.Info("Austin", "you", "are") |
There was a problem hiding this comment.
What do you mean what happened?
There was a problem hiding this comment.
What's the point of writing it in the test if you don't assert that the strings are carried through?
There was a problem hiding this comment.
Oh yeah when I reworked it I just accessed from the message. I was just trying to keep it fun. I changed it now so everything is covered, and it reads properly in the setup and in the test table 😂.
3 participants
Why this should be merged
We decided that geth warnings should not be surfaced as errors in strevm, due to there being far too many false/meaningless warnings. Now
Enabled()enables all logs (visible viatb.Logffor debugging) andHandle()usesh.levelas thetb.Errorfthreshold, so e.g.NewTBLogHandler(tb, slog.LevelError)only fails the test on errors, not warnings.How this was tested
I updated
TestTBLogHandlerappropriately.