environs/dummy: add delay

environs/dummy: add delay

This proposal implements a delay in most dummy operations. The delay 
can be set test wide by setting JUJU_DUMMY_DELAY to a time.Duration 
parsable value.

https://code.launchpad.net/~dave-cheney/juju-core/074-environs-dummy-seniors-moment/+merge/121526

(do not edit description out of merge proposal)

[dave_cheney.net, 2012-08-28 02:00:02]

Please take a look.

[dave_cheney.net, 2012-08-28 02:11:53]

Please take a look.

[niemeyer, 2012-08-28 13:11:56]

http://codereview.appspot.com/6482081/diff/3001/environs/dummy/environs.go
File environs/dummy/environs.go (right):

http://codereview.appspot.com/6482081/diff/3001/environs/dummy/environs.go#ne...
environs/dummy/environs.go:120: <-time.After(e.delayTime)
time.Sleep(e.delayTime)

http://codereview.appspot.com/6482081/diff/3001/environs/dummy/environs.go#ne...
environs/dummy/environs.go:162: providerDelay, _ =
time.ParseDuration(os.Getenv("JUJU_DUMMY_DELAY"))
I'd prefer to stay out of environment variables for that kind of purpose, as
hidden state affects things in a way that is not obvious. What about having a
-dummy.delay flag that is added by a test suite, and causes a
dummy.DelayOps(duration) function in here to be called?

[dave_cheney.net, 2012-08-29 04:48:12]

As discussed on IRC I think this is unavoidable in this case as a test flag only
works local to the package being tested. In this case we need to test things
that consume dummy.

[dave_cheney.net, 2012-08-29 04:49:11]

Please take a look.

[dave_cheney.net, 2012-08-29 04:56:38]

Please take a look.

[niemeyer, 2012-08-29 13:32:48]

LGTM with the unused variable dropped.

https://codereview.appspot.com/6482081/diff/9001/environs/dummy/environs.go
File environs/dummy/environs.go (right):

https://codereview.appspot.com/6482081/diff/9001/environs/dummy/environs.go#n...
environs/dummy/environs.go:113: delayTime     time.Duration // delay before
actioning request
This variable is unused.

https://codereview.appspot.com/6482081/diff/9001/environs/dummy/environs.go#n...
environs/dummy/environs.go:597: // pause execution to simulate the latency of a
real provider
I'd pull down the variable and init function excerpt down here instead. One of
the great benefits of Go's parsing model is being able to keep things in context
in these cases, keeping logic clean from unrelated matters.

[dave_cheney.net, 2012-08-29 23:14:11]

Please take a look.

https://codereview.appspot.com/6482081/diff/9001/environs/dummy/environs.go
File environs/dummy/environs.go (right):

https://codereview.appspot.com/6482081/diff/9001/environs/dummy/environs.go#n...
environs/dummy/environs.go:113: delayTime     time.Duration // delay before
actioning request
On 2012/08/29 13:32:48, niemeyer wrote:
> This variable is unused.

Done.

https://codereview.appspot.com/6482081/diff/9001/environs/dummy/environs.go#n...
environs/dummy/environs.go:597: // pause execution to simulate the latency of a
real provider
On 2012/08/29 13:32:48, niemeyer wrote:
> I'd pull down the variable and init function excerpt down here instead. One of
> the great benefits of Go's parsing model is being able to keep things in
context
> in these cases, keeping logic clean from unrelated matters.

Done.

[dave_cheney.net, 2012-08-29 23:26:39]

*** Submitted:

environs/dummy: add delay

This proposal implements a delay in most dummy operations. The delay 
can be set test wide by setting JUJU_DUMMY_DELAY to a time.Duration 
parsable value.

R=niemeyer
CC=
https://codereview.appspot.com/6482081