Skip to content

Commit

Permalink
Remove uses of x.Check in tests in favor of t.Fatalf (#4180)
Browse files Browse the repository at this point in the history
Where possible, I have removed calls to x.Check in the tests and
replaced them with calls to t.Fatalf or b.Fatalf.
  • Loading branch information
martinmr committed Oct 17, 2019
1 parent f7da338 commit 8d76d7d
Show file tree
Hide file tree
Showing 9 changed files with 78 additions and 48 deletions.
12 changes: 9 additions & 3 deletions codec/codec_test.go
Expand Up @@ -184,13 +184,17 @@ func BenchmarkGzip(b *testing.B) {
for _, uid := range uids {
n := binary.PutUvarint(tmp, uid)
_, err := buf.Write(tmp[:n])
x.Check(err)
if err != nil {
b.Fatalf("Error while writing to buffer: %s", err.Error())
}
}

var out bytes.Buffer
zw := gzip.NewWriter(&out)
_, err := zw.Write(buf.Bytes())
x.Check(err)
if err != nil {
b.Fatalf("Error while writing to gzip writer: %s", err.Error())
}

data = out.Bytes()
}
Expand All @@ -211,7 +215,9 @@ func benchmarkUidPackEncode(b *testing.B, blockSize int) {
for i := 0; i < b.N; i++ {
pack := Encode(uids, blockSize)
out, err := pack.Marshal()
x.Check(err)
if err != nil {
b.Fatalf("Error marshaling uid pack: %s", err.Error())
}
data = out
}
b.Logf("Output size: %s. Compression: %.2f",
Expand Down
12 changes: 9 additions & 3 deletions dgraph/cmd/alpha/http_test.go
Expand Up @@ -114,7 +114,9 @@ func queryWithGz(queryText, contentType, debug, timeout string, gzReq, gzResp bo
}

var r res
x.Check(json.Unmarshal(body, &r))
if err := json.Unmarshal(body, &r); err != nil {
return "", nil, err
}

// Check for errors
if len(r.Errors) != 0 {
Expand Down Expand Up @@ -146,7 +148,9 @@ func queryWithTs(queryText, contentType, debug string, ts uint64) (string, uint6
}

var r res
x.Check(json.Unmarshal(body, &r))
if err := json.Unmarshal(body, &r); err != nil {
return "", 0, err
}
startTs := r.Extensions.Txn.StartTs

// Remove the extensions.
Expand Down Expand Up @@ -178,7 +182,9 @@ func mutationWithTs(m, t string, isJson bool, commitNow bool, ts uint64) (
}

var r res
x.Check(json.Unmarshal(body, &r))
if err := json.Unmarshal(body, &r); err != nil {
return nil, nil, 0, err
}
startTs := r.Extensions.Txn.StartTs

return r.Extensions.Txn.Keys, r.Extensions.Txn.Preds, startTs, nil
Expand Down
13 changes: 9 additions & 4 deletions dgraph/cmd/alpha/mutations_mode/mutations_mode_test.go
Expand Up @@ -23,7 +23,6 @@ import (

"github.com/dgraph-io/dgo/v2"
"github.com/dgraph-io/dgo/v2/protos/api"
"github.com/dgraph-io/dgraph/x"
"github.com/stretchr/testify/require"

"google.golang.org/grpc"
Expand Down Expand Up @@ -161,7 +160,9 @@ func mutateExistingAllowed2(t *testing.T, dg *dgo.Dgraph) {

func TestMutationsDisallow(t *testing.T) {
conn, err := grpc.Dial(disallowModeAlpha, grpc.WithInsecure())
x.Check(err)
if err != nil {
t.Fatalf("Cannot perform drop all op: %s", err.Error())
}
defer conn.Close()

t.Run("disallow drop all in no mutations mode",
Expand All @@ -176,11 +177,15 @@ func TestMutationsDisallow(t *testing.T) {

func TestMutationsStrict(t *testing.T) {
conn1, err := grpc.Dial(strictModeAlphaGroup1, grpc.WithInsecure())
x.Check(err)
if err != nil {
t.Fatalf("Cannot perform drop all op: %s", err.Error())
}
defer conn1.Close()

conn2, err := grpc.Dial(strictModeAlphaGroup2, grpc.WithInsecure())
x.Check(err)
if err != nil {
t.Fatalf("Cannot perform drop all op: %s", err.Error())
}
defer conn2.Close()

t.Run("allow group1 drop all in strict mutations mode",
Expand Down
5 changes: 3 additions & 2 deletions dgraph/cmd/counter/increment_test.go
Expand Up @@ -29,7 +29,6 @@ import (
"github.com/dgraph-io/dgo/v2"
"github.com/dgraph-io/dgo/v2/protos/api"
"github.com/dgraph-io/dgraph/testutil"
"github.com/dgraph-io/dgraph/x"
"github.com/spf13/viper"
"github.com/stretchr/testify/require"
"google.golang.org/grpc/metadata"
Expand Down Expand Up @@ -146,7 +145,9 @@ func setup(t *testing.T) *dgo.Dgraph {
md := metadata.New(nil)
md.Append("auth-token", "mrjn2")
ctx = metadata.NewOutgoingContext(ctx, md)
x.Check(dg.Alter(ctx, &op))
if err := dg.Alter(ctx, &op); err != nil {
t.Fatalf("Cannot perform drop all op: %s", err.Error())
}

conf := viper.New()
conf.Set("pred", "counter.val")
Expand Down
42 changes: 28 additions & 14 deletions ee/backup/tests/filesystem/backup_test.go
Expand Up @@ -93,7 +93,7 @@ func TestBackupFilesystem(t *testing.T) {
require.True(t, moveOk)

// Setup test directories.
dirSetup()
dirSetup(t)

// Send backup request.
_ = runBackup(t, 3, 1)
Expand Down Expand Up @@ -195,7 +195,7 @@ func TestBackupFilesystem(t *testing.T) {
runFailingRestore(t, copyBackupDir, "", incr3.Txn.CommitTs)

// Clean up test directories.
dirCleanup()
dirCleanup(t)
}

func runBackup(t *testing.T, numExpectedFiles, numExpectedDirs int) []string {
Expand All @@ -220,7 +220,7 @@ func runBackupInternal(t *testing.T, forceFull bool, numExpectedFiles,
require.Contains(t, string(buf), "Backup completed.")

// Verify that the right amount of files and directories were created.
copyToLocalFs()
copyToLocalFs(t)

files := x.WalkPathFunc(copyBackupDir, func(path string, isdir bool) bool {
return !isdir && strings.HasSuffix(path, ".backup")
Expand Down Expand Up @@ -269,33 +269,47 @@ func runFailingRestore(t *testing.T, backupLocation, lastDir string, commitTs ui
require.Contains(t, err.Error(), "expected a BackupNum value of 1")
}

func dirSetup() {
func dirSetup(t *testing.T) {
// Clean up data from previous runs.
dirCleanup()
dirCleanup(t)

for _, dir := range testDirs {
x.Check(os.MkdirAll(dir, os.ModePerm))
if err := os.MkdirAll(dir, os.ModePerm); err != nil {
t.Fatalf("Error creating directory: %s", err.Error())
}
}

for _, alpha := range alphaContainers {
cmd := []string{"mkdir", "-p", alphaBackupDir}
x.Check(testutil.DockerExec(alpha, cmd...))
if err := testutil.DockerExec(alpha, cmd...); err != nil {
t.Fatalf("Error executing command in docker container: %s", err.Error())
}
}
}

func dirCleanup() {
x.Check(os.RemoveAll(restoreDir))
x.Check(os.RemoveAll(copyBackupDir))
func dirCleanup(t *testing.T) {
if err := os.RemoveAll(restoreDir); err != nil {
t.Fatalf("Error removing directory: %s", err.Error())
}
if err := os.RemoveAll(copyBackupDir); err != nil {
t.Fatalf("Error removing directory: %s", err.Error())
}

cmd := []string{"bash", "-c", "rm -rf /data/backups/dgraph.*"}
x.Check(testutil.DockerExec(alphaContainers[0], cmd...))
if err := testutil.DockerExec(alphaContainers[0], cmd...); err != nil {
t.Fatalf("Error executing command in docker container: %s", err.Error())
}
}

func copyToLocalFs() {
func copyToLocalFs(t *testing.T) {
// The original backup files are not accessible because docker creates all files in
// the shared volume as the root user. This restriction is circumvented by using
// "docker cp" to create a copy that is not owned by the root user.
x.Check(os.RemoveAll(copyBackupDir))
if err := os.RemoveAll(copyBackupDir); err != nil {
t.Fatalf("Error removing directory: %s", err.Error())
}
srcPath := "alpha1:/data/backups"
x.Check(testutil.DockerCp(srcPath, copyBackupDir))
if err := testutil.DockerCp(srcPath, copyBackupDir); err != nil {
t.Fatalf("Error copying files from docker container: %s", err.Error())
}
}
18 changes: 11 additions & 7 deletions ee/backup/tests/minio/backup_test.go
Expand Up @@ -98,7 +98,7 @@ func TestBackupMinio(t *testing.T) {
require.True(t, moveOk)

// Setup test directories.
dirSetup()
dirSetup(t)

// Send backup request.
_ = runBackup(t, 3, 1)
Expand Down Expand Up @@ -200,7 +200,7 @@ func TestBackupMinio(t *testing.T) {
runFailingRestore(t, backupDir, "", incr3.Txn.CommitTs)

// Clean up test directories.
dirCleanup()
dirCleanup(t)
}

func runBackup(t *testing.T, numExpectedFiles, numExpectedDirs int) []string {
Expand Down Expand Up @@ -274,17 +274,21 @@ func runFailingRestore(t *testing.T, backupLocation, lastDir string, commitTs ui
require.Contains(t, err.Error(), "expected a BackupNum value of 1")
}

func dirSetup() {
func dirSetup(t *testing.T) {
// Clean up data from previous runs.
dirCleanup()
dirCleanup(t)

for _, dir := range testDirs {
x.Check(os.MkdirAll(dir, os.ModePerm))
if err := os.MkdirAll(dir, os.ModePerm); err != nil {
t.Fatalf("Error while creaing directory: %s", err.Error())
}
}
}

func dirCleanup() {
x.Check(os.RemoveAll("./data"))
func dirCleanup(t *testing.T) {
if err := os.RemoveAll("./data"); err != nil {
t.Fatalf("Error removing direcotory: %s", err.Error())
}
}

func copyToLocalFs(t *testing.T) {
Expand Down
9 changes: 0 additions & 9 deletions query/common_test.go
Expand Up @@ -24,19 +24,10 @@ import (

"github.com/stretchr/testify/require"

"github.com/dgraph-io/dgo/v2"
"github.com/dgraph-io/dgo/v2/protos/api"
"github.com/dgraph-io/dgraph/testutil"
"github.com/dgraph-io/dgraph/x"
"google.golang.org/grpc"
)

func getNewClient() *dgo.Dgraph {
conn, err := grpc.Dial(testutil.SockAddr, grpc.WithInsecure())
x.Check(err)
return dgo.NewDgraphClient(api.NewDgraphClient(conn))
}

func setSchema(schema string) {
err := client.Alter(context.Background(), &api.Operation{
Schema: schema,
Expand Down
10 changes: 6 additions & 4 deletions systest/21million/run_test.go
Expand Up @@ -30,8 +30,6 @@ import (

"github.com/dgraph-io/dgraph/chunker"
"github.com/dgraph-io/dgraph/testutil"
"github.com/dgraph-io/dgraph/x"

"github.com/stretchr/testify/require"
)

Expand All @@ -54,7 +52,9 @@ func TestQueries(t *testing.T) {
}

files, err := ioutil.ReadDir(queryDir)
x.CheckfNoTrace(err)
if err != nil {
t.Fatalf("Error reading directory: %s", err.Error())
}

savepath := ""
diffs := 0
Expand All @@ -66,7 +66,9 @@ func TestQueries(t *testing.T) {
filename := path.Join(queryDir, file.Name())
reader, cleanup := chunker.FileReader(filename)
bytes, err := ioutil.ReadAll(reader)
x.CheckfNoTrace(err)
if err != nil {
t.Fatalf("Error reading file: %s", err.Error())
}
contents := string(bytes[:])
cleanup()

Expand Down
5 changes: 3 additions & 2 deletions xidmap/xidmap_test.go
Expand Up @@ -124,8 +124,9 @@ func TestXidmapMemory(t *testing.T) {

func BenchmarkXidmap(b *testing.B) {
conn, err := x.SetupConnection(testutil.SockAddrZero, nil, false)
x.Check(err)
x.AssertTrue(conn != nil)
if err != nil {
b.Fatalf("Error setting up connection: %s", err.Error())
}

var counter uint64
xidmap := New(conn, nil)
Expand Down

0 comments on commit 8d76d7d

Please sign in to comment.