Code review comment for lp:~mfoord/juju-core/instancepoller-aggregate

LGTM with the timing-based test fixed to make it less potentially flaky.

https://codereview.appspot.com/74900044/diff/20001/worker/instancepoller/aggregate.go
File worker/instancepoller/aggregate.go (right):

https://codereview.appspot.com/74900044/diff/20001/worker/instancepoller/aggregate.go#newcode60
worker/instancepoller/aggregate.go:60: var capacity int64 = 1
const capacity = 1

or just inline it, possibly with a comment saying
why we choose a capacity of 1 for the bucket.

https://codereview.appspot.com/74900044/diff/20001/worker/instancepoller/aggregate_test.go
File worker/instancepoller/aggregate_test.go (right):

https://codereview.appspot.com/74900044/diff/20001/worker/instancepoller/aggregate_test.go#newcode47
worker/instancepoller/aggregate_test.go:47: ids []instance.Id
It would still be nice to see a comment here.

https://codereview.appspot.com/74900044/diff/20001/worker/instancepoller/aggregate_test.go#newcode48
worker/instancepoller/aggregate_test.go:48: results []*testInstance
I'd still suggest that this is []instance.Instance, unless there's a
particular reason not to.

Then Instances is just:

func (i *testInstanceGetter) Instances(ids []instance.Id) (result
[]instance.Instance, err error) {
      i.id = ids
      atomic.AddInt32(i.counter, 1)
      return i.result, i.err
}

https://codereview.appspot.com/74900044/diff/20001/worker/instancepoller/aggregate_test.go#newcode148
worker/instancepoller/aggregate_test.go:148:
c.Assert(testGetter.counter, jc.GreaterThan, 10)
I think this is potentially flaky. If this test runs on a heavily loaded
machine, the millisecond sleeps may expand indefinitely.

I think a better approach might be to measure the total time taken
between starting the requests and receiving all the results, then
checking that the number of requests is no greater than that time
divided by 10ms + 1.

https://codereview.appspot.com/74900044/