Deflake some tests.

Updates #25

compaction_test.go

@@ -632,19 +632,22 @@ func TestCompaction(t *testing.T) {
 			}
 		}
 
-		// Allow any writes to the memfs to complete.
-		time.Sleep(1 * time.Millisecond)
-
-		gotMem, gotDisk, err := getAll()
+		// try backs off to allow any writes to the memfs to complete.
+		err := try(100*time.Microsecond, 20*time.Second, func() error {
+			gotMem, gotDisk, err := getAll()
+			if err != nil {
+				return err
+			}
+			if gotMem != tc.wantMem {
+				return fmt.Errorf("mem: got %q, want %q", gotMem, tc.wantMem)
+			}
+			if gotDisk != tc.wantDisk {
+				return fmt.Errorf("ldb: got %q, want %q", gotDisk, tc.wantDisk)
+			}
+			return nil
+		})
 		if err != nil {
 			t.Errorf("%q: %v", tc.key, err)
-			break
-		}
-		if gotMem != tc.wantMem {
-			t.Errorf("%q: mem: got %q, want %q", tc.key, gotMem, tc.wantMem)
-		}
-		if gotDisk != tc.wantDisk {
-			t.Errorf("%q: ldb: got %q, want %q", tc.key, gotDisk, tc.wantDisk)
 		}
 	}
 

db/file.go

