mstate: moved life with tests in own files

mstate: moved life with tests in own files

As the lifecycle implementation is used by the four
entities Relation, Unit, Service and Machine it is
now moved into an own file with an own test suite.

https://code.launchpad.net/~themue/juju-core/go-mstate-life-file/+merge/120800

Requires: https://code.launchpad.net/~themue/juju-core/go-mstate-unit-lifecycle/+merge/120744

(do not edit description out of merge proposal)

[TheMue, 2012-08-22 14:39:54]

Please take a look.

[aram, 2012-08-22 14:48:39]

LGTM. Fantastic, thank you.

https://codereview.appspot.com/6481045/diff/1/mstate/life_test.go
File mstate/life_test.go (right):

https://codereview.appspot.com/6481045/diff/1/mstate/life_test.go#newcode137
mstate/life_test.go:137: err = l.Die()
I think it's clearer if we move this call to the teardown function.

https://codereview.appspot.com/6481045/diff/1/mstate/life_test.go#newcode158
mstate/life_test.go:158: err = s.State.RemoveRelation(r)
Here, before this call, so we see them grouped together.

https://codereview.appspot.com/6481045/diff/1/mstate/life_test.go#newcode173
mstate/life_test.go:173: err = peer.RemoveUnit(u)
Ditto.

[TheMue, 2012-08-22 14:59:02]

Please take a look.

https://codereview.appspot.com/6481045/diff/1/mstate/life_test.go
File mstate/life_test.go (right):

https://codereview.appspot.com/6481045/diff/1/mstate/life_test.go#newcode137
mstate/life_test.go:137: err = l.Die()
On 2012/08/22 14:48:39, aram wrote:
> I think it's clearer if we move this call to the teardown function.

Done.

https://codereview.appspot.com/6481045/diff/1/mstate/life_test.go#newcode158
mstate/life_test.go:158: err = s.State.RemoveRelation(r)
On 2012/08/22 14:48:39, aram wrote:
> Here, before this call, so we see them grouped together.

Done.

https://codereview.appspot.com/6481045/diff/1/mstate/life_test.go#newcode173
mstate/life_test.go:173: err = peer.RemoveUnit(u)
On 2012/08/22 14:48:39, aram wrote:
> Ditto.

Done.

[aram, 2012-08-22 15:05:01]

LGTM

[niemeyer, 2012-08-23 23:04:07]

https://codereview.appspot.com/6481045/diff/4001/mstate/life.go
File mstate/life.go (right):

