Skip to content

Commit a867f49

Browse files
ansidevansidev
committed
test: fix failed unit tests
1 parent c1aa46e commit a867f49

File tree

5 files changed

+181
-124
lines changed

5 files changed

+181
-124
lines changed

‎common/blobstore/retryable_client_test.go‎

Lines changed: 160 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -28,115 +28,174 @@ import (
2828
"testing"
2929

3030
"github.com/stretchr/testify/assert"
31-
"github.com/stretchr/testify/mock"
31+
"go.uber.org/mock/gomock"
3232

3333
"github.com/uber/cadence/common/backoff"
3434
)
3535

36-
func TestRetryableClient_Put(t *testing.T) {
37-
mockClient := new(MockClient)
38-
policy := backoff.NewExponentialRetryPolicy(0)
39-
client := NewRetryableClient(mockClient, policy)
40-
41-
req := &PutRequest{}
42-
resp := &PutResponse{}
43-
mockClient.On("Put", mock.Anything, req).Return(resp, nil).Once()
44-
45-
result, err := client.Put(context.Background(), req)
46-
assert.NoError(t, err)
47-
assert.Equal(t, resp, result)
48-
49-
mockClient.AssertExpectations(t)
50-
}
51-
52-
func TestRetryableClient_Get(t *testing.T) {
53-
mockClient := new(MockClient)
54-
policy := backoff.NewExponentialRetryPolicy(0)
55-
client := NewRetryableClient(mockClient, policy)
56-
57-
req := &GetRequest{}
58-
resp := &GetResponse{}
59-
mockClient.On("Get", mock.Anything, req).Return(resp, nil).Once()
60-
61-
result, err := client.Get(context.Background(), req)
62-
assert.NoError(t, err)
63-
assert.Equal(t, resp, result)
64-
65-
mockClient.AssertExpectations(t)
66-
}
67-
68-
func TestRetryableClient_Exists(t *testing.T) {
69-
mockClient := new(MockClient)
70-
policy := backoff.NewExponentialRetryPolicy(0)
71-
client := NewRetryableClient(mockClient, policy)
72-
73-
req := &ExistsRequest{}
74-
resp := &ExistsResponse{}
75-
mockClient.On("Exists", mock.Anything, req).Return(resp, nil).Once()
76-
77-
result, err := client.Exists(context.Background(), req)
78-
assert.NoError(t, err)
79-
assert.Equal(t, resp, result)
80-
81-
mockClient.AssertExpectations(t)
82-
}
83-
84-
func TestRetryableClient_Delete(t *testing.T) {
85-
mockClient := new(MockClient)
86-
policy := backoff.NewExponentialRetryPolicy(0)
87-
client := NewRetryableClient(mockClient, policy)
88-
89-
req := &DeleteRequest{}
90-
resp := &DeleteResponse{}
91-
mockClient.On("Delete", mock.Anything, req).Return(resp, nil).Once()
92-
93-
result, err := client.Delete(context.Background(), req)
94-
assert.NoError(t, err)
95-
assert.Equal(t, resp, result)
96-
97-
mockClient.AssertExpectations(t)
98-
}
99-
100-
func TestRetryableClient_RetryOnError(t *testing.T) {
101-
mockClient := new(MockClient)
102-
policy := backoff.NewExponentialRetryPolicy(1) // Adjusting the retry interval to ensure retry is attempted
103-
client := NewRetryableClient(mockClient, policy)
104-
105-
req := &PutRequest{}
106-
resp := &PutResponse{}
107-
retryableError := errors.New("retryable error")
108-
109-
mockClient.On("Put", mock.Anything, req).Return(nil, retryableError).Once()
110-
mockClient.On("IsRetryableError", retryableError).Return(true).Once()
111-
mockClient.On("Put", mock.Anything, req).Return(resp, nil).Once()
112-
113-
result, err := client.Put(context.Background(), req)
114-
assert.NoError(t, err, "Expected no error on successful retry")
115-
assert.Equal(t, resp, result, "Expected the response to match")
36+
func runCRUDTest(
37+
t *testing.T,
38+
retryPolicy backoff.RetryPolicy,
39+
retryableError bool,
40+
req, resp any,
41+
expectFn func(*MockClient, any, any),
42+
callFn func(Client, context.Context, any) (any, error),
43+
assertFn func(*testing.T, any, any, any, error),
44+
) {
45+
mockClient := NewMockClient(gomock.NewController(t))
46+
throttleRetryOptions := []backoff.ThrottleRetryOption{
47+
backoff.WithRetryPolicy(retryPolicy),
48+
}
49+
if retryableError {
50+
throttleRetryOptions = append(throttleRetryOptions, backoff.WithRetryableError(mockClient.IsRetryableError))
51+
}
52+
client := &retryableClient{
53+
client: mockClient,
54+
throttleRetry: backoff.NewThrottleRetry(throttleRetryOptions...),
55+
}
11656

117-
mockClient.AssertExpectations(t)
57+
expectFn(mockClient, req, resp)
58+
result, err := callFn(client, context.Background(), req)
59+
assertFn(t, req, resp, result, err)
11860
}
11961

120-
func TestRetryableClient_NotRetryOnError(t *testing.T) {
121-
mockClient := new(MockClient)
122-
policy := backoff.NewExponentialRetryPolicy(0)
123-
client := NewRetryableClient(mockClient, policy)
124-
125-
req := &PutRequest{}
126-
nonRetryableError := errors.New("non-retryable error")
127-
128-
mockClient.On("Put", mock.Anything, req).Return(nil, nonRetryableError).Once()
129-
mockClient.On("IsRetryableError", nonRetryableError).Return(false).Once()
130-
131-
result, err := client.Put(context.Background(), req)
132-
assert.Error(t, err)
133-
assert.Nil(t, result)
62+
func TestRetryableClient(t *testing.T) {
63+
tests := []struct {
64+
name string
65+
retryPolicy backoff.RetryPolicy
66+
retryableError bool
67+
req any
68+
resp any
69+
expectFn func(*MockClient, any, any)
70+
callFn func(Client, context.Context, any) (any, error)
71+
assertFn func(*testing.T, any, any, any, error)
72+
}{
73+
{
74+
name: "Put",
75+
retryPolicy: backoff.NewExponentialRetryPolicy(0),
76+
retryableError: false,
77+
req: &PutRequest{},
78+
resp: &PutResponse{},
79+
expectFn: func(m *MockClient, req, resp any) {
80+
m.EXPECT().Put(gomock.Any(), req.(*PutRequest)).Return(resp.(*PutResponse), nil).Times(1)
81+
},
82+
callFn: func(c Client, ctx context.Context, req any) (any, error) {
83+
return c.Put(ctx, req.(*PutRequest))
84+
},
85+
assertFn: func(t *testing.T, req any, resp any, result any, err error) {
86+
assert.NoError(t, err)
87+
assert.Equal(t, resp.(*PutResponse), result)
88+
},
89+
},
90+
{
91+
name: "Get",
92+
retryPolicy: backoff.NewExponentialRetryPolicy(0),
93+
retryableError: false,
94+
req: &GetRequest{},
95+
resp: &GetResponse{},
96+
expectFn: func(m *MockClient, req, resp any) {
97+
m.EXPECT().Get(gomock.Any(), req.(*GetRequest)).Return(resp.(*GetResponse), nil).Times(1)
98+
},
99+
callFn: func(c Client, ctx context.Context, req any) (any, error) {
100+
return c.Get(ctx, req.(*GetRequest))
101+
},
102+
assertFn: func(t *testing.T, req any, resp any, result any, err error) {
103+
assert.NoError(t, err)
104+
assert.Equal(t, resp.(*GetResponse), result)
105+
},
106+
},
107+
{
108+
name: "Exists",
109+
retryPolicy: backoff.NewExponentialRetryPolicy(0),
110+
retryableError: false,
111+
req: &ExistsRequest{},
112+
resp: &ExistsResponse{},
113+
expectFn: func(m *MockClient, req, resp any) {
114+
m.EXPECT().Exists(gomock.Any(), req.(*ExistsRequest)).Return(resp.(*ExistsResponse), nil).Times(1)
115+
},
116+
callFn: func(c Client, ctx context.Context, req any) (any, error) {
117+
return c.Exists(ctx, req.(*ExistsRequest))
118+
},
119+
assertFn: func(t *testing.T, req any, resp any, result any, err error) {
120+
assert.NoError(t, err)
121+
assert.Equal(t, resp.(*ExistsResponse), result)
122+
},
123+
},
124+
{
125+
name: "Delete",
126+
retryPolicy: backoff.NewExponentialRetryPolicy(0),
127+
retryableError: false,
128+
req: &DeleteRequest{},
129+
resp: &DeleteResponse{},
130+
expectFn: func(m *MockClient, req, resp any) {
131+
m.EXPECT().Delete(gomock.Any(), req.(*DeleteRequest)).Return(resp.(*DeleteResponse), nil).Times(1)
132+
},
133+
callFn: func(c Client, ctx context.Context, req any) (any, error) {
134+
return c.Delete(ctx, req.(*DeleteRequest))
135+
},
136+
assertFn: func(t *testing.T, req any, resp any, result any, err error) {
137+
assert.NoError(t, err)
138+
assert.Equal(t, resp.(*DeleteResponse), result)
139+
},
140+
},
141+
{
142+
name: "RetryOnError",
143+
retryPolicy: backoff.NewExponentialRetryPolicy(1),
144+
retryableError: true,
145+
req: &PutRequest{},
146+
resp: &PutResponse{},
147+
expectFn: func(m *MockClient, req, resp any) {
148+
retryableError := errors.New("retryable error")
149+
m.EXPECT().Put(gomock.Any(), req.(*PutRequest)).Return(nil, retryableError).Times(1)
150+
m.EXPECT().IsRetryableError(retryableError).Return(true).Times(1)
151+
m.EXPECT().Put(gomock.Any(), req.(*PutRequest)).Return(resp, nil).Times(1)
152+
},
153+
callFn: func(c Client, ctx context.Context, req any) (any, error) {
154+
return c.Put(ctx, req.(*PutRequest))
155+
},
156+
assertFn: func(t *testing.T, req any, resp any, result any, err error) {
157+
assert.NoError(t, err, "Expected no error on successful retry")
158+
assert.Equal(t, resp.(*PutResponse), result, "Expected the response to match")
159+
},
160+
},
161+
{
162+
name: "NotRetryOnError",
163+
retryPolicy: backoff.NewExponentialRetryPolicy(0),
164+
retryableError: true,
165+
req: &PutRequest{},
166+
expectFn: func(m *MockClient, req, resp any) {
167+
nonRetryableError := errors.New("non-retryable error")
168+
m.EXPECT().Put(gomock.Any(), req.(*PutRequest)).Return(nil, nonRetryableError).Times(1)
169+
m.EXPECT().IsRetryableError(nonRetryableError).Return(false).Times(1)
170+
},
171+
callFn: func(c Client, ctx context.Context, req any) (any, error) {
172+
return c.Put(ctx, req.(*PutRequest))
173+
},
174+
assertFn: func(t *testing.T, req any, resp any, result any, err error) {
175+
assert.Error(t, err)
176+
assert.Nil(t, result)
177+
},
178+
},
179+
}
134180

135-
mockClient.AssertExpectations(t)
181+
for _, tc := range tests {
182+
t.Run(tc.name, func(t *testing.T) {
183+
runCRUDTest(
184+
t,
185+
tc.retryPolicy,
186+
tc.retryableError,
187+
tc.req,
188+
tc.resp,
189+
tc.expectFn,
190+
tc.callFn,
191+
tc.assertFn,
192+
)
193+
})
194+
}
136195
}
137196

138197
func TestRetryableClient_IsRetryableError(t *testing.T) {
139-
mockClient := new(MockClient)
198+
mockClient := NewMockClient(gomock.NewController(t))
140199
client := &retryableClient{
141200
client: mockClient,
142201
throttleRetry: backoff.NewThrottleRetry(
@@ -146,11 +205,8 @@ func TestRetryableClient_IsRetryableError(t *testing.T) {
146205
}
147206

148207
retryableError := errors.New("retryable error")
208+
mockClient.EXPECT().IsRetryableError(retryableError).Return(true).Times(1)
149209

150-
mockClient.On("IsRetryableError", retryableError).Return(true).Once()
151-
152-
isRetryable := client.IsRetryableError(retryableError)
153-
assert.True(t, isRetryable, "Expected error to be retryable")
154-
155-
mockClient.AssertExpectations(t)
210+
isRetryableError := client.IsRetryableError(retryableError)
211+
assert.True(t, isRetryableError, "Expected error to be retryable")
156212
}

‎common/resource/resource_test_utils.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ func NewTest(
190190
MetricsClient: metrics.NewClient(scope, serviceMetricsIndex),
191191
ArchivalMetadata: &archiver.MockArchivalMetadata{},
192192
ArchiverProvider: provider.NewMockArchiverProvider(controller),
193-
BlobstoreClient: &blobstore.MockClient{},
193+
BlobstoreClient: blobstore.NewMockClient(controller),
194194
MockPayloadSerializer: persistence.NewMockPayloadSerializer(controller),
195195

196196
// membership infos

‎service/worker/scanner/executions/concrete_execution_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -402,12 +402,12 @@ func Test_concreteExecutionScannerIterator(t *testing.T) {
402402

403403
func Test_concreteExecutionFixerIterator(t *testing.T) {
404404
ctx := context.Background()
405-
mockClient := &blobstore.MockClient{}
405+
mockClient := blobstore.NewMockClient(gomock.NewController(t))
406406
req := &blobstore.GetRequest{
407407
Key: concreteExecutionsFixerTaskListName + "_0.",
408408
}
409409

410-
mockClient.On("Get", ctx, req).Return(&blobstore.GetResponse{}, nil).Once()
410+
mockClient.EXPECT().Get(ctx, req).Return(&blobstore.GetResponse{}, nil).Times(1)
411411

412412
it := concreteExecutionFixerIterator(
413413
ctx,

0 commit comments

Comments
 (0)