Merge lp:~axwalk/juju-core/testing-mgo-wait-listening into lp:~go-bot/juju-core/trunk

[Andrew Wilkins]

Improve mgo testing

- Use testing.MgoSuite in the store package
- Wait for "waiting for connections" message in mongod
  output before starting tests.
- Fix testing.MgoInstance.DialInfo to configure
  the client based on whether TLS is being used.

https://codereview.appspot.com/97110043/

Improve mgo testing

- Use testing.MgoSuite in the store package
- Wait for "waiting for connections" message in mongod
  output before starting tests.
- Fix testing.MgoInstance.DialInfo to configure
  the client based on whether TLS is being used.

https://codereview.appspot.com/97110043/

[Andrew Wilkins (axwalk)]

Reviewers: mp+218571_code.launchpad.net,

Message:
Please take a look.

Description:
Improve mgo testing

- Use testing.MgoSuite in the store package
- Wait for "waiting for connections" message in mongod
   output before starting tests.
- Fix testing.MgoInstance.DialInfo to configure
   the client based on whether TLS is being used.

https://code.launchpad.net/~axwalk/juju-core/testing-mgo-wait-listening/+merge/218571

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/97110043/

Affected files (+90, -140 lines):
   A [revision details]
   D store/mgo_test.go
   M store/store_test.go
   M testing/mgo.go

[William Reade (fwereade)]

LGTM

https://codereview.appspot.com/97110043/

=== removed file 'store/mgo_test.go'
--- store/mgo_test.go	2013-09-13 14:48:13 +0000
+++ store/mgo_test.go	1970-01-01 00:00:00 +0000
@@ -1,103 +0,0 @@
-// Copyright 2011, 2012, 2013 Canonical Ltd.
-// Licensed under the AGPLv3, see LICENCE file for details.
-
-package store_test
-
-import (
-	"bytes"
-	"os/exec"
-	"time"
-
-	"labix.org/v2/mgo"
-	gc "launchpad.net/gocheck"
-
-	"launchpad.net/juju-core/testing"
-)
-
-// ----------------------------------------------------------------------------
-// The mgo test suite
-
-type MgoSuite struct {
-	Addr    string
-	Session *mgo.Session
-	output  bytes.Buffer
-	server  *exec.Cmd
-}
-
-func (s *MgoSuite) SetUpSuite(c *gc.C) {
-	mgo.SetDebug(true)
-	mgo.SetStats(true)
-	dbdir := c.MkDir()
-	args := []string{
-		"--dbpath", dbdir,
-		"--bind_ip", "127.0.0.1",
-		"--port", "50017",
-		"--nssize", "1",
-		"--noprealloc",
-		"--smallfiles",
-		"--nojournal",
-	}
-	s.server = exec.Command("mongod", args...)
-	s.server.Stdout = &s.output
-	s.server.Stderr = &s.output
-	err := s.server.Start()
-	c.Assert(err, gc.IsNil)
-}
-
-func (s *MgoSuite) TearDownSuite(c *gc.C) {
-	s.server.Process.Kill()
-	s.server.Process.Wait()
-}
-
-func (s *MgoSuite) SetUpTest(c *gc.C) {
-	err := DropAll("localhost:50017")
-	c.Assert(err, gc.IsNil)
-	mgo.SetLogger(c)
-	mgo.ResetStats()
-	s.Addr = "127.0.0.1:50017"
-	s.Session, err = mgo.Dial(s.Addr)
-	c.Assert(err, gc.IsNil)
-}
-
-func (s *MgoSuite) TearDownTest(c *gc.C) {
-	if s.Session != nil {
-		s.Session.Close()
-	}
-	t0 := time.Now()
-	for i := 0; ; i++ {
-		stats := mgo.GetStats()
-		if stats.SocketsInUse == 0 && stats.SocketsAlive == 0 {
-			break
-		}
-		if time.Since(t0) > testing.LongWait {
-			// We wait up to 10s for all workers to finish up
-			c.Fatal("Test left sockets in a dirty state")
-		}
-		c.Logf("Waiting for sockets to die: %d in use, %d alive", stats.SocketsInUse, stats.SocketsAlive)
-		time.Sleep(testing.ShortWait)
-	}
-}
-
-func DropAll(mongourl string) (err error) {
-	session, err := mgo.Dial(mongourl)
-	if err != nil {
-		return err
-	}
-	defer session.Close()
-
-	names, err := session.DatabaseNames()
-	if err != nil {
-		return err
-	}
-	for _, name := range names {
-		switch name {
-		case "admin", "local", "config":
-		default:
-			err = session.DB(name).DropDatabase()
-			if err != nil {
-				return err
-			}
-		}
-	}
-	return nil
-}

