wtxmgr/query_test: remove t.Fatal calls within db transactions in TestStoreQueries

wtxmgr/query_test.go

@@ -7,6 +7,7 @@ package wtxmgr_test
 import (
 	"bytes"
 	"encoding/binary"
+	"errors"
 	"fmt"
 	"testing"
 	"time"
@@ -64,12 +65,8 @@ func deepCopyTxDetails(d *TxDetails) *TxDetails {
 	return &cpy
 }
 
-func (q *queryState) compare(t *testing.T, s *Store, ns walletdb.ReadBucket, changeDesc string) {
+func (q *queryState) compare(s *Store, ns walletdb.ReadBucket,
-	defer func() {
+	changeDesc string) error {
-		if t.Failed() {
-			t.Fatalf("Store state queries failed after '%s'", changeDesc)
-		}
-	}()
 
 	fwdBlocks := q.blocks
 	revBlocks := make([][]TxDetails, len(q.blocks))
@@ -80,17 +77,22 @@ func (q *queryState) compare(t *testing.T, s *Store, ns walletdb.ReadBucket, cha
 	checkBlock := func(blocks [][]TxDetails) func([]TxDetails) (bool, error) {
 		return func(got []TxDetails) (bool, error) {
 			if len(fwdBlocks) == 0 {
-				return false, fmt.Errorf("entered range when no more details expected")
+				return false, errors.New("entered range " +
+					"when no more details expected")
 			}
 			exp := blocks[0]
 			if len(got) != len(exp) {
-				return false, fmt.Errorf("got len(details)=%d in transaction range, expected %d", len(got), len(exp))
+				return false, fmt.Errorf("got len(details)=%d "+
+					"in transaction range, expected %d",
+					len(got), len(exp))
 			}
 			for i := range got {
-				equalTxDetails(t, &got[i], &exp[i])
+				err := equalTxDetails(&got[i], &exp[i])
-			}
+				if err != nil {
-			if t.Failed() {
+					return false, fmt.Errorf("failed "+
-				return false, fmt.Errorf("Failed comparing range of transaction details")
+						"comparing range of "+
+						"transaction details: %v", err)
+				}
 			}
 			blocks = blocks[1:]
 			return false, nil
@@ -98,11 +100,13 @@ func (q *queryState) compare(t *testing.T, s *Store, ns walletdb.ReadBucket, cha
 	}
 	err := s.RangeTransactions(ns, 0, -1, checkBlock(fwdBlocks))
 	if err != nil {
-		t.Fatalf("Failed in RangeTransactions (forwards iteration): %v", err)
+		return fmt.Errorf("%s: failed in RangeTransactions (forwards "+
+			"iteration): %v", changeDesc, err)
 	}
 	err = s.RangeTransactions(ns, -1, 0, checkBlock(revBlocks))
 	if err != nil {
-		t.Fatalf("Failed in RangeTransactions (reverse iteration): %v", err)
+		return fmt.Errorf("%s: failed in RangeTransactions (reverse "+
+			"iteration): %v", changeDesc, err)
 	}
 
 	for txHash, details := range q.txDetails {
@@ -113,16 +117,18 @@ func (q *queryState) compare(t *testing.T, s *Store, ns walletdb.ReadBucket, cha
 			}
 			d, err := s.UniqueTxDetails(ns, &txHash, blk)
 			if err != nil {
-				t.Fatal(err)
+				return err
 			}
 			if d == nil {
-				t.Errorf("Found no matching transaction at height %d", detail.Block.Height)
+				return fmt.Errorf("found no matching "+
-				continue
+					"transaction at height %d",
+					detail.Block.Height)
+			}
+			if err := equalTxDetails(d, &detail); err != nil {
+				return fmt.Errorf("%s: failed querying latest "+
+					"details regarding transaction %v",
+					changeDesc, txHash)
 			}
-			equalTxDetails(t, d, &detail)
-		}
-		if t.Failed() {
-			t.Fatalf("Failed querying unique details regarding transaction %v", txHash)
 		}
 
 		// For the most recent tx with this hash, check that
@@ -131,79 +137,80 @@ func (q *queryState) compare(t *testing.T, s *Store, ns walletdb.ReadBucket, cha
 		detail := &details[len(details)-1]
 		d, err := s.TxDetails(ns, &txHash)
 		if err != nil {
-			t.Fatal(err)
+			return err
 		}
-		equalTxDetails(t, d, detail)
+		if err := equalTxDetails(d, detail); err != nil {
-		if t.Failed() {
+			return fmt.Errorf("%s: failed querying latest details "+
-			t.Fatalf("Failed querying latest details regarding transaction %v", txHash)
+				"regarding transaction %v", changeDesc, txHash)
 		}
 	}
+
+	return nil
 }
 
-func equalTxDetails(t *testing.T, got, exp *TxDetails) {
+func equalTxDetails(got, exp *TxDetails) error {
 	// Need to avoid using reflect.DeepEqual against slices, since it
 	// returns false for nil vs non-nil zero length slices.
+	if err := equalTxs(&got.MsgTx, &exp.MsgTx); err != nil {
+		return err
+	}
 
-	equalTxs(t, &got.MsgTx, &exp.MsgTx)
 	if got.Hash != exp.Hash {
-		t.Errorf("Found mismatched hashes")
+		return fmt.Errorf("found mismatched hashes: got %v, expected %v",
-		t.Errorf("Got: %v", got.Hash)
+			got.Hash, exp.Hash)
-		t.Errorf("Expected: %v", exp.Hash)
 	}
 	if got.Received != exp.Received {
-		t.Errorf("Found mismatched receive time")
+		return fmt.Errorf("found mismatched receive time: got %v, "+
-		t.Errorf("Got: %v", got.Received)
+			"expected %v", got.Received, exp.Received)
-		t.Errorf("Expected: %v", exp.Received)
 	}
 	if !bytes.Equal(got.SerializedTx, exp.SerializedTx) {
-		t.Errorf("Found mismatched serialized txs")
+		return fmt.Errorf("found mismatched serialized txs: got %v, "+
-		t.Errorf("Got: %x", got.SerializedTx)
+			"expected %v", got.SerializedTx, exp.SerializedTx)
-		t.Errorf("Expected: %x", exp.SerializedTx)
 	}
 	if got.Block != exp.Block {
-		t.Errorf("Found mismatched block meta")
+		return fmt.Errorf("found mismatched block meta: got %v, "+
-		t.Errorf("Got: %v", got.Block)
+			"expected %v", got.Block, exp.Block)
-		t.Errorf("Expected: %v", exp.Block)
 	}
 	if len(got.Credits) != len(exp.Credits) {
-		t.Errorf("Credit slice lengths differ: Got %d Expected %d", len(got.Credits), len(exp.Credits))
+		return fmt.Errorf("credit slice lengths differ: got %d, "+
-	} else {
+			"expected %d", len(got.Credits), len(exp.Credits))
-		for i := range got.Credits {
+	}
-			if got.Credits[i] != exp.Credits[i] {
+	for i := range got.Credits {
-				t.Errorf("Found mismatched Credit[%d]", i)
+		if got.Credits[i] != exp.Credits[i] {
-				t.Errorf("Got: %v", got.Credits[i])
+			return fmt.Errorf("found mismatched credit[%d]: got %v, "+
-				t.Errorf("Expected: %v", exp.Credits[i])
+				"expected %v", i, got.Credits[i], exp.Credits[i])
-			}
 		}
 	}
 	if len(got.Debits) != len(exp.Debits) {
-		t.Errorf("Debit slice lengths differ: Got %d Expected %d", len(got.Debits), len(exp.Debits))
+		return fmt.Errorf("debit slice lengths differ: got %d, "+
-	} else {
+			"expected %d", len(got.Debits), len(exp.Debits))
-		for i := range got.Debits {
+	}
-			if got.Debits[i] != exp.Debits[i] {
+	for i := range got.Debits {
-				t.Errorf("Found mismatched Debit[%d]", i)
+		if got.Debits[i] != exp.Debits[i] {
-				t.Errorf("Got: %v", got.Debits[i])
+			return fmt.Errorf("found mismatched debit[%d]: got %v, "+
-				t.Errorf("Expected: %v", exp.Debits[i])
+				"expected %v", i, got.Debits[i], exp.Debits[i])
-			}
 		}
 	}
+
+	return nil
 }
 
-func equalTxs(t *testing.T, got, exp *wire.MsgTx) {
+func equalTxs(got, exp *wire.MsgTx) error {
 	var bufGot, bufExp bytes.Buffer
 	err := got.Serialize(&bufGot)
 	if err != nil {
-		t.Fatal(err)
+		return err
 	}
 	err = exp.Serialize(&bufExp)
 	if err != nil {
-		t.Fatal(err)
+		return err
 	}
 	if !bytes.Equal(bufGot.Bytes(), bufExp.Bytes()) {
-		t.Errorf("Found unexpected wire.MsgTx:")
+		return fmt.Errorf("found unexpected wire.MsgTx: got: %v, "+
-		t.Errorf("Got: %v", got)
+			"expected %v", got, exp)
-		t.Errorf("Expected: %v", exp)
 	}
+
+	return nil
 }
 
 // Returns time.Now() with seconds resolution, this is what Store saves.
@@ -238,7 +245,7 @@ func TestStoreQueries(t *testing.T) {
 
 	type queryTest struct {
 		desc    string
-		updates func(ns walletdb.ReadWriteBucket) // Unwinds from t.Fatal if the update errors.
+		updates func(ns walletdb.ReadWriteBucket) error
 		state   *queryState
 	}
 	var tests []queryTest
@@ -252,40 +259,16 @@ func TestStoreQueries(t *testing.T) {
 	lastState := newQueryState()
 	tests = append(tests, queryTest{
 		desc:    "initial store",
-		updates: func(walletdb.ReadWriteBucket) {},
+		updates: func(walletdb.ReadWriteBucket) error { return nil },
 		state:   lastState,
 	})
 
-	// simplify error handling
-	insertTx := func(ns walletdb.ReadWriteBucket, rec *TxRecord, block *BlockMeta) {
-		err := s.InsertTx(ns, rec, block)
-		if err != nil {
-			t.Fatal(err)
-		}
-	}
-	addCredit := func(s *Store, ns walletdb.ReadWriteBucket, rec *TxRecord, block *BlockMeta, index uint32, change bool) {
-		err := s.AddCredit(ns, rec, block, index, change)
-		if err != nil {
-			t.Fatal(err)
-		}
-	}
-	newTxRecordFromMsgTx := func(tx *wire.MsgTx, received time.Time) *TxRecord {
-		rec, err := NewTxRecordFromMsgTx(tx, received)
-		if err != nil {
-			t.Fatal(err)
-		}
-		return rec
-	}
-	rollback := func(ns walletdb.ReadWriteBucket, height int32) {
-		err := s.Rollback(ns, height)
-		if err != nil {
-			t.Fatal(err)
-		}
-	}
-
 	// Insert an unmined transaction.  Mark no credits yet.
 	txA := spendOutput(&chainhash.Hash{}, 0, 100e8)
-	recA := newTxRecordFromMsgTx(txA, timeNow())
+	recA, err := NewTxRecordFromMsgTx(txA, timeNow())
+	if err != nil {
+		t.Fatal(err)
+	}
 	newState := lastState.deepCopy()
 	newState.blocks = [][]TxDetails{
 		{
@@ -300,9 +283,11 @@ func TestStoreQueries(t *testing.T) {
 	}
 	lastState = newState
 	tests = append(tests, queryTest{
-		desc:    "insert tx A unmined",
+		desc: "insert tx A unmined",
-		updates: func(ns walletdb.ReadWriteBucket) { insertTx(ns, recA, nil) },
+		updates: func(ns walletdb.ReadWriteBucket) error {
-		state:   newState,
+			return s.InsertTx(ns, recA, nil)
+		},
+		state: newState,
 	})
 
 	// Add txA:0 as a change credit.
@@ -318,15 +303,20 @@ func TestStoreQueries(t *testing.T) {
 	newState.txDetails[recA.Hash][0].Credits = newState.blocks[0][0].Credits
 	lastState = newState
 	tests = append(tests, queryTest{
-		desc:    "mark unconfirmed txA:0 as credit",
+		desc: "mark unconfirmed txA:0 as credit",
-		updates: func(ns walletdb.ReadWriteBucket) { addCredit(s, ns, recA, nil, 0, true) },
+		updates: func(ns walletdb.ReadWriteBucket) error {
-		state:   newState,
+			return s.AddCredit(ns, recA, nil, 0, true)
+		},
+		state: newState,
 	})
 
 	// Insert another unmined transaction which spends txA:0, splitting the
 	// amount into outputs of 40 and 60 BTC.
 	txB := spendOutput(&recA.Hash, 0, 40e8, 60e8)
-	recB := newTxRecordFromMsgTx(txB, timeNow())
+	recB, err := NewTxRecordFromMsgTx(txB, timeNow())
+	if err != nil {
+		t.Fatal(err)
+	}
 	newState = lastState.deepCopy()
 	newState.blocks[0][0].Credits[0].Spent = true
 	newState.blocks[0] = append(newState.blocks[0], TxDetails{
@@ -343,9 +333,11 @@ func TestStoreQueries(t *testing.T) {
 	newState.txDetails[recB.Hash] = []TxDetails{newState.blocks[0][1]}
 	lastState = newState
 	tests = append(tests, queryTest{
-		desc:    "insert tx B unmined",
+		desc: "insert tx B unmined",
-		updates: func(ns walletdb.ReadWriteBucket) { insertTx(ns, recB, nil) },
+		updates: func(ns walletdb.ReadWriteBucket) error {
-		state:   newState,
+			return s.InsertTx(ns, recB, nil)
+		},
+		state: newState,
 	})
 	newState = lastState.deepCopy()
 	newState.blocks[0][1].Credits = []CreditRecord{
@@ -359,9 +351,11 @@ func TestStoreQueries(t *testing.T) {
 	newState.txDetails[recB.Hash][0].Credits = newState.blocks[0][1].Credits
 	lastState = newState
 	tests = append(tests, queryTest{
-		desc:    "mark txB:0 as non-change credit",
+		desc: "mark txB:0 as non-change credit",
-		updates: func(ns walletdb.ReadWriteBucket) { addCredit(s, ns, recB, nil, 0, false) },
+		updates: func(ns walletdb.ReadWriteBucket) error {
-		state:   newState,
+			return s.AddCredit(ns, recB, nil, 0, false)
+		},
+		state: newState,
 	})
 
 	// Mine tx A at block 100.  Leave tx B unmined.
@@ -373,9 +367,11 @@ func TestStoreQueries(t *testing.T) {
 	newState.txDetails[recA.Hash][0].Block = b100
 	lastState = newState
 	tests = append(tests, queryTest{
-		desc:    "mine tx A",
+		desc: "mine tx A",
-		updates: func(ns walletdb.ReadWriteBucket) { insertTx(ns, recA, &b100) },
+		updates: func(ns walletdb.ReadWriteBucket) error {
-		state:   newState,
+			return s.InsertTx(ns, recA, &b100)
+		},
+		state: newState,
 	})
 
 	// Mine tx B at block 101.
@@ -385,17 +381,20 @@ func TestStoreQueries(t *testing.T) {
 	newState.txDetails[recB.Hash][0].Block = b101
 	lastState = newState
 	tests = append(tests, queryTest{
-		desc:    "mine tx B",
+		desc: "mine tx B",
-		updates: func(ns walletdb.ReadWriteBucket) { insertTx(ns, recB, &b101) },
+		updates: func(ns walletdb.ReadWriteBucket) error {
-		state:   newState,
+			return s.InsertTx(ns, recB, &b101)
+		},
+		state: newState,
 	})
 
 	for _, tst := range tests {
 		err := walletdb.Update(db, func(tx walletdb.ReadWriteTx) error {
 			ns := tx.ReadWriteBucket(namespaceKey)
-			tst.updates(ns)
+			if err := tst.updates(ns); err != nil {
-			tst.state.compare(t, s, ns, tst.desc)
+				return err
-			return nil
+			}
+			return tst.state.compare(s, ns, tst.desc)
 		})
 		if err != nil {
 			t.Fatal(err)
@@ -414,14 +413,18 @@ func TestStoreQueries(t *testing.T) {
 		ns := tx.ReadWriteBucket(namespaceKey)
 
 		missingTx := spendOutput(&recB.Hash, 0, 40e8)
-		missingRec := newTxRecordFromMsgTx(missingTx, timeNow())
+		missingRec, err := NewTxRecordFromMsgTx(missingTx, timeNow())
+		if err != nil {
+			return err
+		}
 		missingBlock := makeBlockMeta(102)
 		missingDetails, err := s.TxDetails(ns, &missingRec.Hash)
 		if err != nil {
-			t.Fatal(err)
+			return err
 		}
 		if missingDetails != nil {
-			t.Errorf("Expected no details, found details for tx %v", missingDetails.Hash)
+			return fmt.Errorf("Expected no details, found details "+
+				"for tx %v", missingDetails.Hash)
 		}
 		missingUniqueTests := []struct {
 			hash  *chainhash.Hash
@@ -461,7 +464,9 @@ func TestStoreQueries(t *testing.T) {
 			t.Errorf("RangeTransactions (reverse) ran func %d times", iterations)
 		}
 		// Make sure it also breaks early after one iteration through unmined transactions.
-		rollback(ns, b101.Height)
+		if err := s.Rollback(ns, b101.Height); err != nil {
+			return err
+		}
 		iterations = 0
 		err = s.RangeTransactions(ns, -1, 0, func([]TxDetails) (bool, error) {
 			iterations++
@@ -487,9 +492,11 @@ func TestStoreQueries(t *testing.T) {
 	newState.txDetails[recB.Hash][0].Block = b100
 	lastState = newState
 	tests = append(tests[:0:0], queryTest{
-		desc:    "move tx B to block 100",
+		desc: "move tx B to block 100",
-		updates: func(ns walletdb.ReadWriteBucket) { insertTx(ns, recB, &b100) },
+		updates: func(ns walletdb.ReadWriteBucket) error {
-		state:   newState,
+			return s.InsertTx(ns, recB, &b100)
+		},
+		state: newState,
 	})
 	newState = lastState.deepCopy()
 	newState.blocks[0][0].Block = makeBlockMeta(-1)
@@ -498,9 +505,11 @@ func TestStoreQueries(t *testing.T) {
 	newState.txDetails[recB.Hash][0].Block = makeBlockMeta(-1)
 	lastState = newState
 	tests = append(tests, queryTest{
-		desc:    "rollback block 100",
+		desc: "rollback block 100",
-		updates: func(ns walletdb.ReadWriteBucket) { rollback(ns, b100.Height) },
+		updates: func(ns walletdb.ReadWriteBucket) error {
-		state:   newState,
+			return s.Rollback(ns, b100.Height)
+		},
+		state: newState,
 	})
 
 	// None of the above tests have tested transactions with colliding
@@ -525,31 +534,23 @@ func TestStoreQueries(t *testing.T) {
 	newState.txDetails[recA.Hash] = append(newState.txDetails[recA.Hash], newState.blocks[1][0])
 	lastState = newState
 	tests = append(tests, queryTest{
-		desc:    "insert duplicate tx A",
+		desc: "insert duplicate tx A",
-		updates: func(ns walletdb.ReadWriteBucket) { insertTx(ns, recA, &b100); insertTx(ns, recA, nil) },
+		updates: func(ns walletdb.ReadWriteBucket) error {
-		state:   newState,
+			if err := s.InsertTx(ns, recA, &b100); err != nil {
-	})
+				return err
-	newState = lastState.deepCopy()
+			}
-	newState.blocks = [][]TxDetails{
+			return s.InsertTx(ns, recA, nil)
-		newState.blocks[0],
+		},
-		{newState.blocks[1][0]},
+		state: newState,
-		{newState.blocks[1][1]},
-	}
-	newState.blocks[1][0].Block = b101
-	newState.txDetails[recA.Hash][1].Block = b101
-	lastState = newState
-	tests = append(tests, queryTest{
-		desc:    "mine duplicate tx A",
-		updates: func(ns walletdb.ReadWriteBucket) { insertTx(ns, recA, &b101) },
-		state:   newState,
 	})
 
 	for _, tst := range tests {
 		err := walletdb.Update(db, func(tx walletdb.ReadWriteTx) error {
 			ns := tx.ReadWriteBucket(namespaceKey)
-			tst.updates(ns)
+			if err := tst.updates(ns); err != nil {
-			tst.state.compare(t, s, ns, tst.desc)
+				return err
-			return nil
+			}
+			return tst.state.compare(s, ns, tst.desc)
 		})
 		if err != nil {
 			t.Fatal(err)