all: fixed broken tests

1. Use cmp package to replace reflect.DeepEqual 2. db.PutIndex and db.DeleteIndex won't return error when remote_api is not setup, since they are only called inside db.Put and db.Delete. db.Search remains unchanged since it is called externally. Change-Id: Ife84f5a98abe721fbb127e497fdae34125d547a1 Reviewed-on: https://go-review.googlesource.com/78435 Reviewed-by: Ross Light <[email protected]>
golang · Nov 17, 2017 · 6bb33fc · 6bb33fc
1 parent 46b0a98
commit 6bb33fc
Show file tree

Hide file tree

Showing 21 changed files with 2,421 additions and 41 deletions.
diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json
diff --git a/database/database.go b/database/database.go
@@ -150,6 +150,8 @@ func New(serverURI string, idleTimeout time.Duration, logConn bool, gaeEndpoint
 		if rc, err = newRemoteClient(gaeEndpoint); err != nil {
 			return nil, err
 		}
+	} else {
+		log.Println("remote_api client not setup to use App Engine search")
 	}
 
 	return &Database{Pool: pool, RemoteClient: rc}, nil
@@ -366,6 +368,8 @@ func pkgIDAndImportCount(c redis.Conn, path string) (id string, numImported int,
 	id, err = redis.String(c.Do("HGET", "ids", path))
 	if err == redis.ErrNil {
 		return "", 0, nil
+	} else if err != nil {
+		return "", 0, err
 	}
 	return id, numImported, nil
 }
