From 18c4ba03861843d549dc5d2c9532e4c95c589ee1 Mon Sep 17 00:00:00 2001 From: Nodar Ambroladze Date: Thu, 30 May 2024 17:37:56 +0200 Subject: [PATCH 1/4] Add tests to check errors are logged when context is cancelled --- validator/valnode/redis/consumer.go | 6 ++--- validator/valnode/redis/consumer_test.go | 29 ++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) create mode 100644 validator/valnode/redis/consumer_test.go diff --git a/validator/valnode/redis/consumer.go b/validator/valnode/redis/consumer.go index 3569e78b5c..016f30bd61 100644 --- a/validator/valnode/redis/consumer.go +++ b/validator/valnode/redis/consumer.go @@ -70,7 +70,7 @@ func (s *ValidationServer) Start(ctx_in context.Context) { } select { case <-ctx.Done(): - log.Info("Context done", "error", ctx.Err().Error()) + log.Info("Context done while checking redis stream existance", "error", ctx.Err().Error()) return case <-time.After(time.Millisecond * 100): } @@ -79,7 +79,7 @@ func (s *ValidationServer) Start(ctx_in context.Context) { s.StopWaiter.LaunchThread(func(ctx context.Context) { select { case <-ctx.Done(): - log.Info("Context done", "error", ctx.Err().Error()) + log.Info("Context done while waiting a redis stream to be ready", "error", ctx.Err().Error()) return case <-ready: // Wait until the stream exists and start consuming iteratively. } @@ -116,7 +116,7 @@ func (s *ValidationServer) Start(ctx_in context.Context) { case <-time.After(s.streamTimeout): log.Error("Waiting for redis streams timed out") case <-ctx.Done(): - log.Info(("Context expired, failed to start")) + log.Info("Context done while waiting redis streams to be ready, failed to start") return } } diff --git a/validator/valnode/redis/consumer_test.go b/validator/valnode/redis/consumer_test.go new file mode 100644 index 0000000000..e7ecb40c8a --- /dev/null +++ b/validator/valnode/redis/consumer_test.go @@ -0,0 +1,29 @@ +package redis + +import ( + "context" + "testing" + "time" + + "github.com/ethereum/go-ethereum/log" + "github.com/offchainlabs/nitro/util/redisutil" + "github.com/offchainlabs/nitro/util/testhelpers" +) + +func TestTimeout(t *testing.T) { + handler := testhelpers.InitTestLog(t, log.LevelInfo) + ctx, cancel := context.WithCancel(context.Background()) + redisURL := redisutil.CreateTestRedis(ctx, t) + TestValidationServerConfig.RedisURL = redisURL + TestValidationServerConfig.ModuleRoots = []string{"0x123"} + vs, err := NewValidationServer(&TestValidationServerConfig, nil) + if err != nil { + t.Fatalf("NewValidationSever() unexpected error: %v", err) + } + vs.Start(ctx) + cancel() + time.Sleep(time.Second) + if !handler.WasLogged("Context done while waiting redis streams to be ready") { + t.Errorf("Context cancelled but error was not logged") + } +} From 388f08db02d77921f38c0c644601a41af30404ad Mon Sep 17 00:00:00 2001 From: Nodar Ambroladze Date: Fri, 31 May 2024 08:48:21 +0200 Subject: [PATCH 2/4] Check timeout logs --- validator/valnode/redis/consumer_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/validator/valnode/redis/consumer_test.go b/validator/valnode/redis/consumer_test.go index e7ecb40c8a..6dd2395cf2 100644 --- a/validator/valnode/redis/consumer_test.go +++ b/validator/valnode/redis/consumer_test.go @@ -24,6 +24,6 @@ func TestTimeout(t *testing.T) { cancel() time.Sleep(time.Second) if !handler.WasLogged("Context done while waiting redis streams to be ready") { - t.Errorf("Context cancelled but error was not logged") + t.Errorf("Waiting for redis streams timed out") } } From f12bc4e9f54d06d369282e4c5000405b4fc0e50c Mon Sep 17 00:00:00 2001 From: Nodar Ambroladze Date: Fri, 31 May 2024 15:15:10 +0200 Subject: [PATCH 3/4] Word test error message clearer --- validator/valnode/redis/consumer_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/validator/valnode/redis/consumer_test.go b/validator/valnode/redis/consumer_test.go index 6dd2395cf2..e25169a79e 100644 --- a/validator/valnode/redis/consumer_test.go +++ b/validator/valnode/redis/consumer_test.go @@ -24,6 +24,6 @@ func TestTimeout(t *testing.T) { cancel() time.Sleep(time.Second) if !handler.WasLogged("Context done while waiting redis streams to be ready") { - t.Errorf("Waiting for redis streams timed out") + t.Error("Expected message about stream time-outs was not logged") } } From c621a9cb41d3b061018ffe1608adaa282522ff71 Mon Sep 17 00:00:00 2001 From: Nodar Ambroladze Date: Mon, 3 Jun 2024 12:48:38 +0200 Subject: [PATCH 4/4] Fix test --- validator/valnode/redis/consumer_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/validator/valnode/redis/consumer_test.go b/validator/valnode/redis/consumer_test.go index e25169a79e..0ebd697f16 100644 --- a/validator/valnode/redis/consumer_test.go +++ b/validator/valnode/redis/consumer_test.go @@ -16,14 +16,15 @@ func TestTimeout(t *testing.T) { redisURL := redisutil.CreateTestRedis(ctx, t) TestValidationServerConfig.RedisURL = redisURL TestValidationServerConfig.ModuleRoots = []string{"0x123"} + TestValidationServerConfig.StreamTimeout = 100 * time.Millisecond vs, err := NewValidationServer(&TestValidationServerConfig, nil) if err != nil { t.Fatalf("NewValidationSever() unexpected error: %v", err) } vs.Start(ctx) - cancel() time.Sleep(time.Second) - if !handler.WasLogged("Context done while waiting redis streams to be ready") { + if !handler.WasLogged("Waiting for redis streams timed out") { t.Error("Expected message about stream time-outs was not logged") } + cancel() }