Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(3)

Issue 10550043: resumer: fix test race (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 10 months ago by mue
Modified:
10 years, 10 months ago
Reviewers:
dimitern, mp+171342, fwereade
Visibility:
Public.

Description

resumer: fix test race The resumer test might lead to a race (detected by Roger). So changed it to taking and comparing timestamps for each call of the resumer mock. https://code.launchpad.net/~themue/juju-core/031-fix-resumer-test/+merge/171342 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+17 lines, -7 lines) Patch
A [revision details] View 1 chunk +2 lines, -0 lines 0 comments Download
M worker/resumer/resumer_test.go View 1 chunk +15 lines, -7 lines 2 comments Download

Messages

Total messages: 4
mue
Please take a look.
10 years, 10 months ago (2013-06-25 15:42:23 UTC) #1
fwereade
LGTM
10 years, 10 months ago (2013-06-26 14:11:43 UTC) #2
dimitern
LGTM modulo a single suggestion. https://codereview.appspot.com/10550043/diff/1/worker/resumer/resumer_test.go File worker/resumer/resumer_test.go (right): https://codereview.appspot.com/10550043/diff/1/worker/resumer/resumer_test.go#newcode47 worker/resumer/resumer_test.go:47: c.Assert(len(tr.timestamps) > 0, Equals, ...
10 years, 10 months ago (2013-06-27 14:33:25 UTC) #3
mue
10 years, 10 months ago (2013-06-27 18:03:31 UTC) #4
https://codereview.appspot.com/10550043/diff/1/worker/resumer/resumer_test.go
File worker/resumer/resumer_test.go (right):

https://codereview.appspot.com/10550043/diff/1/worker/resumer/resumer_test.go...
worker/resumer/resumer_test.go:47: c.Assert(len(tr.timestamps) > 0, Equals,
true)
On 2013/06/27 14:33:26, dimitern wrote:
> This code block is a bit obscure to my puny mind - can you put some comment
> describing what we're actually doing here please?

Done.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b