[ca/dispatcher]: Use the mock proposer in the test CA memory store#2097
Conversation
cyli
changed the title
Codecov Report
@@ Coverage Diff @@
## master #2097 +/- ##
==========================================
- Coverage 57.24% 57.22% -0.03%
==========================================
Files 117 117
Lines 20128 20124 -4
==========================================
- Hits 11522 11515 -7
+ Misses 7279 7274 -5
- Partials 1327 1335 +8Continue to review full report at Codecov.
|
|
|
||
| }) | ||
| assert.NoError(t, err) | ||
| assert.NoError(t, testutils.PollFuncWithTimeout(nil, func() error { |
There was a problem hiding this comment.
Using PollFuncWithTimeout doesn't seem right here. What temporary error condition is it protecting against?
There was a problem hiding this comment.
The intention was to prevent a mismatched version number, I guess in case something updates one of the tasks in between the get and update.
There was a problem hiding this comment.
Not possible - Only one Update can run at a time
There was a problem hiding this comment.
ah right, I forgot there's no real agent here that updates the task status
There was a problem hiding this comment.
Even if there was, it would not be able to change the task status in betweenthe GetTask and the UpdateTask. Update transactions are atomic because they take an update lock on the store.
There was a problem hiding this comment.
Have removed the polls - PTAL
cyli
force-pushed
the
use-mock-proposer-in-testca-store
branch
from
587fda9 to
eef27d1
Compare
|
I noticed there are a few places that wrap the context using |
| recvChan <- struct{}{} | ||
| select { | ||
| case recvChan <- struct{}{}: | ||
| case <-ctx.Done(): |
There was a problem hiding this comment.
What would cause this case to be reached?
cyli
force-pushed
the
use-mock-proposer-in-testca-store
branch
from
eef27d1 to
e1f9b0c
Compare
…e when updating during the tests that there are no conflicts. Signed-off-by: cyli <ying.li@docker.com>
@aaronlehmann Reverted those changes :) Thanks! |
|
LGTM |
2 participants
This way during tests we can make sure there are no conflicts when updating a store from multiple goroutines simultaneously.
cc @aaronlehmann