state: Test Destroy() better, small fix.

state: Test Destroy() better, small fix.

We weren't testing the service.Destroy() behavior
separately and since we encountered an issue with
the API tests when service is not destroyed properly,
here are more specific tests. Also a small issue was
fixed in Destory()'s defer func.

https://code.launchpad.net/~dimitern/juju-core/011-fix-state-service-destroy/+merge/152477

(do not edit description out of merge proposal)

[dimitern, 2013-03-08 17:59:28]

Please take a look.

[gz, 2013-03-08 19:52:07]

LGTM

https://codereview.appspot.com/7616044/diff/1/state/service.go
File state/service.go (right):

https://codereview.appspot.com/7616044/diff/1/state/service.go#newcode78
state/service.go:78: if err == nil {
I got William to explain this to me as well...

[fwereade, 2013-03-08 19:53:30]

LGTM

https://codereview.appspot.com/7616044/diff/1/state/service_test.go
File state/service_test.go (right):

https://codereview.appspot.com/7616044/diff/1/state/service_test.go#newcode445
state/service_test.go:445: func (s *ServiceSuite) TestDestroyHaveNoUnits(c *C) {
s/HaveNoUnits/OnceHadUnits/

[dimitern, 2013-03-08 20:02:57]

*** Submitted:

state: Test Destroy() better, small fix.

We weren't testing the service.Destroy() behavior
separately and since we encountered an issue with
the API tests when service is not destroyed properly,
here are more specific tests. Also a small issue was
fixed in Destory()'s defer func.

R=gz, fwereade
CC=
https://codereview.appspot.com/7616044

https://codereview.appspot.com/7616044/diff/1/state/service_test.go
File state/service_test.go (right):

https://codereview.appspot.com/7616044/diff/1/state/service_test.go#newcode445
state/service_test.go:445: func (s *ServiceSuite) TestDestroyHaveNoUnits(c *C) {
On 2013/03/08 19:53:30, fwereade wrote:
> s/HaveNoUnits/OnceHadUnits/

Done.

[jameinel, 2013-03-08 20:29:48]

LGTM

https://codereview.appspot.com/7616044/diff/7001/state/service_test.go
File state/service_test.go (right):

https://codereview.appspot.com/7616044/diff/7001/state/service_test.go#newcod...
state/service_test.go:457: c.Assert(s.mysql.Life(), Equals, state.Dying)
Would you want to assert that s.mysql.Life() is *not* state.Dying before you
call Destroy? (Then you are asserting that it transitioned from something to
something.)

Just a thought.

[dimitern, 2013-03-08 20:52:44]

https://codereview.appspot.com/7616044/diff/7001/state/service_test.go
File state/service_test.go (right):

https://codereview.appspot.com/7616044/diff/7001/state/service_test.go#newcod...
state/service_test.go:457: c.Assert(s.mysql.Life(), Equals, state.Dying)
On 2013/03/08 20:29:48, jameinel wrote:
> Would you want to assert that s.mysql.Life() is *not* state.Dying before you
> call Destroy? (Then you are asserting that it transitioned from something to
> something.)
> 
> Just a thought.

It'll either be Alive or it won't exist. Destroy() does not care if it's already
dying. And anyway - we start with the case that it's Alive anyway, so I don't
think we need to check it here. There are other Life() tests who do that.