Merge lp:~axwalk/juju-core/lp1282692-test-bootstrapssh-timesensitive into lp:~go-bot/juju-core/trunk

[Andrew Wilkins]

provider/common: fix time-sensitive test

Test unnecessarily uses a goroutine; we
can just prime the "interrupted" channel
before calling into WaitSSH.

Fixes lp:1282692

https://codereview.appspot.com/97730043/

provider/common: fix time-sensitive test

Test unnecessarily uses a goroutine; we
can just prime the "interrupted" channel
before calling into WaitSSH.

Fixes lp:1282692

https://codereview.appspot.com/97730043/

[Andrew Wilkins (axwalk)]

Reviewers: mp+216997_code.launchpad.net,

Message:
Please take a look.

Description:
provider/common: fix time-sensitive test

Test unnecessarily uses a goroutine; we
can just prime the "interrupted" channel
before calling into WaitSSH.

Fixes lp:1282692

https://code.launchpad.net/~axwalk/juju-core/lp1282692-test-bootstrapssh-timesensitive/+merge/216997

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/97730043/

Affected files (+3, -4 lines):
   A [revision details]
   M provider/common/bootstrap_test.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: tarmac-20140424054112-tzfpqi85a2hve52c
+New revision: <email address hidden>

Index: provider/common/bootstrap_test.go
=== modified file 'provider/common/bootstrap_test.go'
--- provider/common/bootstrap_test.go 2014-04-24 02:27:38 +0000
+++ provider/common/bootstrap_test.go 2014-04-24 08:15:45 +0000
@@ -250,10 +250,7 @@
  func (s *BootstrapSuite) TestWaitSSHKilledWaitingForAddresses(c *gc.C) {
   ctx := coretesting.Context(c)
   interrupted := make(chan os.Signal, 1)
- go func() {
- <-time.After(2 * time.Millisecond)
- interrupted <- os.Interrupt
- }()
+ interrupted <- os.Interrupt
   _, err := common.WaitSSH(ctx, interrupted,
ssh.DefaultClient, "/bin/true", neverAddresses{}, testSSHTimeout)
   c.Check(err, gc.ErrorMatches, "interrupted")
   c.Check(coretesting.Stderr(ctx), gc.Matches, "Waiting for address\n")

[John A Meinel (jameinel)]

so this may never actually wait for connecting, right? (it could easily
notice that it was interrupted before it even tried to connect. And the
test is at least theoretically that it notices while it is waiting.

Anyway, LGTM mostly because it makes the test more straightforward.

https://codereview.appspot.com/97730043/

=== modified file 'provider/common/bootstrap_test.go'
--- provider/common/bootstrap_test.go	2014-04-24 02:27:38 +0000
+++ provider/common/bootstrap_test.go	2014-04-24 08:18:14 +0000
@@ -250,10 +250,7 @@
 func (s *BootstrapSuite) TestWaitSSHKilledWaitingForAddresses(c *gc.C) {
 	ctx := coretesting.Context(c)
 	interrupted := make(chan os.Signal, 1)
-	go func() {
-		<-time.After(2 * time.Millisecond)
-		interrupted <- os.Interrupt
-	}()
+	interrupted <- os.Interrupt
 	_, err := common.WaitSSH(ctx, interrupted, ssh.DefaultClient, "/bin/true", neverAddresses{}, testSSHTimeout)
 	c.Check(err, gc.ErrorMatches, "interrupted")
 	c.Check(coretesting.Stderr(ctx), gc.Matches, "Waiting for address\n")