Skip to content

Commit ff82a88

Browse files
[CLD-559]: test(job): fix race condition (#330)
Recently, we been [getting race condition](https://github.com/smartcontractkit/chainlink-deployments-framework/actions/runs/17254351292/job/48963163120?pr=328#step:2:491) on the mocked JobClient when ProposeJob is called. From the stack trace, we know there is a read ProposeJob from log: ``` github.com/stretchr/testify/mock.(*Mock).Called() /home/runner/go/pkg/mod/github.com/stretchr/[email protected]/mock/mock.go:481 +0x191 github.com/smartcontractkit/chainlink-deployments-framework/offchain/jd/internal/mocks.(*MockJobServiceClient).ProposeJob() /home/runner/work/chainlink-deployments-framework/chainlink-deployments-framework/offchain/jd/internal/mocks/mock_job_service_client.go:546 +0x34c ``` and a write at ``` Previous write at 0x00c000188060 by goroutine 15: sync/atomic.StoreInt64() /opt/hostedtoolcache/go/1.24.4/x64/src/runtime/race_amd64.s:237 +0xb sync/atomic.StorePointer() /opt/hostedtoolcache/go/1.24.4/x64/src/runtime/atomic_pointer.go:92 +0x44 github.com/smartcontractkit/chainlink-protos/job-distributor/v1/job.(*ProposeJobRequest).ProtoReflect() /home/runner/go/pkg/mod/github.com/smartcontractkit/chainlink-protos/[email protected]/v1/job/job.pb.go:791 +0x93 ``` We know it is the test `TestJDClient_ProposeJob` from the logs below ``` Goroutine 15 (running) created at: testing.(*T).Run() /opt/hostedtoolcache/go/1.24.4/x64/src/testing/testing.go:1851 +0x8f2 github.com/smartcontractkit/chainlink-deployments-framework/offchain/jd.TestJDClient_ProposeJob() /home/runner/work/chainlink-deployments-framework/chainlink-deployments-framework/offchain/jd/client_test.go:308 +0xae4 ``` After some investigation, we use the same pointer for `jobv1.ProposeJobRequest` on all the test cases for `TestJDClient_ProposeJob`. ``` --- FAIL: TestJDClient_ProposeJob (0.00s) --- FAIL: TestJDClient_ProposeJob/success_with_empty_request (0.00s) testing.go:1490: race detected during execution of test --- FAIL: TestJDClient_ProposeJob/success_with_valid_proposal (0.00s) testing.go:1490: race detected during execution of test --- FAIL: TestJDClient_ProposeJob/error_when_ProposeJob_service_call_fails (0.00s) testing.go:1490: race detected during execution of test ``` How it happen: ``` // Shared ProposeJobRequest object request := &jobv1.ProposeJobRequest{...} // Goroutine 15 (Test A) mockJob.EXPECT().ProposeJob(ctx, request) // Triggers: └─ Arguments.Diff() └─ fmt.Sprintf("%v", request) └─ request.String() └─ request.ProtoReflect() └─ atomic.StorePointer(&metadata) // WRITE! // Goroutine 16 (Test B) - happening simultaneously mockJob.EXPECT().ProposeJob(ctx, request) // Triggers: └─ Arguments comparison └─ reflect.DeepEqual() └─ reflect.Value.IsNil() └─ reads the same memory location // READ! ``` Looks like for mocked object, when using `Arguments.Diff()` for comparing arguments called and expected, it will call `String()` of the object, which for protobuf object, it eventually calls ProtoReflect which performs a write [here](https://github.com/smartcontractkit/chainlink-protos/blob/main/job-distributor/v1/job/job.pb.go#L174), this may be the reason for the race condition. Not super confident, but lets see if it happens again. JIRA: https://smartcontract-it.atlassian.net/browse/CLD-559
1 parent 240a44f commit ff82a88

File tree

1 file changed

+6
-14
lines changed

1 file changed

+6
-14
lines changed

offchain/jd/client_test.go

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -198,23 +198,16 @@ func TestJDClient_GetCSAPublicKey(t *testing.T) {
198198
func TestJDClient_ProposeJob(t *testing.T) {
199199
t.Parallel()
200200

201-
var (
202-
request = &jobv1.ProposeJobRequest{
203-
NodeId: "test-node-123",
204-
Spec: "test-job-spec",
205-
}
206-
)
207-
208201
tests := []struct {
209202
name string
210-
beforeFunc func(t *testing.T, mockJob *mocks.MockJobServiceClient)
203+
beforeFunc func(t *testing.T, mockJob *mocks.MockJobServiceClient, request *jobv1.ProposeJobRequest)
211204
request *jobv1.ProposeJobRequest
212205
want *jobv1.ProposeJobResponse
213206
wantErr string
214207
}{
215208
{
216209
name: "success with valid proposal",
217-
beforeFunc: func(t *testing.T, mockJob *mocks.MockJobServiceClient) {
210+
beforeFunc: func(t *testing.T, mockJob *mocks.MockJobServiceClient, request *jobv1.ProposeJobRequest) {
218211
t.Helper()
219212

220213
response := &jobv1.ProposeJobResponse{
@@ -244,7 +237,7 @@ func TestJDClient_ProposeJob(t *testing.T) {
244237
},
245238
{
246239
name: "error when ProposeJob service call fails",
247-
beforeFunc: func(t *testing.T, mockJob *mocks.MockJobServiceClient) {
240+
beforeFunc: func(t *testing.T, mockJob *mocks.MockJobServiceClient, request *jobv1.ProposeJobRequest) {
248241
t.Helper()
249242

250243
mockJob.EXPECT().ProposeJob(
@@ -260,7 +253,7 @@ func TestJDClient_ProposeJob(t *testing.T) {
260253
},
261254
{
262255
name: "error when proposal is nil in response",
263-
beforeFunc: func(t *testing.T, mockJob *mocks.MockJobServiceClient) {
256+
beforeFunc: func(t *testing.T, mockJob *mocks.MockJobServiceClient, request *jobv1.ProposeJobRequest) {
264257
t.Helper()
265258

266259
response := &jobv1.ProposeJobResponse{
@@ -280,10 +273,9 @@ func TestJDClient_ProposeJob(t *testing.T) {
280273
},
281274
{
282275
name: "success with empty request",
283-
beforeFunc: func(t *testing.T, mockJob *mocks.MockJobServiceClient) {
276+
beforeFunc: func(t *testing.T, mockJob *mocks.MockJobServiceClient, request *jobv1.ProposeJobRequest) {
284277
t.Helper()
285278

286-
request := &jobv1.ProposeJobRequest{}
287279
response := &jobv1.ProposeJobResponse{
288280
Proposal: &jobv1.Proposal{
289281
Id: "empty-proposal-123",
@@ -310,7 +302,7 @@ func TestJDClient_ProposeJob(t *testing.T) {
310302

311303
// Create mock job service client
312304
mockClients := mocks.NewMockClients(t)
313-
tt.beforeFunc(t, mockClients.JobServiceClient)
305+
tt.beforeFunc(t, mockClients.JobServiceClient, tt.request)
314306

315307
// Create JobDistributor with mock
316308
jd := &JobDistributor{

0 commit comments

Comments
 (0)