Skip to content

Commit

Permalink
[chore]: enable error-is-as rule from testifylint (open-telemetry#34995)
Browse files Browse the repository at this point in the history
#### Description

Testifylint is a linter that provides best practices with the use of
testify.

This PR enables
[error-is-as](https://github.com/Antonboom/testifylint?tab=readme-ov-file#error-is-as)
rule from [testifylint](https://github.com/Antonboom/testifylint)

Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
  • Loading branch information
mmorel-35 committed Sep 5, 2024
1 parent 2bc5b37 commit 0055e6e
Show file tree
Hide file tree
Showing 20 changed files with 36 additions and 46 deletions.
1 change: 0 additions & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@ linters-settings:

testifylint:
disable:
- error-is-as
- expected-actual
- float-compare
- formatter
Expand Down
2 changes: 1 addition & 1 deletion Makefile.Common
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ GOTESTSUM := $(TOOLS_BIN_DIR)/gotestsum
TESTIFYLINT := $(TOOLS_BIN_DIR)/testifylint

GOTESTSUM_OPT?= --rerun-fails=1
TESTIFYLINT_OPT?= --enable-all --disable=error-is-as,expected-actual,float-compare,formatter,go-require,negative-positive,require-error,suite-dont-use-pkg,suite-subtest-run,useless-assert
TESTIFYLINT_OPT?= --enable-all --disable=expected-actual,float-compare,formatter,go-require,negative-positive,require-error,suite-dont-use-pkg,suite-subtest-run,useless-assert

# BUILD_TYPE should be one of (dev, release).
BUILD_TYPE?=release
Expand Down
6 changes: 3 additions & 3 deletions exporter/kafkaexporter/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func TestCreateMetricExporter(t *testing.T) {
name string
conf *Config
marshalers []MetricsMarshaler
err error
err *net.DNSError
}{
{
name: "valid config (no validating broker)",
Expand Down Expand Up @@ -104,7 +104,7 @@ func TestCreateLogExporter(t *testing.T) {
name string
conf *Config
marshalers []LogsMarshaler
err error
err *net.DNSError
}{
{
name: "valid config (no validating broker)",
Expand Down Expand Up @@ -169,7 +169,7 @@ func TestCreateTraceExporter(t *testing.T) {
name string
conf *Config
marshalers []TracesMarshaler
err error
err *net.DNSError
}{
{
name: "valid config (no validating brokers)",
Expand Down
2 changes: 1 addition & 1 deletion exporter/loadbalancingexporter/resolver_k8s_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ func Test_newK8sResolver(t *testing.T) {
_, tb := getTelemetryAssets(t)
got, err := newK8sResolver(fake.NewSimpleClientset(), tt.args.logger, tt.args.service, tt.args.ports, defaultListWatchTimeout, tb)
if tt.wantErr != nil {
require.Error(t, err, tt.wantErr)
require.ErrorIs(t, err, tt.wantErr)
} else {
require.NoError(t, err)
require.Equal(t, tt.wantNil, got == nil)
Expand Down
5 changes: 2 additions & 3 deletions exporter/otelarrowexporter/internal/arrow/stream_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package arrow

import (
"context"
"errors"
"fmt"
"sync"
"testing"
Expand Down Expand Up @@ -216,7 +215,7 @@ func TestStreamUnknownBatchError(t *testing.T) {
// sender should get ErrStreamRestarting
err := tc.mustSendAndWait()
require.Error(t, err)
require.True(t, errors.Is(err, ErrStreamRestarting))
require.ErrorIs(t, err, ErrStreamRestarting)
})
}
}
Expand Down Expand Up @@ -347,7 +346,7 @@ func TestStreamSendError(t *testing.T) {
// sender should get ErrStreamRestarting
err := tc.mustSendAndWait()
require.Error(t, err)
require.True(t, errors.Is(err, ErrStreamRestarting))
require.ErrorIs(t, err, ErrStreamRestarting)
})
}
}
7 changes: 3 additions & 4 deletions exporter/sumologicexporter/exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package sumologicexporter

import (
"context"
"errors"
"net/http"
"net/http/httptest"
"sync"
Expand Down Expand Up @@ -192,7 +191,7 @@ func TestAllFailed(t *testing.T) {
assert.EqualError(t, err, "failed sending data: status: 500 Internal Server Error")

var partial consumererror.Logs
require.True(t, errors.As(err, &partial))
require.ErrorAs(t, err, &partial)
assert.Equal(t, logsExpected, partial.Data())
}

Expand Down Expand Up @@ -231,7 +230,7 @@ func TestPartiallyFailed(t *testing.T) {
assert.EqualError(t, err, "failed sending data: status: 500 Internal Server Error")

var partial consumererror.Logs
require.True(t, errors.As(err, &partial))
require.ErrorAs(t, err, &partial)
assert.Equal(t, logsExpected, partial.Data())
}

Expand Down Expand Up @@ -462,7 +461,7 @@ gauge_metric_name{foo="bar",remote_name="156955",url="http://another_url"} 245 1
assert.EqualError(t, err, tc.expectedError)

var partial consumererror.Metrics
require.True(t, errors.As(err, &partial))
require.ErrorAs(t, err, &partial)
// TODO fix
// assert.Equal(t, metrics, partial.GetMetrics())
})
Expand Down
2 changes: 1 addition & 1 deletion extension/basicauthextension/extension_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ func TestBasicAuth_HtpasswdInlinePrecedence(t *testing.T) {
auth = base64.StdEncoding.EncodeToString([]byte("username:fromfile"))

_, err = ext.Authenticate(context.Background(), map[string][]string{"authorization": {"Basic " + auth}})
assert.Error(t, errInvalidCredentials, err)
assert.ErrorIs(t, errInvalidCredentials, err)
}

func TestBasicAuth_SupportedHeaders(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion extension/headerssetterextension/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func TestLoadConfig(t *testing.T) {
require.NoError(t, sub.Unmarshal(cfg))

if tt.expectedError != nil {
assert.Error(t, component.ValidateConfig(cfg), tt.expectedError)
assert.ErrorIs(t, component.ValidateConfig(cfg), tt.expectedError)
return
}
assert.NoError(t, component.ValidateConfig(cfg))
Expand Down
8 changes: 4 additions & 4 deletions extension/oauth2clientauthextension/extension_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func TestOAuthClientSettingsCredsConfig(t *testing.T) {
settings *Config
expectedClientConfig *clientcredentials.Config
shouldError bool
expectedError *error
expectedError error
}{
{
name: "client_id_file",
Expand Down Expand Up @@ -151,7 +151,7 @@ func TestOAuthClientSettingsCredsConfig(t *testing.T) {
ClientSecret: "testsecret",
},
shouldError: true,
expectedError: &errNoClientIDProvided,
expectedError: errNoClientIDProvided,
},
{
name: "missing_client_creds_file",
Expand All @@ -160,7 +160,7 @@ func TestOAuthClientSettingsCredsConfig(t *testing.T) {
ClientSecretFile: testCredsMissingFile,
},
shouldError: true,
expectedError: &errNoClientSecretProvided,
expectedError: errNoClientSecretProvided,
},
}

Expand All @@ -170,7 +170,7 @@ func TestOAuthClientSettingsCredsConfig(t *testing.T) {
cfg, err := rc.clientCredentials.createConfig()
if test.shouldError {
assert.Error(t, err)
assert.ErrorAs(t, err, test.expectedError)
assert.ErrorIs(t, err, test.expectedError)
return
}
assert.NoError(t, err)
Expand Down
5 changes: 2 additions & 3 deletions extension/observer/ecsobserver/exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
package ecsobserver

import (
"errors"
"testing"

"github.com/aws/aws-sdk-go/aws"
Expand All @@ -27,7 +26,7 @@ func TestTaskExporter(t *testing.T) {
})
assert.Error(t, err)
v := &errPrivateIPNotFound{}
assert.True(t, errors.As(err, &v))
assert.ErrorAs(t, err, &v)
})

awsVpcTask := &ecs.Task{
Expand Down Expand Up @@ -118,7 +117,7 @@ func TestTaskExporter(t *testing.T) {
merr := multierr.Errors(err)
require.Len(t, merr, 1)
v := &errMappedPortNotFound{}
assert.True(t, errors.As(merr[0], &v))
assert.ErrorAs(t, merr[0], &v)
assert.Len(t, targets, 2)
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package ecsmock

import (
"context"
"errors"
"fmt"
"testing"

Expand All @@ -28,7 +27,7 @@ func TestCluster_ListTasksWithContext(t *testing.T) {
_, err := c.ListTasksWithContext(ctx, req)
require.Error(t, err)
var aerr awserr.Error
assert.True(t, errors.As(err, &aerr))
assert.ErrorAs(t, err, &aerr)
assert.Equal(t, ecs.ErrCodeClusterNotFoundException, aerr.Code())
assert.Equal(t, "code "+ecs.ErrCodeClusterNotFoundException+" message "+aerr.Message(), aerr.Error())
assert.NoError(t, aerr.OrigErr())
Expand Down
4 changes: 2 additions & 2 deletions internal/sqlquery/db_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func TestDBSQLClient_Nulls(t *testing.T) {
}
rows, err := cl.QueryRows(context.Background())
assert.Error(t, err)
assert.True(t, errors.Is(err, ErrNullValueWarning))
assert.ErrorIs(t, err, ErrNullValueWarning)
assert.Len(t, rows, 1)
assert.EqualValues(t, map[string]string{
"col_0": "42",
Expand All @@ -96,7 +96,7 @@ func TestDBSQLClient_Nulls_MultiRow(t *testing.T) {
assert.Len(t, uw, 2)

for _, err := range uw {
assert.True(t, errors.Is(err, ErrNullValueWarning))
assert.ErrorIs(t, err, ErrNullValueWarning)
}
}
assert.Len(t, rows, 2)
Expand Down
5 changes: 2 additions & 3 deletions pkg/sampling/encoding_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package sampling

import (
"encoding/binary"
"errors"
"fmt"
"math/rand"
"strconv"
Expand Down Expand Up @@ -187,7 +186,7 @@ func TestRValueSyntax(t *testing.T) {
rnd, err := RValueToRandomness(test.in)

if test.expectErr != nil {
require.True(t, errors.Is(err, test.expectErr),
require.ErrorIs(t, err, test.expectErr,
"%q: not expecting %v wanted %v", test.in, err, test.expectErr,
)
require.Equal(t, must(RValueToRandomness("00000000000000")), rnd)
Expand Down Expand Up @@ -241,7 +240,7 @@ func TestTValueSyntax(t *testing.T) {
_, err := TValueToThreshold(test.in)

if test.expectErr != nil {
require.True(t, errors.Is(err, test.expectErr),
require.ErrorIs(t, err, test.expectErr,
"%q: not expecting %v wanted %v", test.in, err, test.expectErr,
)
} else {
Expand Down
3 changes: 1 addition & 2 deletions pkg/sampling/oteltracestate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
package sampling

import (
"errors"
"fmt"
"strconv"
"strings"
Expand Down Expand Up @@ -233,7 +232,7 @@ func TestParseOpenTelemetryTraceState(t *testing.T) {
otts, err := NewOpenTelemetryTraceState(test.in)

if test.expectErr != nil {
require.True(t, errors.Is(err, test.expectErr), "%q: not expecting %v wanted %v", test.in, err, test.expectErr)
require.ErrorIs(t, err, test.expectErr, "%q: not expecting %v wanted %v", test.in, err, test.expectErr)
} else {
require.NoError(t, err)
}
Expand Down
3 changes: 1 addition & 2 deletions pkg/sampling/w3ctracestate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
package sampling

import (
"errors"
"fmt"
"strconv"
"strings"
Expand Down Expand Up @@ -116,7 +115,7 @@ func TestParseW3CTraceState(t *testing.T) {
w3c, err := NewW3CTraceState(test.in)

if test.expectErr != nil {
require.True(t, errors.Is(err, test.expectErr),
require.ErrorIs(t, err, test.expectErr,
"%q: not expecting %v wanted %v", test.in, err, test.expectErr,
)
} else {
Expand Down
2 changes: 1 addition & 1 deletion processor/groupbytraceprocessor/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func TestCreateTestProcessorWithNotImplementedOptions(t *testing.T) {
p, err := f.CreateTracesProcessor(context.Background(), processortest.NewNopSettings(), tt.config, consumertest.NewNop())

// verify
assert.Error(t, tt.expectedErr, err)
assert.ErrorIs(t, tt.expectedErr, err)
assert.Nil(t, p)
}
}
8 changes: 4 additions & 4 deletions processor/groupbytraceprocessor/processor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ func TestTraceErrorFromStorageWhileReleasing(t *testing.T) {
err = p.markAsReleased(traceID, p.eventMachine.workers[workerIndexForTraceID(traceID, config.NumWorkers)].fire)

// verify
assert.True(t, errors.Is(err, expectedError))
assert.ErrorIs(t, err, expectedError)
}

func TestTraceErrorFromStorageWhileProcessingTrace(t *testing.T) {
Expand Down Expand Up @@ -290,7 +290,7 @@ func TestTraceErrorFromStorageWhileProcessingTrace(t *testing.T) {
err := p.onTraceReceived(tracesWithID{id: traceID, td: batch[0]}, p.eventMachine.workers[0])

// verify
assert.True(t, errors.Is(err, expectedError))
assert.ErrorIs(t, err, expectedError)
}

func TestAddSpansToExistingTrace(t *testing.T) {
Expand Down Expand Up @@ -385,7 +385,7 @@ func TestTraceErrorFromStorageWhileProcessingSecondTrace(t *testing.T) {
)

// verify
assert.True(t, errors.Is(err, expectedError))
assert.ErrorIs(t, err, expectedError)
}

func TestErrorFromStorageWhileRemovingTrace(t *testing.T) {
Expand All @@ -412,7 +412,7 @@ func TestErrorFromStorageWhileRemovingTrace(t *testing.T) {
err := p.onTraceRemoved(traceID)

// verify
assert.True(t, errors.Is(err, expectedError))
assert.ErrorIs(t, err, expectedError)
}

func TestTraceNotFoundWhileRemovingTrace(t *testing.T) {
Expand Down
9 changes: 4 additions & 5 deletions receiver/awsxrayreceiver/internal/tracesegment/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
package tracesegment

import (
"errors"
"fmt"
"testing"

Expand Down Expand Up @@ -31,7 +30,7 @@ func TestSplitHeaderBodyWithSeparatorDoesNotExist(t *testing.T) {
_, _, err := SplitHeaderBody(buf)

var errRecv *recvErr.ErrRecoverable
assert.True(t, errors.As(err, &errRecv), "should return recoverable error")
assert.ErrorAs(t, err, &errRecv, "should return recoverable error")
assert.EqualError(t, err,
fmt.Sprintf("unable to split incoming data as header and segment, incoming bytes: %v", buf),
"expected error messages")
Expand All @@ -41,7 +40,7 @@ func TestSplitHeaderBodyNilBuf(t *testing.T) {
_, _, err := SplitHeaderBody(nil)

var errRecv *recvErr.ErrRecoverable
assert.True(t, errors.As(err, &errRecv), "should return recoverable error")
assert.ErrorAs(t, err, &errRecv, "should return recoverable error")
assert.EqualError(t, err, "buffer to split is nil",
"expected error messages")
}
Expand All @@ -52,7 +51,7 @@ func TestSplitHeaderBodyNonJsonHeader(t *testing.T) {
_, _, err := SplitHeaderBody(buf)

var errRecv *recvErr.ErrRecoverable
assert.True(t, errors.As(err, &errRecv), "should return recoverable error")
assert.ErrorAs(t, err, &errRecv, "should return recoverable error")
assert.Contains(t, err.Error(), "invalid character 'o'")
}

Expand All @@ -76,7 +75,7 @@ func TestSplitHeaderBodyInvalidJsonHeader(t *testing.T) {
assert.Error(t, err, "should fail because version is invalid")

var errRecv *recvErr.ErrRecoverable
assert.True(t, errors.As(err, &errRecv), "should return recoverable error")
assert.ErrorAs(t, err, &errRecv, "should return recoverable error")
assert.Contains(t, err.Error(),
fmt.Sprintf("invalid header %+v", Header{
Format: "json",
Expand Down
2 changes: 1 addition & 1 deletion receiver/couchdbreceiver/scraper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func TestScrape(t *testing.T) {
assert.Equal(t, 0, metrics.DataPointCount(), "Expected 0 datapoints to be collected")

var partialScrapeErr scrapererror.PartialScrapeError
require.True(t, errors.As(err, &partialScrapeErr), "returned error was not PartialScrapeError")
require.ErrorAs(t, err, &partialScrapeErr, "returned error was not PartialScrapeError")
require.Greater(t, partialScrapeErr.Failed, 0, "Expected scrape failures, but none were recorded!")
})

Expand Down
Loading

0 comments on commit 0055e6e

Please sign in to comment.