@@ -56,7 +56,7 @@ type FileSystem interface {
 	// Closer to release the lock.
 	//
 	// On Linux and OSX, a lock has the same semantics as fcntl(2)'s advisory
-	// locks.  In particular, closing any other file descriptor for the same
+	// locks. In particular, closing any other file descriptor for the same
 	// file will release the lock prematurely.
 	//
 	// Attempting to lock a file that is already locked by the current process

db/file_lock_test.go

@@ -14,7 +14,7 @@ import (
 	"github.com/golang/leveldb/db"
 )
 
-var lockFilename = flag.String("lockfile", "", "File to lock.  Non-empty value pimples child process.")
+var lockFilename = flag.String("lockfile", "", "File to lock. A non-empty value implies a child process.")
 
 func spawn(prog, filename string) ([]byte, error) {
 	return exec.Command(prog, "-lockfile", filename, "-test.v",
@@ -31,7 +31,7 @@ func TestLock(t *testing.T) {
 	if child {
 		filename = *lockFilename
 	} else {
-		f, err := ioutil.TempFile("", "")
+		f, err := ioutil.TempFile("", "golang-leveldb-db-testlock-")
 		if err != nil {
 			t.Fatal(err)
 		}

leveldb_test.go

@@ -1,11 +1,11 @@
 // Copyright 2012 The LevelDB-Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
+// Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file.
 
 package leveldb
 
 import (
 	"bytes"
+	"errors"
 	"io"
 	"math/rand"
 	"os"
@@ -14,12 +14,63 @@ import (
 	"sort"
 	"strconv"
 	"strings"
+	"sync"
 	"testing"
+	"time"
 
 	"github.com/golang/leveldb/db"
 	"github.com/golang/leveldb/memfs"
 )
 
+// try repeatedly calls f, sleeping between calls with exponential back-off,
+// until f returns a nil error or the total sleep time is greater than or equal
+// to maxTotalSleep. It always calls f at least once.
+func try(initialSleep, maxTotalSleep time.Duration, f func() error) error {
+	totalSleep := time.Duration(0)
+	for d := initialSleep; ; d *= 2 {
+		time.Sleep(d)
+		totalSleep += d
+		if err := f(); err == nil || totalSleep >= maxTotalSleep {
+			return err
+		}
+	}
+}
+
+func TestTry(t *testing.T) {
+	c := make(chan struct{})
+	go func() {
+		time.Sleep(1 * time.Millisecond)
+		close(c)
+	}()
+
+	attemptsMu := sync.Mutex{}
+	attempts := 0
+
+	err := try(100*time.Microsecond, 20*time.Second, func() error {
+		attemptsMu.Lock()
+		attempts++
+		attemptsMu.Unlock()
+
+		select {
+		default:
+			return errors.New("timed out")
+		case <-c:
+			return nil
+		}
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	attemptsMu.Lock()
+	a := attempts
+	attemptsMu.Unlock()
+
+	if a == 0 {
+		t.Fatalf("attempts: got 0, want > 0")
+	}
+}
+
 func TestErrorIfDBExists(t *testing.T) {
 	for _, b := range [...]bool{false, true} {
 		fs := memfs.New()

table_cache_test.go

@@ -53,45 +53,68 @@ func (fs *tableCacheTestFS) Open(name string) (db.File, error) {
 	return &tableCacheTestFile{f, fs, name}, nil
 }
 
-func (fs *tableCacheTestFS) validate(t *testing.T, c *tableCache, f func(i, gotO, gotC int)) {
-	// Let any clean-up goroutines do their work.
-	time.Sleep(1 * time.Millisecond)
-
-	fs.mu.Lock()
-	defer fs.mu.Unlock()
-	numStillOpen := 0
-	for i := 0; i < tableCacheTestNumTables; i++ {
-		filename := dbFilename("", fileTypeTable, uint64(i))
-		gotO, gotC := fs.openCounts[filename], fs.closeCounts[filename]
-		if gotO > gotC {
-			numStillOpen++
-		}
-		if gotC != gotO && gotC != gotO-1 {
-			t.Errorf("i=%d: table closed too many or too few times: opened %d times, closed %d times", i, gotO, gotC)
-		}
-		if f != nil {
-			f(i, gotO, gotC)
-		}
+func (fs *tableCacheTestFS) validate(t *testing.T, c *tableCache, f func(i, gotO, gotC int) error) {
+	if err := fs.validateOpenTables(f); err != nil {
+		t.Error(err)
+		return
 	}
-	if numStillOpen > tableCacheTestCacheSize {
-		t.Errorf("numStillOpen is %d, want <= %d", numStillOpen, tableCacheTestCacheSize)
-	}
-
-	// Close the tableCache and let any clean-up goroutines do their work.
-	fs.mu.Unlock()
 	c.Close()
-	time.Sleep(1 * time.Millisecond)
-	fs.mu.Lock()
-
-	for i := 0; i < tableCacheTestNumTables; i++ {
-		filename := dbFilename("", fileTypeTable, uint64(i))
-		gotO, gotC := fs.openCounts[filename], fs.closeCounts[filename]
-		if gotO != gotC {
-			t.Errorf("i=%d: opened %d times, closed %d times", i, gotO, gotC)
-		}
+	if err := fs.validateNoneStillOpen(); err != nil {
+		t.Error(err)
+		return
 	}
 }
 
+// validateOpenTables validates that no tables in the cache are open twice, and
+// the number still open is no greater than tableCacheTestCacheSize.
+func (fs *tableCacheTestFS) validateOpenTables(f func(i, gotO, gotC int) error) error {
+	// try backs off to let any clean-up goroutines do their work.
+	return try(100*time.Microsecond, 20*time.Second, func() error {
+		fs.mu.Lock()
+		defer fs.mu.Unlock()
+
+		numStillOpen := 0
+		for i := 0; i < tableCacheTestNumTables; i++ {
+			filename := dbFilename("", fileTypeTable, uint64(i))
+			gotO, gotC := fs.openCounts[filename], fs.closeCounts[filename]
+			if gotO > gotC {
+				numStillOpen++
+			}
+			if gotC != gotO && gotC != gotO-1 {
+				return fmt.Errorf("i=%d: table closed too many or too few times: opened %d times, closed %d times",
+					i, gotO, gotC)
+			}
+			if f != nil {
+				if err := f(i, gotO, gotC); err != nil {
+					return err
+				}
+			}
+		}
+		if numStillOpen > tableCacheTestCacheSize {
+			return fmt.Errorf("numStillOpen is %d, want <= %d", numStillOpen, tableCacheTestCacheSize)
+		}
+		return nil
+	})
+}
+
+// validateNoneStillOpen validates that no tables in the cache are open.
+func (fs *tableCacheTestFS) validateNoneStillOpen() error {
+	// try backs off to let any clean-up goroutines do their work.
+	return try(100*time.Microsecond, 20*time.Second, func() error {
+		fs.mu.Lock()
+		defer fs.mu.Unlock()
+
+		for i := 0; i < tableCacheTestNumTables; i++ {
+			filename := dbFilename("", fileTypeTable, uint64(i))
+			gotO, gotC := fs.openCounts[filename], fs.closeCounts[filename]
+			if gotO != gotC {
+				return fmt.Errorf("i=%d: opened %d times, closed %d times", i, gotO, gotC)
+			}
+		}
+		return nil
+	})
+}
+
 const (
 	tableCacheTestNumTables = 300
 	tableCacheTestCacheSize = 100
@@ -212,14 +235,15 @@ func TestTableCacheFrequentlyUsed(t *testing.T) {
 		}
 	}
 
-	fs.validate(t, c, func(i, gotO, gotC int) {
+	fs.validate(t, c, func(i, gotO, gotC int) error {
 		if i == pinned0 || i == pinned1 {
 			if gotO != 1 || gotC != 0 {
-				t.Errorf("i=%d: pinned table: got %d, %d, want %d, %d", i, gotO, gotC, 1, 0)
+				return fmt.Errorf("i=%d: pinned table: got %d, %d, want %d, %d", i, gotO, gotC, 1, 0)
 			}
 		} else if gotO == 1 {
-			t.Errorf("i=%d: table only opened once", i)
+			return fmt.Errorf("i=%d: table only opened once", i)
 		}
+		return nil
 	})
 }
 
@@ -249,7 +273,7 @@ func TestTableCacheEvictions(t *testing.T) {
 
 	sumEvicted, nEvicted := 0, 0
 	sumSafe, nSafe := 0, 0
-	fs.validate(t, c, func(i, gotO, gotC int) {
+	fs.validate(t, c, func(i, gotO, gotC int) error {
 		if lo <= i && i < hi {
 			sumEvicted += gotO
 			nEvicted++
@@ -257,6 +281,7 @@ func TestTableCacheEvictions(t *testing.T) {
 			sumSafe += gotO
 			nSafe++
 		}
+		return nil
 	})
 	fEvicted := float64(sumEvicted) / float64(nEvicted)
 	fSafe := float64(sumSafe) / float64(nSafe)