Open grevych opened 2 years ago
Re-bootstrapping with a recent version flags a race condition when running tests:
:: Running go test (or_test) 60ms . ·······················↷·································✖✖✖✖✖✖ graphql_test 63 tests, 1 skipped, 6 failures in 8.945s === Skipped === SKIP: . TestDoCustom (0.00s) do_test.go:51: === Failed === FAIL: . TestCustomOperationWithHeaders (0.01s) testing.go:1152: race detected during execution of test === FAIL: . TestQuery (0.01s) testing.go:1152: race detected during execution of test === FAIL: . TestCustomOperation (0.01s) testing.go:1152: race detected during execution of test === FAIL: . TestQueryWithHeaders (0.01s) testing.go:1152: race detected during execution of test === FAIL: . TestMutate (0.01s) testing.go:1152: race detected during execution of test === FAIL: . TestMutateWithHeaders (0.01s) testing.go:1152: race detected during execution of test DONE 63 tests, 1 skipped, 6 failures in 8.963s make: *** [test] Error 1
The issue occurs in the marshal method when multiple requests try to update the declaration name of the same cached operation https://github.com/getoutreach/goql/blob/6f72c1efa60eb971ac68dd8dbd602d133083d6e0/query.go#L636
The proposed fix copies the cached operation by value before it is updated. Since no other fields are updated during the marshalling process, there's no need make a deep copy (by reference).
N/A
What this PR does / why we need it
Re-bootstrapping with a recent version flags a race condition when running tests:
The issue occurs in the marshal method when multiple requests try to update the declaration name of the same cached operation https://github.com/getoutreach/goql/blob/6f72c1efa60eb971ac68dd8dbd602d133083d6e0/query.go#L636
The proposed fix copies the cached operation by value before it is updated. Since no other fields are updated during the marshalling process, there's no need make a deep copy (by reference).
Jira ID
N/A
Notes for your reviewers