Code review comment for lp://staging/~jimmiebtlr/juju-core/add_machines_tag

> +func runAddMachine(c *gc.C, args ...string) (*cmd.Context, error) {
> + cxt, err := testing.RunCommand(c, envcmd.Wrap(&AddMachineCommand{}), args...)
> + return cxt, err
> }

This can be collapsed onto one line

> func (s *AddMachineSuite) TestAddMachine(c *gc.C) {
> - err := runAddMachine(c)
> + cxt, err := runAddMachine(c)
> + c.Assert(testing.Stderr(cxt), gc.Equals, "created machine machine-0\n")

Why is this going to stderr, it's not an error.

> c.Assert(err, gc.IsNil)

Test the error first

> m, err := s.State.Machine("0")
> c.Assert(err, gc.IsNil)
> c.Assert(m.Life(), gc.Equals, state.Alive)
> - c.Assert(m.Series(), gc.DeepEquals, "precise")
> + c.Assert(m.Series(), gc.Equals, "precise")
> mcons, err := m.Constraints()
> c.Assert(err, gc.IsNil)
> c.Assert(&mcons, jc.Satisfies, constraints.IsEmpty)
> }
>
> func (s *AddMachineSuite) TestAddMachineWithSeries(c *gc.C) {
> - err := runAddMachine(c, "--series", "series")
> + cxt, err := runAddMachine(c, "--series", "series")
> + c.Assert(testing.Stderr(cxt), gc.Equals, "created machine machine-0\n")

same

> c.Assert(err, gc.IsNil)
> m, err := s.State.Machine("0")
> c.Assert(err, gc.IsNil)
> @@ -50,7 +53,8 @@
> }
>
> func (s *AddMachineSuite) TestAddMachineWithConstraints(c *gc.C) {
> - err := runAddMachine(c, "--constraints", "mem=4G")
> + cxt, err := runAddMachine(c, "--constraints", "mem=4G")
> + c.Assert(testing.Stderr(cxt), gc.Equals, "created machine machine-0\n")

same

> c.Assert(err, gc.IsNil)
> m, err := s.State.Machine("0")
> c.Assert(err, gc.IsNil)
> @@ -76,49 +80,65 @@
>
> func (s *AddMachineSuite) TestAddContainerToNewMachine(c *gc.C) {
> for i, ctype := range instance.ContainerTypes {
> - c.Logf("test %d: %s", i, ctype)
> - err := runAddMachine(c, string(ctype))
> + cxt, err := runAddMachine(c, string(ctype))
> + c.Assert(testing.Stderr(cxt), gc.Equals, "created container machine-"+strconv.Itoa(i)+"-"+string(ctype)+"-0\n")

same

> c.Assert(err, gc.IsNil)
> s._assertAddContainer(c, strconv.Itoa(i), fmt.Sprintf("%d/%s/0", i, ctype), ctype)
> }
> }
>
> func (s *AddMachineSuite) TestAddContainerToExistingMachine(c *gc.C) {
> - err := runAddMachine(c)
> + cxt, err := runAddMachine(c)
> + c.Assert(testing.Stderr(cxt), gc.Equals, "created machine machine-0\n")
> c.Assert(err, gc.IsNil)
> for i, container := range instance.ContainerTypes {
> machineNum := strconv.Itoa(i + 1)
> - err = runAddMachine(c)
> + cxt, err = runAddMachine(c)
> + c.Assert(testing.Stderr(cxt), gc.Equals, "created machine machine-"+machineNum+"\n")
> c.Assert(err, gc.IsNil)
> - err := runAddMachine(c, fmt.Sprintf("%s:%s", container, machineNum))
> + cxt, err := runAddMachine(c, fmt.Sprintf("%s:%s", container, machineNum))
> + if string(container) == "machine" {
> + c.Assert(testing.Stderr(cxt), gc.Equals, "created container machine-"+machineNum+"\n")
> + } else {
> + c.Assert(testing.Stderr(cxt), gc.Equals, "created container machine-"+machineNum+"-"+string(container)+"-0\n")
> + }
> c.Assert(err, gc.IsNil)
> s._assertAddContainer(c, machineNum, fmt.Sprintf("%s/%s/0", machineNum, container), container)
> }
> }
>
> func (s *AddMachineSuite) TestAddUnsupportedContainerToMachine(c *gc.C) {
> - err := runAddMachine(c)
> + cxt, err := runAddMachine(c)
> + c.Assert(testing.Stderr(cxt), gc.Equals, "created machine machine-0\n")
> c.Assert(err, gc.IsNil)
> m, err := s.State.Machine("0")
> c.Assert(err, gc.IsNil)
> m.SetSupportedContainers([]instance.ContainerType{instance.KVM})
> - err = runAddMachine(c, "lxc:0")
> + cxt, err = runAddMachine(c, "lxc:0")
> + c.Assert(testing.Stderr(cxt), gc.Equals, "")
> c.Assert(err, gc.ErrorMatches, "cannot add a new machine: machine 0 cannot host lxc containers")
> }
>
> func (s *AddMachineSuite) TestAddMachineErrors(c *gc.C) {
> - err := runAddMachine(c, ":lxc")
> + cxt, err := runAddMachine(c, ":lxc")
> + c.Assert(testing.Stderr(cxt), gc.Equals, "")
> c.Check(err, gc.ErrorMatches, `cannot add a new machine: :lxc placement is invalid`)
> - err = runAddMachine(c, "lxc:")
> + cxt, err = runAddMachine(c, "lxc:")
> + c.Assert(cxt, gc.IsNil)
> c.Check(err, gc.ErrorMatches, `invalid value "" for "lxc" scope: expected machine-id`)
> - err = runAddMachine(c, "2")
> + cxt, err = runAddMachine(c, "2")
> + c.Assert(testing.Stderr(cxt), gc.Equals, "")
> c.Check(err, gc.ErrorMatches, `machine-id cannot be specified when adding machines`)
> - err = runAddMachine(c, "foo")
> + cxt, err = runAddMachine(c, "foo")
> + c.Assert(testing.Stderr(cxt), gc.Equals, "")
> c.Check(err, gc.ErrorMatches, `cannot add a new machine: foo placement is invalid`)
> - err = runAddMachine(c, "foo:bar")
> + cxt, err = runAddMachine(c, "foo:bar")
> + c.Assert(testing.Stderr(cxt), gc.Equals, "")
> c.Check(err, gc.ErrorMatches, `invalid environment name "foo"`)
> - err = runAddMachine(c, "dummyenv:invalid")
> + cxt, err = runAddMachine(c, "dummyenv:invalid")
> + c.Assert(testing.Stderr(cxt), gc.Equals, "")
> c.Check(err, gc.ErrorMatches, `cannot add a new machine: invalid placement is invalid`)
> - err = runAddMachine(c, "lxc", "--constraints", "container=lxc")
> + cxt, err = runAddMachine(c, "lxc", "--constraints", "container=lxc")
> + c.Assert(cxt, gc.IsNil)
> c.Check(err, gc.ErrorMatches, `container constraint "lxc" not allowed when adding a machine`)
> }
>
>