=== modified file 'store/store_test.go'
--- store/store_test.go	2014-04-12 05:53:58 +0000
+++ store/store_test.go	2014-05-07 08:33:15 +0000
@@ -24,14 +24,14 @@
 )
 
 func Test(t *stdtesting.T) {
-	gc.TestingT(t)
+	testing.MgoTestPackageSsl(t, false)
 }
 
 var _ = gc.Suite(&StoreSuite{})
 var _ = gc.Suite(&TrivialSuite{})
 
 type StoreSuite struct {
-	MgoSuite
+	testing.MgoSuite
 	testing.HTTPSuite
 	testbase.LoggingSuite
 	store *store.Store
@@ -42,10 +42,9 @@
 type TrivialSuite struct{}
 
 func (s *StoreSuite) SetUpSuite(c *gc.C) {
+	s.LoggingSuite.SetUpSuite(c)
 	s.MgoSuite.SetUpSuite(c)
 	s.HTTPSuite.SetUpSuite(c)
-	s.LoggingSuite.SetUpSuite(c)
-
 	if os.Getenv("JUJU_NOTEST_MONGOJS") == "1" {
 		c.Log("Tests requiring MongoDB Javascript will be skipped")
 		*noTestMongoJs = true
@@ -53,26 +52,27 @@
 }
 
 func (s *StoreSuite) TearDownSuite(c *gc.C) {
-	s.LoggingSuite.TearDownSuite(c)
 	s.HTTPSuite.TearDownSuite(c)
 	s.MgoSuite.TearDownSuite(c)
+	s.LoggingSuite.TearDownSuite(c)
 }
 
 func (s *StoreSuite) SetUpTest(c *gc.C) {
+	s.LoggingSuite.SetUpTest(c)
 	s.MgoSuite.SetUpTest(c)
-	s.LoggingSuite.SetUpTest(c)
+	s.HTTPSuite.SetUpTest(c)
 	var err error
-	s.store, err = store.Open(s.Addr)
+	s.store, err = store.Open(testing.MgoServer.Addr())
 	c.Assert(err, gc.IsNil)
 }
 
 func (s *StoreSuite) TearDownTest(c *gc.C) {
-	s.LoggingSuite.TearDownTest(c)
-	s.HTTPSuite.TearDownTest(c)
 	if s.store != nil {
 		s.store.Close()
 	}
+	s.HTTPSuite.TearDownTest(c)
 	s.MgoSuite.TearDownTest(c)
+	s.LoggingSuite.TearDownTest(c)
 }
 
 // FakeCharmDir is a charm that implements the interface that the
@@ -839,7 +839,7 @@
 	}
 
 	// Use a different store to exercise cache filling.
-	st, err := store.Open(s.Addr)
+	st, err := store.Open(testing.MgoServer.Addr())
 	c.Assert(err, gc.IsNil)
 	defer st.Close()
 

=== modified file 'testing/mgo.go'
--- testing/mgo.go	2014-04-17 00:19:01 +0000
+++ testing/mgo.go	2014-05-07 08:33:15 +0000
@@ -5,6 +5,7 @@
 
 import (
 	"bufio"
+	"bytes"
 	"crypto/tls"
 	"crypto/x509"
 	"fmt"
@@ -14,6 +15,7 @@
 	"os"
 	"os/exec"
 	"path/filepath"
+	"regexp"
 	"strconv"
 	"strings"
 	stdtesting "testing"
@@ -31,6 +33,9 @@
 	// MgoServer is a shared mongo server used by tests.
 	MgoServer = &MgoInstance{ssl: true}
 	logger    = loggo.GetLogger("juju.testing")
+
+	// regular expression to match output of mongod
+	waitingForConnectionsRe = regexp.MustCompile(".*initandlisten.*waiting for connections.*")
 )
 
 type MgoInstance struct {
@@ -150,10 +155,21 @@
 	server.Stderr = server.Stdout
 	exited := make(chan struct{})
 	started := make(chan struct{})
+	listening := make(chan error, 1)
 	go func() {
 		<-started
-		lines := readLines(fmt.Sprintf("mongod:%v", mgoport), out, 20)
-		err := server.Wait()
+		// Wait until the server is listening.
+		var buf bytes.Buffer
+		prefix := fmt.Sprintf("mongod:%v", mgoport)
+		if readUntilMatching(prefix, io.TeeReader(out, &buf), waitingForConnectionsRe) {
+			listening <- nil
+		} else {
+			listening <- fmt.Errorf("mongod failed to listen on port %v", mgoport)
+		}
+		// Capture the last 20 lines of output from mongod, to log
+		// in the event of unclean exit.
+		lines := readLastLines(prefix, io.MultiReader(&buf, out), 20)
+		err = server.Wait()
 		exitErr, _ := err.(*exec.ExitError)
 		if err == nil || exitErr != nil && exitErr.Exited() {
 			// mongodb has exited without being killed, so print the
@@ -171,6 +187,11 @@
 	if err != nil {
 		return err
 	}
+	err = <-listening
+	close(listening)
+	if err != nil {
+		return err
+	}
 	inst.server = server
 
 	return nil
@@ -220,27 +241,41 @@
 	if MgoServer.addr == "" {
 		panic("MgoSuite tests must be run with MgoTestPackage")
 	}
+	mgo.SetDebug(true)
 	mgo.SetStats(true)
 	// Make tests that use password authentication faster.
 	utils.FastInsecureHash = true
 }
 
-// readLines reads lines from the given reader and returns
+// readUntilMatching reads lines from the given reader until the reader
+// is depleted or a line matches the given regular expression.
+func readUntilMatching(prefix string, r io.Reader, re *regexp.Regexp) bool {
+	sc := bufio.NewScanner(r)
+	for sc.Scan() {
+		line := sc.Text()
+		logger.Tracef("%s: %s", prefix, line)
+		if re.MatchString(line) {
+			return true
+		}
+	}
+	return false
+}
+
+// readLastLines reads lines from the given reader and returns
 // the last n non-empty lines, ignoring empty lines.
-func readLines(prefix string, r io.Reader, n int) []string {
-	br := bufio.NewReader(r)
+func readLastLines(prefix string, r io.Reader, n int) []string {
+	sc := bufio.NewScanner(r)
 	lines := make([]string, n)
 	i := 0
-	for {
-		line, err := br.ReadString('\n')
-		if line = strings.TrimRight(line, "\n"); line != "" {
+	for sc.Scan() {
+		if line := strings.TrimRight(sc.Text(), "\n"); line != "" {
 			logger.Tracef("%s: %s", prefix, line)
 			lines[i%n] = line
 			i++
 		}
-		if err != nil {
-			break
-		}
+	}
+	if err := sc.Err(); err != nil {
+		panic(err)
 	}
 	final := make([]string, 0, n+1)
 	if i > n {
@@ -276,7 +311,7 @@
 // DialInfo returns information suitable for dialling the
 // receiving MongoDB instance.
 func (inst *MgoInstance) DialInfo() *mgo.DialInfo {
-	return MgoDialInfo(inst.addr)
+	return MgoDialInfoTls(inst.ssl, inst.addr)
 }
 
 // DialDirect returns a new direct connection to the shared MongoDB server. This
@@ -301,28 +336,44 @@
 // for dialling an MgoInstance at any of the
 // given addresses.
 func MgoDialInfo(addrs ...string) *mgo.DialInfo {
-	pool := x509.NewCertPool()
-	xcert, err := cert.ParseCert(CACert)
-	if err != nil {
-		panic(err)
-	}
-	pool.AddCert(xcert)
-	tlsConfig := &tls.Config{
-		RootCAs:    pool,
-		ServerName: "anything",
-	}
-	return &mgo.DialInfo{
-		Addrs: addrs,
-		Dial: func(addr net.Addr) (net.Conn, error) {
+	return MgoDialInfoTls(true, addrs...)
+}
+
+// MgoDialInfoTls returns a DialInfo suitable
+// for dialling an MgoInstance at any of the
+// given addresses, optionally using TLS.
+func MgoDialInfoTls(useTls bool, addrs ...string) *mgo.DialInfo {
+	var dial func(addr net.Addr) (net.Conn, error)
+	if useTls {
+		pool := x509.NewCertPool()
+		xcert, err := cert.ParseCert(CACert)
+		if err != nil {
+			panic(err)
+		}
+		pool.AddCert(xcert)
+		tlsConfig := &tls.Config{
+			RootCAs:    pool,
+			ServerName: "anything",
+		}
+		dial = func(addr net.Addr) (net.Conn, error) {
 			conn, err := tls.Dial("tcp", addr.String(), tlsConfig)
 			if err != nil {
 				logger.Debugf("tls.Dial(%s) failed with %v", addr, err)
 				return nil, err
 			}
 			return conn, nil
-		},
-		Timeout: mgoDialTimeout,
+		}
+	} else {
+		dial = func(addr net.Addr) (net.Conn, error) {
+			conn, err := net.Dial("tcp", addr.String())
+			if err != nil {
+				logger.Debugf("net.Dial(%s) failed with %v", addr, err)
+				return nil, err
+			}
+			return conn, nil
+		}
 	}
+	return &mgo.DialInfo{Addrs: addrs, Dial: dial, Timeout: mgoDialTimeout}
 }
 
 func (s *MgoSuite) SetUpTest(c *gc.C) {