Skip to content

Commit

Permalink
testifylint: enable require-error (jaegertracing#5034)
Browse files Browse the repository at this point in the history
## Which problem is this PR solving?
- Apply require-error rule of testifylint

## Description of the changes
-  use require instead of assert on errors testing 

## How was this change tested?
- CI 

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [ ] I have added unit tests for the new functionality
- [ ] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
  • Loading branch information
mmorel-35 committed Dec 24, 2023
1 parent cf917dd commit 7d1cc9f
Show file tree
Hide file tree
Showing 157 changed files with 900 additions and 842 deletions.
2 changes: 1 addition & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -141,14 +141,14 @@ linters-settings:
- compares
- float-compare
- go-require
- require-error
enable:
- bool-compare
- empty
- error-is-as
- error-nil
- expected-actual
- len
- require-error
- suite-dont-use-pkg
- suite-extra-assert-call

Expand Down
8 changes: 4 additions & 4 deletions cmd/agent/app/agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func TestAgentStartError(t *testing.T) {
agent, err := cfg.CreateAgent(fakeCollectorProxy{}, zap.NewNop(), metrics.NullFactory)
require.NoError(t, err)
agent.httpServer.Addr = "bad-address"
assert.Error(t, agent.Run())
require.Error(t, agent.Run())
}

func TestAgentSamplingEndpoint(t *testing.T) {
Expand Down Expand Up @@ -67,7 +67,7 @@ func TestAgentSamplingEndpoint(t *testing.T) {
resp, err := http.Get(url)
require.NoError(t, err)
body, err := io.ReadAll(resp.Body)
assert.NoError(t, err)
require.NoError(t, err)
assert.Equal(t, "collector error: no peers available\n", string(body))
})
}
Expand All @@ -78,7 +78,7 @@ func TestAgentMetricsEndpoint(t *testing.T) {
resp, err := http.Get(url)
require.NoError(t, err)
body, err := io.ReadAll(resp.Body)
assert.NoError(t, err)
require.NoError(t, err)
assert.Contains(t, string(body), "# HELP")
})
}
Expand Down Expand Up @@ -129,7 +129,7 @@ func withRunningAgent(t *testing.T, testcase func(string, chan error)) {
testcase(agent.HTTPAddr(), ch)

agent.Stop()
assert.NoError(t, <-ch)
require.NoError(t, <-ch)

for i := 0; i < 1000; i++ {
if strings.Contains(logBuf.String(), "agent's http server exiting") {
Expand Down
10 changes: 5 additions & 5 deletions cmd/agent/app/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ func TestBuilderFromConfig(t *testing.T) {
func TestBuilderWithExtraReporter(t *testing.T) {
cfg := &Builder{}
agent, err := cfg.CreateAgent(fakeCollectorProxy{}, zap.NewNop(), metrics.NullFactory)
assert.NoError(t, err)
require.NoError(t, err)
assert.NotNil(t, agent)
}

Expand Down Expand Up @@ -159,7 +159,7 @@ func TestBuilderWithProcessorErrors(t *testing.T) {
},
}
_, err := cfg.CreateAgent(&fakeCollectorProxy{}, zap.NewNop(), metrics.NullFactory)
assert.Error(t, err)
require.Error(t, err)
if testCase.err != "" {
assert.Contains(t, err.Error(), testCase.err)
} else if testCase.errContains != "" {
Expand Down Expand Up @@ -262,7 +262,7 @@ func TestCreateCollectorProxy(t *testing.T) {
Logger: zap.NewNop(),
}, builders)
if test.err != "" {
assert.EqualError(t, err, test.err)
require.EqualError(t, err, test.err)
assert.Nil(t, proxy)
} else {
require.NoError(t, err)
Expand All @@ -280,7 +280,7 @@ func TestCreateCollectorProxy_UnknownReporter(t *testing.T) {
}
proxy, err := CreateCollectorProxy(ProxyBuilderOptions{}, builders)
assert.Nil(t, proxy)
assert.EqualError(t, err, "unknown reporter type ")
require.EqualError(t, err, "unknown reporter type ")
}

func TestPublishOpts(t *testing.T) {
Expand All @@ -305,7 +305,7 @@ func TestPublishOpts(t *testing.T) {
forkFactory := metricstest.NewFactory(time.Second)
metricsFactory := fork.New("internal", forkFactory, baseMetrics)
agent, err := cfg.CreateAgent(fakeCollectorProxy{}, zap.NewNop(), metricsFactory)
assert.NoError(t, err)
require.NoError(t, err)
assert.NotNil(t, agent)

forkFactory.AssertGaugeMetrics(t, metricstest.ExpectedMetric{
Expand Down
2 changes: 1 addition & 1 deletion cmd/agent/app/configmanager/grpc/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func TestSamplingManager_GetBaggageRestrictions(t *testing.T) {
manager := NewConfigManager(nil)
rest, err := manager.GetBaggageRestrictions(context.Background(), "foo")
require.Nil(t, rest)
assert.EqualError(t, err, "baggage not implemented")
require.EqualError(t, err, "baggage not implemented")
}

type mockSamplingHandler struct{}
Expand Down
5 changes: 2 additions & 3 deletions cmd/agent/app/configmanager/metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/goleak"

Expand Down Expand Up @@ -73,10 +72,10 @@ func TestMetrics(t *testing.T) {
if test.err != nil {
s, err := mgr.GetSamplingStrategy(context.Background(), test.err.Error())
require.Nil(t, s)
assert.EqualError(t, err, test.err.Error())
require.EqualError(t, err, test.err.Error())
b, err := mgr.GetBaggageRestrictions(context.Background(), test.err.Error())
require.Nil(t, b)
assert.EqualError(t, err, test.err.Error())
require.EqualError(t, err, test.err.Error())
} else {
s, err := mgr.GetSamplingStrategy(context.Background(), "")
require.NoError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion cmd/agent/app/processors/thrift_processor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func initCollectorAndReporter(t *testing.T) (*metricstest.Factory, *testutils.Gr

func TestNewThriftProcessor_ZeroCount(t *testing.T) {
_, err := NewThriftProcessor(nil, 0, nil, nil, nil, zaptest.NewLogger(t))
assert.EqualError(t, err, "number of processors must be greater than 0, called with 0")
require.EqualError(t, err, "number of processors must be greater than 0, called with 0")
}

func TestProcessorWithCompactZipkin(t *testing.T) {
Expand Down
6 changes: 3 additions & 3 deletions cmd/agent/app/reporter/client_metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func testClientMetricsWithParams(params ClientMetricsReporterParams, fn func(tr

func TestClientMetricsReporter_Zipkin(t *testing.T) {
testClientMetrics(func(tr *clientMetricsTest) {
assert.NoError(t, tr.r.EmitZipkinBatch(context.Background(), []*zipkincore.Span{{}}))
require.NoError(t, tr.r.EmitZipkinBatch(context.Background(), []*zipkincore.Span{{}}))
assert.Len(t, tr.mr.ZipkinSpans(), 1)
})
}
Expand Down Expand Up @@ -181,7 +181,7 @@ func TestClientMetricsReporter_Jaeger(t *testing.T) {
}

err := tr.r.EmitBatch(context.Background(), batch)
assert.NoError(t, err)
require.NoError(t, err)
assert.Len(t, tr.mr.Spans(), i+1)

tr.assertLog(t, "new client", test.expLog)
Expand Down Expand Up @@ -248,7 +248,7 @@ func TestClientMetricsReporter_Expire(t *testing.T) {
assert.EqualValues(t, 0, getGauge(), "start with gauge=0")

err := tr.r.EmitBatch(context.Background(), batch)
assert.NoError(t, err)
require.NoError(t, err)
assert.Len(t, tr.mr.Spans(), 1)

// we want this test to pass asap, but need to account for possible CPU contention in the CI
Expand Down
6 changes: 3 additions & 3 deletions cmd/agent/app/reporter/grpc/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,8 @@ func TestProxyBuilder(t *testing.T) {
assert.NotNil(t, proxy.GetReporter())
assert.NotNil(t, proxy.GetManager())

assert.NoError(t, proxy.Close())
assert.EqualError(t, proxy.Close(), "rpc error: code = Canceled desc = grpc: the client connection is closing")
require.NoError(t, proxy.Close())
require.EqualError(t, proxy.Close(), "rpc error: code = Canceled desc = grpc: the client connection is closing")
}
})
}
Expand Down Expand Up @@ -423,5 +423,5 @@ func TestBuilderWithAdditionalDialOptions(t *testing.T) {
assert.NotNil(t, r)

err = r.Invoke(context.Background(), "test", map[string]string{}, map[string]string{}, []grpc.CallOption{}...)
assert.Error(t, err, "should error because no server is running")
require.Error(t, err, "should error because no server is running")
}
4 changes: 2 additions & 2 deletions cmd/agent/app/reporter/grpc/reporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func TestReporter_EmitZipkinBatch(t *testing.T) {
for _, test := range tests {
err = rep.EmitZipkinBatch(context.Background(), []*zipkincore.Span{test.in})
if test.err != "" {
assert.EqualError(t, err, test.err)
require.EqualError(t, err, test.err)
} else {
assert.Len(t, handler.requests, 1)
assert.Equal(t, test.expected, handler.requests[0].GetBatch())
Expand Down Expand Up @@ -122,7 +122,7 @@ func TestReporter_EmitBatch(t *testing.T) {
for _, test := range tests {
err = rep.EmitBatch(context.Background(), test.in)
if test.err != "" {
assert.EqualError(t, err, test.err)
require.EqualError(t, err, test.err)
} else {
assert.Len(t, handler.requests, 1)
assert.Equal(t, test.expected, handler.requests[0].GetBatch())
Expand Down
9 changes: 5 additions & 4 deletions cmd/agent/app/reporter/reporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/jaegertracing/jaeger/cmd/agent/app/testutils"
"github.com/jaegertracing/jaeger/thrift-gen/jaeger"
Expand All @@ -39,8 +40,8 @@ func TestMultiReporter(t *testing.T) {
{},
},
})
assert.NoError(t, e1)
assert.NoError(t, e2)
require.NoError(t, e1)
require.NoError(t, e2)
assert.Len(t, r1.ZipkinSpans(), 1)
assert.Len(t, r1.Spans(), 1)
assert.Len(t, r2.ZipkinSpans(), 1)
Expand All @@ -60,8 +61,8 @@ func TestMultiReporterErrors(t *testing.T) {
{},
},
})
assert.EqualError(t, e1, fmt.Sprintf("%s\n%s", errMsg, errMsg))
assert.EqualError(t, e2, fmt.Sprintf("%s\n%s", errMsg, errMsg))
require.EqualError(t, e1, fmt.Sprintf("%s\n%s", errMsg, errMsg))
require.EqualError(t, e2, fmt.Sprintf("%s\n%s", errMsg, errMsg))
}

type mockReporter struct {
Expand Down
3 changes: 2 additions & 1 deletion cmd/agent/app/servers/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/goleak"
)

Expand All @@ -34,7 +35,7 @@ func TestReadBuf_Read(t *testing.T) {
b := &ReadBuf{bytes: []byte("hello"), n: 5}
r := make([]byte, 5)
n, err := b.Read(r)
assert.NoError(t, err)
require.NoError(t, err)
assert.Equal(t, 5, n)
assert.Equal(t, "hello", string(r))
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/agent/app/servers/thriftudp/transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ func withLocalServer(t *testing.T, f func(addr string)) {

func TestCreateClient(t *testing.T) {
_, err := createClient(nil, nil)
assert.EqualError(t, err, "dial udp: missing address")
require.EqualError(t, err, "dial udp: missing address")
}

func TestMain(m *testing.M) {
Expand Down
5 changes: 3 additions & 2 deletions cmd/agent/app/testutils/in_memory_reporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/jaegertracing/jaeger/thrift-gen/jaeger"
"github.com/jaegertracing/jaeger/thrift-gen/zipkincore"
Expand All @@ -35,8 +36,8 @@ func TestInMemoryReporter(t *testing.T) {
{},
},
})
assert.NoError(t, e1)
assert.NoError(t, e2)
require.NoError(t, e1)
require.NoError(t, e2)
assert.Len(t, r.ZipkinSpans(), 1)
assert.Len(t, r.Spans(), 1)
}
5 changes: 2 additions & 3 deletions cmd/agent/app/testutils/thriftudp_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,12 @@ import (
"testing"

"github.com/apache/thrift/lib/go/thrift"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestNewZipkinThriftUDPClient(t *testing.T) {
_, _, err := NewZipkinThriftUDPClient("256.2.3:0")
assert.Error(t, err)
require.Error(t, err)

_, cl, err := NewZipkinThriftUDPClient("127.0.0.1:12345")
require.NoError(t, err)
Expand All @@ -36,7 +35,7 @@ func TestNewJaegerThriftUDPClient(t *testing.T) {
compactFactory := thrift.NewTCompactProtocolFactoryConf(&thrift.TConfiguration{})

_, _, err := NewJaegerThriftUDPClient("256.2.3:0", compactFactory)
assert.Error(t, err)
require.Error(t, err)

_, cl, err := NewJaegerThriftUDPClient("127.0.0.1:12345", compactFactory)
require.NoError(t, err)
Expand Down
5 changes: 3 additions & 2 deletions cmd/anonymizer/app/anonymizer/anonymizer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/zap"

"github.com/jaegertracing/jaeger/model"
Expand Down Expand Up @@ -81,7 +82,7 @@ func TestNew(t *testing.T) {
tempDir := t.TempDir()

file, err := os.CreateTemp(tempDir, "mapping.json")
assert.NoError(t, err)
require.NoError(t, err)

_, err = file.Write([]byte(`
{
Expand All @@ -93,7 +94,7 @@ func TestNew(t *testing.T) {
}
}
`))
assert.NoError(t, err)
require.NoError(t, err)

anonymizer := New(file.Name(), Options{}, nopLogger)
assert.NotNil(t, anonymizer)
Expand Down
6 changes: 3 additions & 3 deletions cmd/collector/app/collector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func TestNewCollector(t *testing.T) {
collectorOpts := optionsForEphemeralPorts()
require.NoError(t, c.Start(collectorOpts))
assert.NotNil(t, c.SpanHandlers())
assert.NoError(t, c.Close())
require.NoError(t, c.Close())
}

func TestCollector_StartErrors(t *testing.T) {
Expand Down Expand Up @@ -208,8 +208,8 @@ func TestAggregator(t *testing.T) {
},
}
_, err := c.spanProcessor.ProcessSpans(spans, processor.SpansOptions{SpanFormat: processor.JaegerSpanFormat})
assert.NoError(t, err)
assert.NoError(t, c.Close())
require.NoError(t, err)
require.NoError(t, c.Close())

// spans are processed by background workers, so we may need to wait
for i := 0; i < 1000; i++ {
Expand Down
2 changes: 1 addition & 1 deletion cmd/collector/app/handler/grpc_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ func TestBatchConsumer(t *testing.T) {
{OperationName: "test-op", Process: &model.Process{ServiceName: "foo"}},
},
})
assert.NoError(t, err)
require.NoError(t, err)
assert.Equal(t, tc.transport, processor.getTransport())
assert.Equal(t, tc.expectedSpanFormat, processor.getSpanFormat())
})
Expand Down
Loading

0 comments on commit 7d1cc9f

Please sign in to comment.