@@ -390,10 +394,12 @@ func (db *Database) updateImportsIndex(ctx context.Context, c redis.Conn, oldDoc
 	for p := range changes {
 		id, n, err := pkgIDAndImportCount(c, p)
 		if err != nil {
-			return err
+			log.Println("database.updateImportsIndex:", err)
 		}
 		if id != "" {
-			db.PutIndex(ctx, nil, id, -1, n)
+			if err := db.PutIndex(ctx, nil, id, -1, n); err != nil {
+				log.Printf("database.updateImportsIndex: putting package %s into the index: %v\n", p, err)
+			}
 		}
 	}
 	return nil
@@ -1282,17 +1288,19 @@ func (db *Database) Search(ctx context.Context, q string) ([]Package, error) {
 }
 
 // PutIndex puts a package into App Engine search index. ID is the package ID in the database.
+// It is no-op when running locally without setting up remote_api.
 func (db *Database) PutIndex(ctx context.Context, pdoc *doc.Package, id string, score float64, importCount int) error {
 	if db.RemoteClient == nil {
-		return errors.New("remote_api client not setup to use App Engine search")
+		return nil
 	}
 	return putIndex(db.RemoteClient.NewContext(ctx), pdoc, id, score, importCount)
 }
 
 // DeleteIndex deletes a package from App Engine search index. ID is the package ID in the database.
+// It is no-op when running locally without setting up remote_api.
 func (db *Database) DeleteIndex(ctx context.Context, id string) error {
 	if db.RemoteClient == nil {
-		return errors.New("database.DeleteIndex: no App Engine endpoint given")
+		return nil
 	}
 	return deleteIndex(db.RemoteClient.NewContext(ctx), id)
 }
diff --git a/database/database_test.go b/database/database_test.go
@@ -7,14 +7,14 @@
 package database
 
 import (
+	"context"
 	"math"
-	"reflect"
 	"strconv"
 	"testing"
 	"time"
 
 	"github.com/garyburd/redigo/redis"
-	"google.golang.org/appengine/aetest"
+	"github.com/google/go-cmp/cmp"
 
 	"github.com/golang/gddo/doc"
 )
@@ -40,13 +40,7 @@ func newDB(t *testing.T) *Database {
 		t.Errorf("DBSIZE returned %d, %v", n, err)
 	}
 
-	ctx, done, err := aetest.NewContext()
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer done()
-
-	return &Database{Pool: p, AppEngineContext: ctx}
+	return &Database{Pool: p, RemoteClient: nil}
 }
 
 func closeDB(db *Database) {
@@ -56,6 +50,7 @@ func closeDB(db *Database) {
 }
 
 func TestPutGet(t *testing.T) {
+	ctx := context.Background()
 	var nextCrawl = time.Unix(time.Now().Add(time.Hour).Unix(), 0).UTC()
 
 	db := newDB(t)
@@ -69,21 +64,21 @@ func TestPutGet(t *testing.T) {
 		Updated:     time.Now().Add(-time.Hour),
 		Imports:     []string{"C", "errors", "github.com/user/repo/foo/bar"}, // self import for testing convenience.
 	}
-	if err := db.Put(pdoc, nextCrawl, false); err != nil {
+	if err := db.Put(ctx, pdoc, nextCrawl, false); err != nil {
 		t.Errorf("db.Put() returned error %v", err)
 	}
-	if err := db.Put(pdoc, time.Time{}, false); err != nil {
+	if err := db.Put(ctx, pdoc, time.Time{}, false); err != nil {
 		t.Errorf("second db.Put() returned error %v", err)
 	}
 
-	actualPdoc, actualSubdirs, actualCrawl, err := db.Get("github.com/user/repo/foo/bar")
+	actualPdoc, actualSubdirs, actualCrawl, err := db.Get(ctx, "github.com/user/repo/foo/bar")
 	if err != nil {
 		t.Fatalf("db.Get(.../foo/bar) returned %v", err)
 	}
 	if len(actualSubdirs) != 0 {
 		t.Errorf("db.Get(.../foo/bar) returned subdirs %v, want none", actualSubdirs)
 	}
-	if !reflect.DeepEqual(actualPdoc, pdoc) {
+	if !cmp.Equal(actualPdoc, pdoc) {
 		t.Errorf("db.Get(.../foo/bar) returned doc %v, want %v", actualPdoc, pdoc)
 	}
 	if !nextCrawl.Equal(actualCrawl) {
@@ -96,7 +91,7 @@ func TestPutGet(t *testing.T) {
 	}
 	after := time.Now().Unix()
 
-	_, _, actualCrawl, _ = db.Get("github.com/user/repo/foo/bar")
+	_, _, actualCrawl, _ = db.Get(ctx, "github.com/user/repo/foo/bar")
 	if actualCrawl.Unix() < before || after < actualCrawl.Unix() {
 		t.Errorf("actualCrawl=%v, expect value between %v and %v", actualCrawl.Unix(), before, after)
 	}
@@ -109,31 +104,31 @@ func TestPutGet(t *testing.T) {
 
 	// Get "-"
 
-	actualPdoc, _, _, err = db.Get("-")
+	actualPdoc, _, _, err = db.Get(ctx, "-")
 	if err != nil {
 		t.Fatalf("db.Get(-) returned %v", err)
 	}
-	if !reflect.DeepEqual(actualPdoc, pdoc) {
+	if !cmp.Equal(actualPdoc, pdoc) {
 		t.Errorf("db.Get(-) returned doc %v, want %v", actualPdoc, pdoc)
 	}
 
-	actualPdoc, actualSubdirs, _, err = db.Get("github.com/user/repo/foo")
+	actualPdoc, actualSubdirs, _, err = db.Get(ctx, "github.com/user/repo/foo")
 	if err != nil {
 		t.Fatalf("db.Get(.../foo) returned %v", err)
 	}
 	if actualPdoc != nil {
 		t.Errorf("db.Get(.../foo) returned doc %v, want %v", actualPdoc, nil)
 	}
 	expectedSubdirs := []Package{{Path: "github.com/user/repo/foo/bar", Synopsis: "hello"}}
-	if !reflect.DeepEqual(actualSubdirs, expectedSubdirs) {
+	if !cmp.Equal(actualSubdirs, expectedSubdirs) {
 		t.Errorf("db.Get(.../foo) returned subdirs %v, want %v", actualSubdirs, expectedSubdirs)
 	}
 	actualImporters, err := db.Importers("github.com/user/repo/foo/bar")
 	if err != nil {
 		t.Fatalf("db.Importers() returned error %v", err)
 	}
 	expectedImporters := []Package{{Path: "github.com/user/repo/foo/bar", Synopsis: "hello"}}
-	if !reflect.DeepEqual(actualImporters, expectedImporters) {
+	if !cmp.Equal(actualImporters, expectedImporters) {
 		t.Errorf("db.Importers() = %v, want %v", actualImporters, expectedImporters)
 	}
 	actualImports, err := db.Packages(pdoc.Imports)
@@ -150,20 +145,20 @@ func TestPutGet(t *testing.T) {
 		{Path: "errors", Synopsis: ""},
 		{Path: "github.com/user/repo/foo/bar", Synopsis: "hello"},
 	}
-	if !reflect.DeepEqual(actualImports, expectedImports) {
+	if !cmp.Equal(actualImports, expectedImports) {
 		t.Errorf("db.Imports() = %v, want %v", actualImports, expectedImports)
 	}
 	importerCount, _ := db.ImporterCount("github.com/user/repo/foo/bar")
 	if importerCount != 1 {
 		t.Errorf("db.ImporterCount() = %d, want %d", importerCount, 1)
 	}
-	if err := db.Delete("github.com/user/repo/foo/bar"); err != nil {
+	if err := db.Delete(ctx, "github.com/user/repo/foo/bar"); err != nil {
 		t.Errorf("db.Delete() returned error %v", err)
 	}
 
 	db.Query("bar")
 
-	if err := db.Put(pdoc, time.Time{}, false); err != nil {
+	if err := db.Put(ctx, pdoc, time.Time{}, false); err != nil {
 		t.Errorf("db.Put() returned error %v", err)
 	}
 

diff --git a/database/index_test.go b/database/index_test.go
@@ -7,11 +7,11 @@
 package database
 
 import (
-	"reflect"
 	"sort"
 	"testing"
 
 	"github.com/golang/gddo/doc"
+	"github.com/google/go-cmp/cmp"
 )
 
 var indexTests = []struct {
@@ -94,7 +94,7 @@ func TestDocTerms(t *testing.T) {
 		terms := documentTerms(tt.pdoc, score)
 		sort.Strings(terms)
 		sort.Strings(tt.terms)
-		if !reflect.DeepEqual(terms, tt.terms) {
+		if !cmp.Equal(terms, tt.terms) {
 			t.Errorf("documentTerms(%s) ->\n got: %#v\nwant: %#v", tt.pdoc.ImportPath, terms, tt.terms)
 		}
 	}
@@ -195,7 +195,7 @@ func TestSynopsisTerms(t *testing.T) {
 		expected := tt.terms
 		sort.Strings(actual)
 		sort.Strings(expected)
-		if !reflect.DeepEqual(actual, expected) {
+		if !cmp.Equal(actual, expected) {
 			t.Errorf("%q ->\n got: %#v\nwant: %#v", tt.synopsis, actual, expected)
 		}
 	}

diff --git a/gddo-server/template_test.go b/gddo-server/template_test.go
@@ -9,9 +9,10 @@ package main
 import (
 	"net/http"
 	"net/http/httptest"
-	"reflect"
 	"strings"
 	"testing"
+
+	"github.com/google/go-cmp/cmp"
 )
 
 func TestFlashMessages(t *testing.T) {
@@ -27,7 +28,7 @@ func TestFlashMessages(t *testing.T) {
 	req := &http.Request{Header: http.Header{"Cookie": {strings.Split(resp.Header().Get("Set-Cookie"), ";")[0]}}}
 
 	actualMessages := getFlashMessages(resp, req)
-	if !reflect.DeepEqual(actualMessages, expectedMessages) {
+	if !cmp.Equal(actualMessages, expectedMessages) {
 		t.Errorf("got messages %+v, want %+v", actualMessages, expectedMessages)
 	}
 }
diff --git a/gosrc/gosrc_test.go b/gosrc/gosrc_test.go
@@ -7,14 +7,16 @@
 package gosrc
 
 import (
+	"context"
 	"fmt"
 	"io/ioutil"
 	"net/http"
 	"path"
-	"reflect"
 	"regexp"
 	"strings"
 	"testing"
+
+	"github.com/google/go-cmp/cmp"
 )
 
 var testWeb = map[string]string{
@@ -220,7 +222,7 @@ func (t testTransport) RoundTrip(req *http.Request) (*http.Response, error) {
 
 var githubPattern = regexp.MustCompile(`^github\.com/(?P<owner>[a-z0-9A-Z_.\-]+)/(?P<repo>[a-z0-9A-Z_.\-]+)(?P<dir>/[a-z0-9A-Z_.\-/]*)?$`)
 
-func testGet(client *http.Client, match map[string]string, etag string) (*Directory, error) {
+func testGet(ctx context.Context, client *http.Client, match map[string]string, etag string) (*Directory, error) {
 	importPath := match["importPath"]
 
 	if m := githubPattern.FindStringSubmatch(importPath); m != nil {
@@ -268,7 +270,7 @@ func TestGetDynamic(t *testing.T) {
 	client := &http.Client{Transport: testTransport(testWeb)}
 
 	for _, tt := range getDynamicTests {
-		dir, err := getDynamic(client, tt.importPath, "")
+		dir, err := getDynamic(context.Background(), client, tt.importPath, "")
 
 		if tt.dir == nil {
 			if err == nil {
@@ -282,7 +284,7 @@ func TestGetDynamic(t *testing.T) {
 			continue
 		}
 
-		if !reflect.DeepEqual(dir, tt.dir) {
+		if !cmp.Equal(dir, tt.dir) {
 			t.Errorf("getDynamic(client, %q, etag) =\n     %+v,\nwant %+v", tt.importPath, dir, tt.dir)
 			for i, f := range dir.Files {
 				var want *File

diff --git a/httputil/header/header_test.go b/httputil/header/header_test.go
@@ -8,9 +8,10 @@ package header
 
 import (
 	"net/http"
-	"reflect"
 	"testing"
 	"time"
+
+	"github.com/google/go-cmp/cmp"
 )
 
 var getHeaderListTests = []struct {
@@ -31,7 +32,7 @@ var getHeaderListTests = []struct {
 func TestGetHeaderList(t *testing.T) {
 	for _, tt := range getHeaderListTests {
 		header := http.Header{"Foo": {tt.s}}
-		if l := ParseList(header, "foo"); !reflect.DeepEqual(tt.l, l) {
+		if l := ParseList(header, "foo"); !cmp.Equal(tt.l, l) {
 			t.Errorf("ParseList for %q = %q, want %q", tt.s, l, tt.l)
 		}
 	}
@@ -68,7 +69,7 @@ func TestParseValueAndParams(t *testing.T) {
 		if value != tt.value {
 			t.Errorf("%q, value=%q, want %q", tt.s, value, tt.value)
 		}
-		if !reflect.DeepEqual(params, tt.params) {
+		if !cmp.Equal(params, tt.params) {
 			t.Errorf("%q, param=%#v, want %#v", tt.s, params, tt.params)
 		}
 	}
@@ -131,7 +132,7 @@ func TestParseAccept(t *testing.T) {
 	for _, tt := range parseAcceptTests {
 		header := http.Header{"Accept": {tt.s}}
 		actual := ParseAccept(header, "Accept")
-		if !reflect.DeepEqual(actual, tt.expected) {
+		if !cmp.Equal(actual, tt.expected) {
 			t.Errorf("ParseAccept(h, %q)=%v, want %v", tt.s, actual, tt.expected)
 		}
 	}

diff --git a/httputil/static_test.go b/httputil/static_test.go
@@ -14,12 +14,12 @@ import (
 	"net/http/httptest"
 	"net/url"
 	"os"
-	"reflect"
 	"strconv"
 	"testing"
 	"time"
 
 	"github.com/golang/gddo/httputil"
+	"github.com/google/go-cmp/cmp"
 )
 
 var (
@@ -151,7 +151,7 @@ func testStaticServer(t *testing.T, f func(*httputil.StaticServer) http.Handler)
 			t.Errorf("%s, status=%d, want %d", tt.name, w.Code, tt.status)
 		}
 
-		if !reflect.DeepEqual(w.HeaderMap, tt.header) {
+		if !cmp.Equal(w.HeaderMap, tt.header) {
 			t.Errorf("%s\n\theader=%v,\n\twant   %v", tt.name, w.HeaderMap, tt.header)
 		}