Code review comment for lp:~axwalk/juju-core/azure-test-ensure-called

Reviewers: mp+214147_code.launchpad.net,

Message:
Please take a look.

Description:
provider/azure: ensure tests work correctly

Some tests I wrote call StartInstance to
check the parameters passed to createInstance.
The tests don't actually verify that it called
that method successfully, and all validation
is done within the method. Updated the tests
to ensure from outside that the method is called.

https://code.launchpad.net/~axwalk/juju-core/azure-test-ensure-called/+merge/214147

(do not edit description out of merge proposal)

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

Affected files (+26, -6 lines):
   A [revision details]
   M provider/azure/environ_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-20140404003547-v7dzlndz9bq9h4qd
+New revision: <email address hidden>

Index: provider/azure/environ_test.go
=== modified file 'provider/azure/environ_test.go'
--- provider/azure/environ_test.go 2014-04-03 18:04:56 +0000
+++ provider/azure/environ_test.go 2014-04-04 01:54:24 +0000
@@ -1429,44 +1429,62 @@
    Tag: "machine-1",
   }
   var params environs.StartInstanceParams
- params.Tools = envtesting.AssertUploadFakeToolsVersions(c, env.storage,
envtesting.V120p...)
- params.MachineConfig = environs.NewMachineConfig("1", "yanonce", nil,
nil, stateInfo, apiInfo)
+ params.Tools = envtesting.AssertUploadFakeToolsVersions(
+ c, env.storage, envtesting.V120p...,
+ )
+ params.MachineConfig = environs.NewMachineConfig(
+ "1", "yanonce", nil, nil, stateInfo, apiInfo,
+ )

   // Start out with availability sets disabled.
   env.ecfg.attrs["availability-sets-enabled"] = false

   var expectServiceName string
   var expectStateServer bool
+ var called int
   s.PatchValue(&createInstance, func(env *azureEnviron, azure
*gwacl.ManagementAPI, role *gwacl.Role, serviceName string, stateServer
bool) (instance.Instance, error) {
    c.Assert(serviceName, gc.Equals, expectServiceName)
    c.Assert(stateServer, gc.Equals, expectStateServer)
+ called++
    return nil, nil
   })
   env.StartInstance(params)
+ c.Assert(called, gc.Equals, 1)

- // DistributionGroup won't have an effect if
availability-sets-enabled=false.
+ // DistributionGroup won't have an effect if
+ // availability-sets-enabled=false.
   expectServiceName = ""
   params.DistributionGroup = func() ([]instance.Id, error) {
- return []instance.Id{instance.Id(env.getEnvPrefix()
+ "whatever-role0")}, nil
+ return []instance.Id{
+ instance.Id(env.getEnvPrefix() + "whatever-role0"),
+ }, nil
   }
   env.StartInstance(params)
+ c.Assert(called, gc.Equals, 2)

   env.ecfg.attrs["availability-sets-enabled"] = true
   expectServiceName = "juju-testenv-whatever"
   env.StartInstance(params)
+ c.Assert(called, gc.Equals, 3)

   expectServiceName = ""
   params.DistributionGroup = nil
   env.StartInstance(params)
+ c.Assert(called, gc.Equals, 4)

   // Empty distribution group is equivalent to no DistributionGroup
function.
   params.DistributionGroup = func() ([]instance.Id, error) {
    return nil, nil
   }
   env.StartInstance(params)
+ c.Assert(called, gc.Equals, 5)

- // If the machine has the JobManagesEnviron job, we should see
stateServer==true.
+ // If the machine has the JobManagesEnviron job, we should see
+ // stateServer==true.
   expectStateServer = true
- params.MachineConfig.Jobs =
[]apiparams.MachineJob{apiparams.JobHostUnits, apiparams.JobManageEnviron}
+ params.MachineConfig.Jobs = []apiparams.MachineJob{
+ apiparams.JobHostUnits, apiparams.JobManageEnviron,
+ }
   env.StartInstance(params)
+ c.Assert(called, gc.Equals, 6)
  }