From ed96f74d9cb251bf707e17252e7fb4ea74ecd5a6 Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Wed, 4 Dec 2024 19:46:25 +0100 Subject: [PATCH] [chore]: use testify instead of testing.Fatal or testing.Error in processor (#36374) --- processor/attributesprocessor/factory_test.go | 3 +- .../internal/tracking/identity_test.go | 9 ++--- .../internal/tracking/tracker_test.go | 8 ++--- .../processor_test.go | 4 +-- .../internal/testing/testar/read_test.go | 10 +++--- .../processor_test.go | 4 +-- .../internal/kube/client_test.go | 10 ++---- .../k8sattributesprocessor/options_test.go | 26 ++++++-------- .../k8sattributesprocessor/processor_test.go | 6 ++-- .../metricstransformprocessor/factory_test.go | 4 +-- .../logsprocessor_test.go | 4 +-- .../tracesprocessor_test.go | 34 +++++++------------ .../internal/idbatcher/id_batcher_test.go | 4 +-- .../internal/sampling/composite_test.go | 8 ++--- ...gregate_on_attribute_value_metrics_test.go | 8 ++--- ..._exponential_hist_to_explicit_hist_test.go | 3 +- 16 files changed, 52 insertions(+), 93 deletions(-) diff --git a/processor/attributesprocessor/factory_test.go b/processor/attributesprocessor/factory_test.go index cda5d91213e9..c210ffd08a1d 100644 --- a/processor/attributesprocessor/factory_test.go +++ b/processor/attributesprocessor/factory_test.go @@ -52,8 +52,7 @@ func TestFactoryCreateTraces_InvalidActions(t *testing.T) { {Key: "http.status_code", ConvertedType: "array", Action: attraction.CONVERT}, } ap2, err2 := factory.CreateTraces(context.Background(), processortest.NewNopSettings(), cfg, consumertest.NewNop()) - assert.Error(t, err2) - assert.Equal(t, "error creating AttrProc due to invalid value \"array\" in field \"converted_type\" for action \"convert\" at the 0-th action", err2.Error()) + require.EqualError(t, err2, "error creating AttrProc due to invalid value \"array\" in field \"converted_type\" for action \"convert\" at the 0-th action") assert.Nil(t, ap2) } diff --git a/processor/cumulativetodeltaprocessor/internal/tracking/identity_test.go b/processor/cumulativetodeltaprocessor/internal/tracking/identity_test.go index 343185c248bd..0d8d7b053a53 100644 --- a/processor/cumulativetodeltaprocessor/internal/tracking/identity_test.go +++ b/processor/cumulativetodeltaprocessor/internal/tracking/identity_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" + "github.com/stretchr/testify/assert" "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/pmetric" @@ -137,9 +138,7 @@ func TestMetricIdentity_IsFloatVal(t *testing.T) { MetricType: pmetric.MetricTypeSum, MetricValueType: tt.fields.MetricValueType, } - if got := mi.IsFloatVal(); got != tt.want { - t.Errorf("MetricIdentity.IsFloatVal() = %v, want %v", got, tt.want) - } + assert.Equal(t, tt.want, mi.IsFloatVal(), "MetricIdentity.IsFloatVal()") }) } } @@ -204,9 +203,7 @@ func TestMetricIdentity_IsSupportedMetricType(t *testing.T) { Attributes: pcommon.NewMap(), MetricType: tt.fields.MetricType, } - if got := mi.IsSupportedMetricType(); got != tt.want { - t.Errorf("MetricIdentity.IsSupportedMetricType() = %v, want %v", got, tt.want) - } + assert.Equal(t, tt.want, mi.IsSupportedMetricType(), "MetricIdentity.IsSupportedMetricType()") }) } } diff --git a/processor/cumulativetodeltaprocessor/internal/tracking/tracker_test.go b/processor/cumulativetodeltaprocessor/internal/tracking/tracker_test.go index 8312532b7e4c..8ebcdb188f23 100644 --- a/processor/cumulativetodeltaprocessor/internal/tracking/tracker_test.go +++ b/processor/cumulativetodeltaprocessor/internal/tracking/tracker_test.go @@ -242,9 +242,7 @@ func TestMetricTracker_Convert(t *testing.T) { IntValue: 100, }, }) - if valid { - t.Error("Expected invalid for non cumulative metric") - } + assert.False(t, valid, "Expected invalid for non cumulative metric") }) } @@ -338,7 +336,5 @@ func Test_metricTracker_sweeper(t *testing.T) { cancel() for range sweepEvent { // nolint } - if !closed.Load() { - t.Errorf("Sweeper did not terminate.") - } + assert.True(t, closed.Load(), "Sweeper did not terminate.") } diff --git a/processor/cumulativetodeltaprocessor/processor_test.go b/processor/cumulativetodeltaprocessor/processor_test.go index 1e394c26af70..d7b0a19ab6cc 100644 --- a/processor/cumulativetodeltaprocessor/processor_test.go +++ b/processor/cumulativetodeltaprocessor/processor_test.go @@ -623,9 +623,7 @@ func BenchmarkConsumeMetrics(b *testing.B) { } cfg := createDefaultConfig().(*Config) p, err := createMetricsProcessor(context.Background(), params, cfg, c) - if err != nil { - b.Fatal(err) - } + require.NoError(b, err) metrics := pmetric.NewMetrics() rms := metrics.ResourceMetrics().AppendEmpty() diff --git a/processor/deltatocumulativeprocessor/internal/testing/testar/read_test.go b/processor/deltatocumulativeprocessor/internal/testing/testar/read_test.go index ac65a7497b86..842ace3eca58 100644 --- a/processor/deltatocumulativeprocessor/internal/testing/testar/read_test.go +++ b/processor/deltatocumulativeprocessor/internal/testing/testar/read_test.go @@ -9,6 +9,8 @@ import ( "strings" "testing" + "github.com/stretchr/testify/require" + "github.com/open-telemetry/opentelemetry-collector-contrib/processor/deltatocumulativeprocessor/internal/testing/testar/crlf" ) @@ -71,16 +73,12 @@ func TestCRLF(t *testing.T) { } err := Read(data, &into) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) must(t, into.String, "foobar\n") } func must[T string | int](t *testing.T, v, want T) { t.Helper() - if v != want { - t.Fatalf("got '%q' != '%q' want", v, want) - } + require.Equal(t, want, v, "got '%q' != '%q' want", v, want) } diff --git a/processor/deltatocumulativeprocessor/processor_test.go b/processor/deltatocumulativeprocessor/processor_test.go index cbddc68ef5d5..23b05dcaa1ad 100644 --- a/processor/deltatocumulativeprocessor/processor_test.go +++ b/processor/deltatocumulativeprocessor/processor_test.go @@ -160,7 +160,5 @@ func TestTelemetry(t *testing.T) { require.NoError(t, err) var rm metricdata.ResourceMetrics - if err := tt.reader.Collect(context.Background(), &rm); err != nil { - t.Fatal(err) - } + require.NoError(t, tt.reader.Collect(context.Background(), &rm)) } diff --git a/processor/k8sattributesprocessor/internal/kube/client_test.go b/processor/k8sattributesprocessor/internal/kube/client_test.go index f05e84f227d4..de701f6fd673 100644 --- a/processor/k8sattributesprocessor/internal/kube/client_test.go +++ b/processor/k8sattributesprocessor/internal/kube/client_test.go @@ -146,8 +146,7 @@ func nodeAddAndUpdateTest(t *testing.T, c *WatchClient, handler func(obj any)) { func TestDefaultClientset(t *testing.T) { c, err := New(componenttest.NewNopTelemetrySettings(), k8sconfig.APIConfig{}, ExtractionRules{}, Filters{}, []Association{}, Excludes{}, nil, nil, nil, nil, false, 10*time.Second) - assert.Error(t, err) - assert.Equal(t, "invalid authType for kubernetes: ", err.Error()) + require.EqualError(t, err, "invalid authType for kubernetes: ") assert.Nil(t, c) c, err = New(componenttest.NewNopTelemetrySettings(), k8sconfig.APIConfig{}, ExtractionRules{}, Filters{}, []Association{}, Excludes{}, newFakeAPIClientset, nil, nil, nil, false, 10*time.Second) @@ -194,8 +193,7 @@ func TestConstructorErrors(t *testing.T) { } c, err := New(componenttest.NewNopTelemetrySettings(), apiCfg, er, ff, []Association{}, Excludes{}, clientProvider, NewFakeInformer, NewFakeNamespaceInformer, nil, false, 10*time.Second) assert.Nil(t, c) - assert.Error(t, err) - assert.Equal(t, "error creating k8s client", err.Error()) + require.EqualError(t, err, "error creating k8s client") assert.Equal(t, apiCfg, gotAPIConfig) }) } @@ -1834,9 +1832,7 @@ func Test_extractField(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := tt.args.r.extractField(tt.args.v); got != tt.want { - t.Errorf("extractField() = %v, want %v", got, tt.want) - } + assert.Equal(t, tt.want, tt.args.r.extractField(tt.args.v), "extractField()") }) } } diff --git a/processor/k8sattributesprocessor/options_test.go b/processor/k8sattributesprocessor/options_test.go index 9afe3cdee1dc..17ca2716a19f 100644 --- a/processor/k8sattributesprocessor/options_test.go +++ b/processor/k8sattributesprocessor/options_test.go @@ -20,12 +20,11 @@ func TestWithAPIConfig(t *testing.T) { p := &kubernetesprocessor{} apiConfig := k8sconfig.APIConfig{AuthType: "test-auth-type"} err := withAPIConfig(apiConfig)(p) - assert.Error(t, err) - assert.Equal(t, "invalid authType for kubernetes: test-auth-type", err.Error()) + require.EqualError(t, err, "invalid authType for kubernetes: test-auth-type") apiConfig = k8sconfig.APIConfig{AuthType: "kubeConfig"} err = withAPIConfig(apiConfig)(p) - assert.NoError(t, err) + require.NoError(t, err) assert.Equal(t, apiConfig, p.apiConfig) } @@ -200,15 +199,12 @@ func TestWithExtractAnnotations(t *testing.T) { p := &kubernetesprocessor{} opt := withExtractAnnotations(tt.args...) err := opt(p) - if tt.wantError == "" { - assert.NoError(t, err) + if tt.wantError != "" { + require.EqualError(t, err, tt.wantError) } else { - assert.Error(t, err) - assert.Equal(t, tt.wantError, err.Error()) - return + require.NoError(t, err) + assert.Equal(t, tt.want, p.rules.Annotations) } - got := p.rules.Annotations - assert.Equal(t, tt.want, got) }) } } @@ -342,14 +338,12 @@ func TestWithExtractLabels(t *testing.T) { p := &kubernetesprocessor{} opt := withExtractLabels(tt.args...) err := opt(p) - if tt.wantError == "" { - assert.NoError(t, err) + if tt.wantError != "" { + require.EqualError(t, err, tt.wantError) } else { - assert.Error(t, err) - assert.Equal(t, tt.wantError, err.Error()) - return + require.NoError(t, err) + assert.Equal(t, tt.want, p.rules.Labels) } - assert.Equal(t, tt.want, p.rules.Labels) }) } } diff --git a/processor/k8sattributesprocessor/processor_test.go b/processor/k8sattributesprocessor/processor_test.go index 875d9119bb60..b24b90419c31 100644 --- a/processor/k8sattributesprocessor/processor_test.go +++ b/processor/k8sattributesprocessor/processor_test.go @@ -271,8 +271,7 @@ func TestProcessorBadClientProvider(t *testing.T) { } newMultiTest(t, NewFactory().CreateDefaultConfig(), func(err error) { - require.Error(t, err) - assert.Equal(t, "bad client error", err.Error()) + require.EqualError(t, err, "bad client error") }, withKubeClientProvider(clientProvider)) } @@ -1520,8 +1519,7 @@ func TestRealClient(t *testing.T) { t, NewFactory().CreateDefaultConfig(), func(err error) { - require.Error(t, err) - assert.Equal(t, "unable to load k8s config, KUBERNETES_SERVICE_HOST and KUBERNETES_SERVICE_PORT must be defined", err.Error()) + require.EqualError(t, err, "unable to load k8s config, KUBERNETES_SERVICE_HOST and KUBERNETES_SERVICE_PORT must be defined") }, withKubeClientProvider(kubeClientProvider), withAPIConfig(k8sconfig.APIConfig{AuthType: "none"}), diff --git a/processor/metricstransformprocessor/factory_test.go b/processor/metricstransformprocessor/factory_test.go index cd85142c44b7..80d36b39d2cd 100644 --- a/processor/metricstransformprocessor/factory_test.go +++ b/processor/metricstransformprocessor/factory_test.go @@ -165,7 +165,7 @@ func TestFactory_validateConfiguration(t *testing.T) { }, } err := validateConfiguration(&v1) - assert.Equal(t, "operation 1: missing required field \"new_label\" while \"action\" is add_label", err.Error()) + assert.EqualError(t, err, "operation 1: missing required field \"new_label\" while \"action\" is add_label") v2 := Config{ Transforms: []transform{ @@ -186,7 +186,7 @@ func TestFactory_validateConfiguration(t *testing.T) { } err = validateConfiguration(&v2) - assert.Equal(t, "operation 1: missing required field \"new_value\" while \"action\" is add_label", err.Error()) + assert.EqualError(t, err, "operation 1: missing required field \"new_value\" while \"action\" is add_label") } func TestCreateProcessorsFilledData(t *testing.T) { diff --git a/processor/probabilisticsamplerprocessor/logsprocessor_test.go b/processor/probabilisticsamplerprocessor/logsprocessor_test.go index 0e9ab99ca71b..7f675d80a09e 100644 --- a/processor/probabilisticsamplerprocessor/logsprocessor_test.go +++ b/processor/probabilisticsamplerprocessor/logsprocessor_test.go @@ -415,7 +415,7 @@ func TestLogsSamplingState(t *testing.T) { } else { require.Len(t, observed.All(), 1, "should have one log: %v", observed.All()) require.Contains(t, observed.All()[0].Message, "logs sampler") - require.Contains(t, observed.All()[0].Context[0].Interface.(error).Error(), tt.log) + require.ErrorContains(t, observed.All()[0].Context[0].Interface.(error), tt.log) } sampledData := sink.AllLogs() @@ -513,7 +513,7 @@ func TestLogsMissingRandomness(t *testing.T) { // pct==0 bypasses the randomness check require.Len(t, observed.All(), 1, "should have one log: %v", observed.All()) require.Contains(t, observed.All()[0].Message, "logs sampler") - require.Contains(t, observed.All()[0].Context[0].Interface.(error).Error(), "missing randomness") + require.ErrorContains(t, observed.All()[0].Context[0].Interface.(error), "missing randomness") } else { require.Empty(t, observed.All(), "should have no logs: %v", observed.All()) } diff --git a/processor/probabilisticsamplerprocessor/tracesprocessor_test.go b/processor/probabilisticsamplerprocessor/tracesprocessor_test.go index 731a6ac1fe95..7c0b39d5505b 100644 --- a/processor/probabilisticsamplerprocessor/tracesprocessor_test.go +++ b/processor/probabilisticsamplerprocessor/tracesprocessor_test.go @@ -151,25 +151,20 @@ func Test_tracesamplerprocessor_SamplingPercentageRange(t *testing.T) { sink := newAssertTraces(t, testSvcName) tsp, err := newTracesProcessor(context.Background(), processortest.NewNopSettings(), tt.cfg, sink) - if err != nil { - t.Errorf("error when creating traceSamplerProcessor: %v", err) - return - } + require.NoError(t, err, "error when creating traceSamplerProcessor") for _, td := range genRandomTestData(tt.numBatches, tt.numTracesPerBatch, testSvcName, 1) { assert.NoError(t, tsp.ConsumeTraces(context.Background(), td)) } sampled := sink.spanCount actualPercentageSamplingPercentage := float32(sampled) / float32(tt.numBatches*tt.numTracesPerBatch) * 100.0 delta := math.Abs(float64(actualPercentageSamplingPercentage - tt.cfg.SamplingPercentage)) - if delta > tt.acceptableDelta { - t.Errorf( - "got %f percentage sampling rate, want %f (allowed delta is %f but got %f)", - actualPercentageSamplingPercentage, - tt.cfg.SamplingPercentage, - tt.acceptableDelta, - delta, - ) - } + assert.LessOrEqualf(t, delta, tt.acceptableDelta, + "got %f percentage sampling rate, want %f (allowed delta is %f but got %f)", + actualPercentageSamplingPercentage, + tt.cfg.SamplingPercentage, + tt.acceptableDelta, + delta, + ) }) } } @@ -212,10 +207,7 @@ func Test_tracesamplerprocessor_SamplingPercentageRange_MultipleResourceSpans(t sink := new(consumertest.TracesSink) tsp, err := newTracesProcessor(context.Background(), processortest.NewNopSettings(), tt.cfg, sink) - if err != nil { - t.Errorf("error when creating traceSamplerProcessor: %v", err) - return - } + require.NoError(t, err, "error when creating traceSamplerProcessor") for _, td := range genRandomTestData(tt.numBatches, tt.numTracesPerBatch, testSvcName, tt.resourceSpanPerTrace) { assert.NoError(t, tsp.ConsumeTraces(context.Background(), td)) @@ -285,7 +277,7 @@ func Test_tracessamplerprocessor_MissingRandomness(t *testing.T) { // pct==0 bypasses the randomness check require.Len(t, observed.All(), 1, "should have one log: %v", observed.All()) require.Contains(t, observed.All()[0].Message, "traces sampler") - require.Contains(t, observed.All()[0].Context[0].Interface.(error).Error(), "missing randomness") + require.ErrorContains(t, observed.All()[0].Context[0].Interface.(error), "missing randomness") } else { require.Empty(t, observed.All(), "should have no logs: %v", observed.All()) } @@ -905,7 +897,7 @@ func Test_tracesamplerprocessor_TraceState(t *testing.T) { } else { require.Len(t, observed.All(), 1, "should have one log: %v", observed.All()) require.Contains(t, observed.All()[0].Message, "traces sampler") - require.Contains(t, observed.All()[0].Context[0].Interface.(error).Error(), tt.log) + require.ErrorContains(t, observed.All()[0].Context[0].Interface.(error), tt.log) } }) } @@ -1026,10 +1018,10 @@ func Test_tracesamplerprocessor_TraceStateErrors(t *testing.T) { require.Len(t, observed.All(), 1, "should have one log: %v", observed.All()) if observed.All()[0].Message == "trace sampler" { - require.Contains(t, observed.All()[0].Context[0].Interface.(error).Error(), expectMessage) + require.ErrorContains(t, observed.All()[0].Context[0].Interface.(error), expectMessage) } else { require.Contains(t, observed.All()[0].Message, "traces sampler") - require.Contains(t, observed.All()[0].Context[0].Interface.(error).Error(), expectMessage) + require.ErrorContains(t, observed.All()[0].Context[0].Interface.(error), expectMessage) } }) } diff --git a/processor/tailsamplingprocessor/internal/idbatcher/id_batcher_test.go b/processor/tailsamplingprocessor/internal/idbatcher/id_batcher_test.go index d3ab8ecea585..95d12ef3c4c4 100644 --- a/processor/tailsamplingprocessor/internal/idbatcher/id_batcher_test.go +++ b/processor/tailsamplingprocessor/internal/idbatcher/id_batcher_test.go @@ -50,9 +50,7 @@ func BenchmarkConcurrentEnqueue(b *testing.B) { ids := generateSequentialIDs(1) batcher, err := New(10, 100, uint64(4*runtime.NumCPU())) defer batcher.Stop() - if err != nil { - b.Fatalf("Failed to create Batcher: %v", err) - } + require.NoError(b, err, "Failed to create Batcher") ticker := time.NewTicker(100 * time.Millisecond) defer ticker.Stop() diff --git a/processor/tailsamplingprocessor/internal/sampling/composite_test.go b/processor/tailsamplingprocessor/internal/sampling/composite_test.go index 67a977f90335..c323fe849946 100644 --- a/processor/tailsamplingprocessor/internal/sampling/composite_test.go +++ b/processor/tailsamplingprocessor/internal/sampling/composite_test.go @@ -217,9 +217,7 @@ func TestCompositeEvaluator2SubpolicyThrottling(t *testing.T) { require.NoError(t, err, "Failed to evaluate composite policy: %v", err) expected := Sampled - if decision != expected { - t.Fatalf("Incorrect decision by composite policy evaluator: expected %v, actual %v", expected, decision) - } + require.Equal(t, expected, decision, "Incorrect decision by composite policy evaluator: expected %v, actual %v", expected, decision) } // Now we hit the rate limit for second subpolicy, so subsequent evaluations should result in NotSampled @@ -228,9 +226,7 @@ func TestCompositeEvaluator2SubpolicyThrottling(t *testing.T) { require.NoError(t, err, "Failed to evaluate composite policy: %v", err) expected := NotSampled - if decision != expected { - t.Fatalf("Incorrect decision by composite policy evaluator: expected %v, actual %v", expected, decision) - } + require.Equal(t, expected, decision, "Incorrect decision by composite policy evaluator: expected %v, actual %v", expected, decision) } // Let the time advance by one second. diff --git a/processor/transformprocessor/internal/metrics/func_agregate_on_attribute_value_metrics_test.go b/processor/transformprocessor/internal/metrics/func_agregate_on_attribute_value_metrics_test.go index 0f84ee276d12..e39c3bee76ea 100644 --- a/processor/transformprocessor/internal/metrics/func_agregate_on_attribute_value_metrics_test.go +++ b/processor/transformprocessor/internal/metrics/func_agregate_on_attribute_value_metrics_test.go @@ -514,17 +514,17 @@ func Test_aggregateOnAttributeValues(t *testing.T) { func Test_createAggregateOnAttributeValueFunction(t *testing.T) { // invalid input arguments - _, e := createAggregateOnAttributeValueFunction(ottl.FunctionContext{}, nil) - require.Contains(t, e.Error(), "AggregateOnAttributeValueFactory args must be of type *AggregateOnAttributeValueArguments") + _, err := createAggregateOnAttributeValueFunction(ottl.FunctionContext{}, nil) + require.ErrorContains(t, err, "AggregateOnAttributeValueFactory args must be of type *AggregateOnAttributeValueArguments") // invalid aggregation function - _, e = createAggregateOnAttributeValueFunction(ottl.FunctionContext{}, &aggregateOnAttributeValueArguments{ + _, err = createAggregateOnAttributeValueFunction(ottl.FunctionContext{}, &aggregateOnAttributeValueArguments{ AggregationFunction: "invalid", Attribute: "attr", Values: []string{"val"}, NewValue: "newVal", }) - require.Contains(t, e.Error(), "invalid aggregation function") + require.ErrorContains(t, err, "invalid aggregation function") } func getTestSumMetricMultipleAggregateOnAttributeValue() pmetric.Metric { diff --git a/processor/transformprocessor/internal/metrics/func_convert_exponential_hist_to_explicit_hist_test.go b/processor/transformprocessor/internal/metrics/func_convert_exponential_hist_to_explicit_hist_test.go index b920c8533a30..aee2cdf07fea 100644 --- a/processor/transformprocessor/internal/metrics/func_convert_exponential_hist_to_explicit_hist_test.go +++ b/processor/transformprocessor/internal/metrics/func_convert_exponential_hist_to_explicit_hist_test.go @@ -751,8 +751,7 @@ func Test_convertExponentialHistToExplicitHist_validate(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { _, err := convertExponentialHistToExplicitHist("random", tt.sliceExplicitBoundsArgs) - assert.Error(t, err) - assert.Contains(t, err.Error(), "explicit bounds cannot be empty") + assert.ErrorContains(t, err, "explicit bounds cannot be empty") }) } }