Code review comment for lp://staging/~fwereade/juju-core/strawman-state-test-simplification

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

https://codereview.appspot.com/6862053/diff/1/state/conn_test.go
File state/conn_test.go (right):

https://codereview.appspot.com/6862053/diff/1/state/conn_test.go#newcode99
state/conn_test.go:99: func (cs *ConnSuite) AddService(c *C, charmName
string) *state.Service {
I can easily see tests being copy & pasted and changing their meaning
wildly and silently depending on where the logic is pasted.

This should probably be AddService(c, charmName, serviceName), plus fail
the test if service name exists.

https://codereview.appspot.com/6862053/diff/1/state/conn_test.go#newcode122
state/conn_test.go:122: cs.RemoveUnit(c, unit)
How often is that done/necessary in a test?

https://codereview.appspot.com/6862053/diff/1/state/conn_test.go#newcode154
state/conn_test.go:154: err := unit.UnassignFromMachine()
Why do we have to unassign the principal? We should be able to remove a
unit without every unassigning it, right?

https://codereview.appspot.com/6862053/

« Back to merge proposal