https://codereview.appspot.com/6481045/diff/4001/mstate/life.go#newcode31
mstate/life.go:31: type Living interface {
This is great.

https://codereview.appspot.com/6481045/diff/4001/mstate/life.go#newcode56
mstate/life.go:56: return fmt.Errorf("cannot set life to %s for %s %q: %v",
life, descr, id, err)
s/life to %s/life to %q/

https://codereview.appspot.com/6481045/diff/4001/mstate/life_test.go
File mstate/life_test.go (right):

https://codereview.appspot.com/6481045/diff/4001/mstate/life_test.go#newcode145
mstate/life_test.go:145: envs := []*livingEnv{
The test refactoring is going in a great direction, but there's no reason for
this to not be normal (readable) code:

type unitLife struct{}
func (unitLife) setup(...) {}
func (unitLife) teardown(...) {}

type lifeFixture struct {
    func setup(...)
    func teardown(...)
}

https://codereview.appspot.com/6481045/diff/4001/mstate/life_test.go#newcode149
mstate/life_test.go:149: return s.createRelationWithLife(peer, cached,
dbinitial, c)
We don't need to copy the database logic for each different entity. All we need
is a collection name and an id. The hacking for all the types will be the same.
We can have an id() method returning (coll string, id interface{}) in the
lifeFixture.

https://codereview.appspot.com/6481045/diff/4001/mstate/life_test.go#newcode182
mstate/life_test.go:182: for _, env := range envs {
for _, lifet := []lifeTest{unitLife, relationLife} {
    unitLife.setup(...)
    ...
}

https://codereview.appspot.com/6481045/diff/4001/mstate/life_test.go#newcode183
mstate/life_test.go:183: s.testLifecycleStateChangesForLiving(env, c)
This can be inlined.

[TheMue, 2012-08-24 13:01:48]

Please take a look.

https://codereview.appspot.com/6481045/diff/4001/mstate/life.go
File mstate/life.go (right):

https://codereview.appspot.com/6481045/diff/4001/mstate/life.go#newcode31
mstate/life.go:31: type Living interface {
On 2012/08/23 23:04:07, niemeyer wrote:
> This is great.

Cheers.

https://codereview.appspot.com/6481045/diff/4001/mstate/life.go#newcode56
mstate/life.go:56: return fmt.Errorf("cannot set life to %s for %s %q: %v",
life, descr, id, err)
On 2012/08/23 23:04:07, niemeyer wrote:
> s/life to %s/life to %q/

Done.

https://codereview.appspot.com/6481045/diff/4001/mstate/life_test.go
File mstate/life_test.go (right):

https://codereview.appspot.com/6481045/diff/4001/mstate/life_test.go#newcode145
mstate/life_test.go:145: envs := []*livingEnv{
On 2012/08/23 23:04:07, niemeyer wrote:
> The test refactoring is going in a great direction, but there's no reason for
> this to not be normal (readable) code:
> 
> type unitLife struct{}
> func (unitLife) setup(...) {}
> func (unitLife) teardown(...) {}
> 
> type lifeFixture struct {
>     func setup(...)
>     func teardown(...)
> }

Done.

https://codereview.appspot.com/6481045/diff/4001/mstate/life_test.go#newcode149
mstate/life_test.go:149: return s.createRelationWithLife(peer, cached,
dbinitial, c)
On 2012/08/23 23:04:07, niemeyer wrote:
> We don't need to copy the database logic for each different entity. All we
need
> is a collection name and an id. The hacking for all the types will be the
same.
> We can have an id() method returning (coll string, id interface{}) in the
> lifeFixture.

Done.

https://codereview.appspot.com/6481045/diff/4001/mstate/life_test.go#newcode182
mstate/life_test.go:182: for _, env := range envs {
On 2012/08/23 23:04:07, niemeyer wrote:
> for _, lifet := []lifeTest{unitLife, relationLife} {
>     unitLife.setup(...)
>     ...
> }

Done.

https://codereview.appspot.com/6481045/diff/4001/mstate/life_test.go#newcode183
mstate/life_test.go:183: s.testLifecycleStateChangesForLiving(env, c)
On 2012/08/23 23:04:07, niemeyer wrote:
> This can be inlined. 

Done.

[TheMue, 2012-08-24 13:10:26]

Please take a look.

[niemeyer, 2012-08-27 15:27:11]

Thanks for the changes. Only a trivial, and LGTM.

https://codereview.appspot.com/6481045/diff/9002/mstate/life_test.go
File mstate/life_test.go (right):

https://codereview.appspot.com/6481045/diff/9002/mstate/life_test.go#newcode130
mstate/life_test.go:130: err := s.session.DB("juju").C(coll).UpdateId(id,
bson.D{{"$set", bson.D{
Local variables FTW:

c := s.session.DB("juju").C(coll)
err := c.UpdateId(id, bson.D{{"$set", bson.D{{"life", cached}}}})
...
err := c.UpdateId(id, bson.D{{"$set", bson.D{{"life", dbinitial}}}})

[TheMue, 2012-08-27 16:08:48]

*** Submitted:

mstate: moved life with tests in own files

As the lifecycle implementation is used by the four
entities Relation, Unit, Service and Machine it is
now moved into an own file with an own test suite.

R=aram, niemeyer
CC=
https://codereview.appspot.com/6481045