-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Hook up mgo logging and reconnect properly. #110
Conversation
0201aa3
to
90e4095
Compare
@@ -422,7 +423,16 @@ func MgoTestPackage(t *testing.T, certs *Certs) { | |||
gc.TestingT(t) | |||
} | |||
|
|||
// Output implements the mgo log_Logger interface. | |||
func (s *MgoSuite) Output(calldepth int, message string) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't mind too much, but this doesn't really belong on the suite. A simpler wrapper type would be cleaner. The logger wouldn't even have to be added as a field on MgoSuite...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
foundAddress := false | ||
for _, addr := range s.Session.LiveServers() { | ||
if addr == MgoServer.Addr() { | ||
foundAddress = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
break here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added.
@@ -758,6 +770,7 @@ func (inst *MgoInstance) EnsureRunning() error { | |||
// If the server has already been destroyed for testing purposes, | |||
// just start it again. | |||
if inst.Addr() == "" { | |||
logger.Infof("restarting mongo instance") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should probably be debug
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
90e4095
to
fad98e2
Compare
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju-testing |
Now the mgo logging is output through loggo to the "mgo" logger.
This means it is now available to be shown, but not shown by default.
To enable in other tests, they can (after logging is set up) set:
loggo.GetLogger("mgo").SetLogLevel(loggo.TRACE)
Also, when the mongo server is changed by either restarting, or other tests killing it, the teardown now reconnects the session, which avoids some of the "no reachable servers" errors we were seeing.