From 3063baf3a1068b497f760093669df5237d1f8c27 Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Mon, 4 Nov 2024 23:18:43 +0100 Subject: [PATCH 1/5] chore: use testify instead of t.Fatal Signed-off-by: Matthieu MOREL --- cmd/agent/app/agent_test.go | 14 +++++--------- internal/metrics/prometheus/factory_test.go | 4 +--- pkg/gogocodec/codec_test.go | 4 +--- plugin/storage/integration/elasticsearch_test.go | 10 ++++------ 4 files changed, 11 insertions(+), 21 deletions(-) diff --git a/cmd/agent/app/agent_test.go b/cmd/agent/app/agent_test.go index e11e88e2769..b54a305ea44 100644 --- a/cmd/agent/app/agent_test.go +++ b/cmd/agent/app/agent_test.go @@ -44,9 +44,7 @@ func TestAgentSamplingEndpoint(t *testing.T) { } select { case err := <-errorch: - if err != nil { - t.Fatalf("error from agent: %s", err) - } + require.NoErrorf(t, err, "error from agent") break wait_loop default: time.Sleep(time.Millisecond) @@ -155,9 +153,8 @@ func TestStartStopRace(t *testing.T) { // Before the bug was fixed this test was failing as expected when // run with -race flag. - if err := agent.Run(); err != nil { - t.Fatalf("error from agent.Run(): %s", err) - } + err = agent.Run() + require.NoErrorf(t, err, "error from agent.Run()") t.Log("stopping agent") agent.Stop() @@ -192,9 +189,8 @@ func TestStartStopWithSocketBufferSet(t *testing.T) { agent, err := cfg.CreateAgent(fakeCollectorProxy{}, zap.NewNop(), metricsFactory) require.NoError(t, err) - if err := agent.Run(); err != nil { - t.Fatalf("error from agent.Run(): %s", err) - } + err = agent.Run() + require.NoErrorf(t, err, "error from agent.Run()") t.Log("stopping agent") agent.Stop() diff --git a/internal/metrics/prometheus/factory_test.go b/internal/metrics/prometheus/factory_test.go index 6acede6b73f..82176c71656 100644 --- a/internal/metrics/prometheus/factory_test.go +++ b/internal/metrics/prometheus/factory_test.go @@ -401,9 +401,7 @@ func findMetric(t *testing.T, snapshot []*promModel.MetricFamily, name string, t continue } for _, m := range mf.GetMetric() { - if len(m.GetLabel()) != len(tags) { - t.Fatalf("Mismatching labels for metric %v: want %v, have %v", name, tags, m.GetLabel()) - } + require.Equalf(t, len(m.GetLabel()), len(tags), "Mismatching labels for metric %v: want %v, have %v", name, tags, m.GetLabel()) match := true for _, l := range m.GetLabel() { if v, ok := tags[l.GetName()]; !ok || v != l.GetValue() { diff --git a/pkg/gogocodec/codec_test.go b/pkg/gogocodec/codec_test.go index 9349e74c62f..5b9d3c5741a 100644 --- a/pkg/gogocodec/codec_test.go +++ b/pkg/gogocodec/codec_test.go @@ -97,9 +97,7 @@ func BenchmarkCodecUnmarshal25Spans(b *testing.B) { for i := 0; i < b.N; i++ { var trace model.Trace err := c.Unmarshal(bytes, &trace) - if err != nil { - b.Fatal(err) - } + require.NoError(b, err) } } diff --git a/plugin/storage/integration/elasticsearch_test.go b/plugin/storage/integration/elasticsearch_test.go index 57b0d8a1ed8..afb52bc3216 100644 --- a/plugin/storage/integration/elasticsearch_test.go +++ b/plugin/storage/integration/elasticsearch_test.go @@ -156,9 +156,8 @@ func healthCheck() error { func testElasticsearchStorage(t *testing.T, allTagsAsFields bool) { SkipUnlessEnv(t, "elasticsearch", "opensearch") - if err := healthCheck(); err != nil { - t.Fatal(err) - } + err := healthCheck() + require.NoError(t, err) s := &ESStorageIntegration{ StorageIntegration: StorageIntegration{ Fixtures: LoadAndParseQueryTestCases(t, "fixtures/queries_es.json"), @@ -182,9 +181,8 @@ func TestElasticsearchStorage_AllTagsAsObjectFields(t *testing.T) { func TestElasticsearchStorage_IndexTemplates(t *testing.T) { SkipUnlessEnv(t, "elasticsearch", "opensearch") - if err := healthCheck(); err != nil { - t.Fatal(err) - } + err := healthCheck() + require.NoError(t, err) s := &ESStorageIntegration{} s.initializeES(t, true) esVersion, err := s.getVersion() From 6889de783532bcc49b95f906adfc30dd973b6e39 Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Mon, 4 Nov 2024 23:43:35 +0100 Subject: [PATCH 2/5] Apply suggestions from code review Co-authored-by: Yuri Shkuro Signed-off-by: Matthieu MOREL --- cmd/agent/app/agent_test.go | 2 +- plugin/storage/integration/elasticsearch_test.go | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/cmd/agent/app/agent_test.go b/cmd/agent/app/agent_test.go index b54a305ea44..81848f6bdc3 100644 --- a/cmd/agent/app/agent_test.go +++ b/cmd/agent/app/agent_test.go @@ -44,7 +44,7 @@ func TestAgentSamplingEndpoint(t *testing.T) { } select { case err := <-errorch: - require.NoErrorf(t, err, "error from agent") + require.NoError(t, err, "error from agent") break wait_loop default: time.Sleep(time.Millisecond) diff --git a/plugin/storage/integration/elasticsearch_test.go b/plugin/storage/integration/elasticsearch_test.go index afb52bc3216..fb37746eb49 100644 --- a/plugin/storage/integration/elasticsearch_test.go +++ b/plugin/storage/integration/elasticsearch_test.go @@ -156,8 +156,7 @@ func healthCheck() error { func testElasticsearchStorage(t *testing.T, allTagsAsFields bool) { SkipUnlessEnv(t, "elasticsearch", "opensearch") - err := healthCheck() - require.NoError(t, err) + require.NoError(t, healthCheck()) s := &ESStorageIntegration{ StorageIntegration: StorageIntegration{ Fixtures: LoadAndParseQueryTestCases(t, "fixtures/queries_es.json"), From b21b8c1c814118bf07d84f30a8741fc33809ef48 Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Mon, 4 Nov 2024 23:48:34 +0100 Subject: [PATCH 3/5] Update factory_test.go Signed-off-by: Matthieu MOREL --- internal/metrics/prometheus/factory_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/metrics/prometheus/factory_test.go b/internal/metrics/prometheus/factory_test.go index 82176c71656..e4531d12a3e 100644 --- a/internal/metrics/prometheus/factory_test.go +++ b/internal/metrics/prometheus/factory_test.go @@ -401,7 +401,7 @@ func findMetric(t *testing.T, snapshot []*promModel.MetricFamily, name string, t continue } for _, m := range mf.GetMetric() { - require.Equalf(t, len(m.GetLabel()), len(tags), "Mismatching labels for metric %v: want %v, have %v", name, tags, m.GetLabel()) + require.Lenf(t, m.GetLabel(), len(tags), "Mismatching labels for metric %v: want %v, have %v", name, tags, m.GetLabel()) match := true for _, l := range m.GetLabel() { if v, ok := tags[l.GetName()]; !ok || v != l.GetValue() { From 9cb1e756bb665e2339e9fe3ab9d79eae7511a14e Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Mon, 4 Nov 2024 23:59:46 +0100 Subject: [PATCH 4/5] Update agent_test.go Signed-off-by: Matthieu MOREL --- cmd/agent/app/agent_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/cmd/agent/app/agent_test.go b/cmd/agent/app/agent_test.go index 81848f6bdc3..0cd3cb08d48 100644 --- a/cmd/agent/app/agent_test.go +++ b/cmd/agent/app/agent_test.go @@ -153,8 +153,7 @@ func TestStartStopRace(t *testing.T) { // Before the bug was fixed this test was failing as expected when // run with -race flag. - err = agent.Run() - require.NoErrorf(t, err, "error from agent.Run()") + require.NoError(t, agent.Run()) t.Log("stopping agent") agent.Stop() @@ -189,8 +188,7 @@ func TestStartStopWithSocketBufferSet(t *testing.T) { agent, err := cfg.CreateAgent(fakeCollectorProxy{}, zap.NewNop(), metricsFactory) require.NoError(t, err) - err = agent.Run() - require.NoErrorf(t, err, "error from agent.Run()") + require.NoError(t, agent.Run()) t.Log("stopping agent") agent.Stop() From bc98d24559bd470bd3a80e325b00f7e7a6b7f60a Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Tue, 5 Nov 2024 00:00:45 +0100 Subject: [PATCH 5/5] Update elasticsearch_test.go Signed-off-by: Matthieu MOREL --- plugin/storage/integration/elasticsearch_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugin/storage/integration/elasticsearch_test.go b/plugin/storage/integration/elasticsearch_test.go index fb37746eb49..5aae3614262 100644 --- a/plugin/storage/integration/elasticsearch_test.go +++ b/plugin/storage/integration/elasticsearch_test.go @@ -180,8 +180,7 @@ func TestElasticsearchStorage_AllTagsAsObjectFields(t *testing.T) { func TestElasticsearchStorage_IndexTemplates(t *testing.T) { SkipUnlessEnv(t, "elasticsearch", "opensearch") - err := healthCheck() - require.NoError(t, err) + require.NoError(t, healthCheck()) s := &ESStorageIntegration{} s.initializeES(t, true) esVersion, err := s.getVersion()