state: fixed FlagWatcher to not fire when only content is changed

state: fixed FlagWatcher to not fire when only content is changed

The first version of the FlagWatcher also fired events when the
content of the watched node has been changed. Even if this usage
is not the intention of the FlagWatcher (it only has to watch for
the creation and deletion of nodes) this is an unwanted behavior.
After the fix it only looks for existence changes.

https://code.launchpad.net/~themue/juju-core/go-state-flag-watcher-fix/+merge/112065

(do not edit description out of merge proposal)

[TheMue, 2012-06-26 10:56:53]

Please take a look.

[fwereade, 2012-06-26 10:58:58]

The change is good, but I'd prefer it if we had a test that exercised this
behaviour somewhere.

[TheMue, 2012-06-26 15:21:28]

Please take a look.

[fwereade, 2012-06-26 15:42:53]

LGTM

[niemeyer, 2012-06-27 00:15:52]

LGTM, with a minor.

https://codereview.appspot.com/6336058/diff/4001/state/watcher_test.go
File state/watcher_test.go (right):

https://codereview.appspot.com/6336058/diff/4001/state/watcher_test.go#newcod...
state/watcher_test.go:145: err = exposedWatcher.Stop()
It's fine to do this here, but this must also be deferred after it's created,
otherwise any assertion failing above leaves things running in background.

I see there is the same problem in other tests. Can you please take the chance
to fix them too?

[TheMue, 2012-06-27 08:46:02]

*** Submitted:

state: fixed FlagWatcher to not fire when only content is changed

The first version of the FlagWatcher also fired events when the
content of the watched node has been changed. Even if this usage
is not the intention of the FlagWatcher (it only has to watch for
the creation and deletion of nodes) this is an unwanted behavior.
After the fix it only looks for existence changes.

R=fwereade, niemeyer
CC=
https://codereview.appspot.com/6336058

https://codereview.appspot.com/6336058/diff/4001/state/watcher_test.go
File state/watcher_test.go (right):

https://codereview.appspot.com/6336058/diff/4001/state/watcher_test.go#newcod...
state/watcher_test.go:145: err = exposedWatcher.Stop()
On 2012/06/27 00:15:52, niemeyer wrote:
> It's fine to do this here, but this must also be deferred after it's created,
> otherwise any assertion failing above leaves things running in background.
> 
> I see there is the same problem in other tests. Can you please take the chance
> to fix them too?

Done.