/juju

state/presence: cleanups for issue 1588574 #5530

Closed
wants to merge 1 commit into
from

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
1 participant
@davecheney
@davecheney
Contributor

davecheney commented Jun 3, 2016 •
Edited 1 time

Updates LP # 1588574

This PR applies some cleanups to state/presence uncovered during the investigation of 1588574. The main change is the removal of the panic -> error logic in ping. Honestly, if the mongo driver panics, we need to fix that problem, not brush it under the rug.

  • Remove unused BaseSuite
  • Move KillForTesting helper to export_test.go
  • Remove recover logic from ping, this practice is questionable, especially as it is not applied in the identical sync path.

(Review request: http://reviews.vapour.ws/r/4977/)
@davecheney
state/presence: stop watchers/pingers before TearDownTest 
Fixes LP 1588574

The `panic: session already closed` comes from a watcher or pinger that
is still running when the test case returns to TearDownTest. This
happens because while we always call w.Stop in a defer, w.Stop does not
actually stop the worker, it just asks it to stop.

To ensure the worker is stopped, introduce a new function `assertStopped`
which stops a worker and waits until it reports stopped.  This method should
be used in favor of defer w.Stop() because you _must_ ensure that all workers
that share a mongo connection (which is _all_ of them) have stopped, and thus
are no longer using a mgo session before `TearDownTest` shuts down the connection.

Because this pattern of stopping and not waiting was common to both the
pinter and watcher tests, and is now resolved, we can remove the
`recover` in the `ping()` method.
dbc4245

@davecheney davecheney closed this Jun 6, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment