From 4a7f2c10780e7415d8c0abc55362f765cdb9fe44 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 29 Aug 2018 16:56:25 -0700 Subject: [PATCH] wtxmgr/query_test: remove t.Fatal calls within db transactions in TestStoreQueries --- wtxmgr/query_test.go | 287 ++++++++++++++++++++++--------------------- 1 file changed, 144 insertions(+), 143 deletions(-) diff --git a/wtxmgr/query_test.go b/wtxmgr/query_test.go index f5a2f50..6a9bb18 100644 --- a/wtxmgr/query_test.go +++ b/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) { - defer func() { - if t.Failed() { - t.Fatalf("Store state queries failed after '%s'", changeDesc) - } - }() +func (q *queryState) compare(s *Store, ns walletdb.ReadBucket, + changeDesc string) error { 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]) - } - if t.Failed() { - return false, fmt.Errorf("Failed comparing range of transaction details") + err := equalTxDetails(&got[i], &exp[i]) + if err != nil { + return false, fmt.Errorf("failed "+ + "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) - continue + return fmt.Errorf("found no matching "+ + "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 t.Failed() { - t.Fatalf("Failed querying latest details regarding transaction %v", txHash) + if err := equalTxDetails(d, detail); err != nil { + return fmt.Errorf("%s: failed querying latest details "+ + "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") - t.Errorf("Got: %v", got.Hash) - t.Errorf("Expected: %v", exp.Hash) + return fmt.Errorf("found mismatched hashes: got %v, expected %v", + got.Hash, exp.Hash) } if got.Received != exp.Received { - t.Errorf("Found mismatched receive time") - t.Errorf("Got: %v", got.Received) - t.Errorf("Expected: %v", exp.Received) + return fmt.Errorf("found mismatched receive time: got %v, "+ + "expected %v", got.Received, exp.Received) } if !bytes.Equal(got.SerializedTx, exp.SerializedTx) { - t.Errorf("Found mismatched serialized txs") - t.Errorf("Got: %x", got.SerializedTx) - t.Errorf("Expected: %x", exp.SerializedTx) + return fmt.Errorf("found mismatched serialized txs: got %v, "+ + "expected %v", got.SerializedTx, exp.SerializedTx) } if got.Block != exp.Block { - t.Errorf("Found mismatched block meta") - t.Errorf("Got: %v", got.Block) - t.Errorf("Expected: %v", exp.Block) + return fmt.Errorf("found mismatched block meta: got %v, "+ + "expected %v", got.Block, 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)) - } else { - for i := range got.Credits { - if got.Credits[i] != exp.Credits[i] { - t.Errorf("Found mismatched Credit[%d]", i) - t.Errorf("Got: %v", got.Credits[i]) - t.Errorf("Expected: %v", exp.Credits[i]) - } + return fmt.Errorf("credit slice lengths differ: got %d, "+ + "expected %d", len(got.Credits), len(exp.Credits)) + } + for i := range got.Credits { + if got.Credits[i] != exp.Credits[i] { + return fmt.Errorf("found mismatched credit[%d]: got %v, "+ + "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)) - } else { - for i := range got.Debits { - if got.Debits[i] != exp.Debits[i] { - t.Errorf("Found mismatched Debit[%d]", i) - t.Errorf("Got: %v", got.Debits[i]) - t.Errorf("Expected: %v", exp.Debits[i]) - } + return fmt.Errorf("debit slice lengths differ: got %d, "+ + "expected %d", len(got.Debits), len(exp.Debits)) + } + for i := range got.Debits { + if got.Debits[i] != exp.Debits[i] { + return fmt.Errorf("found mismatched debit[%d]: got %v, "+ + "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:") - t.Errorf("Got: %v", got) - t.Errorf("Expected: %v", exp) + return fmt.Errorf("found unexpected wire.MsgTx: got: %v, "+ + "expected %v", got, 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", - updates: func(ns walletdb.ReadWriteBucket) { insertTx(ns, recA, nil) }, - state: newState, + desc: "insert tx A unmined", + updates: func(ns walletdb.ReadWriteBucket) error { + 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", - updates: func(ns walletdb.ReadWriteBucket) { addCredit(s, ns, recA, nil, 0, true) }, - state: newState, + desc: "mark unconfirmed txA:0 as credit", + updates: func(ns walletdb.ReadWriteBucket) error { + 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", - updates: func(ns walletdb.ReadWriteBucket) { insertTx(ns, recB, nil) }, - state: newState, + desc: "insert tx B unmined", + updates: func(ns walletdb.ReadWriteBucket) error { + 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", - updates: func(ns walletdb.ReadWriteBucket) { addCredit(s, ns, recB, nil, 0, false) }, - state: newState, + desc: "mark txB:0 as non-change credit", + updates: func(ns walletdb.ReadWriteBucket) error { + 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", - updates: func(ns walletdb.ReadWriteBucket) { insertTx(ns, recA, &b100) }, - state: newState, + desc: "mine tx A", + updates: func(ns walletdb.ReadWriteBucket) error { + 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", - updates: func(ns walletdb.ReadWriteBucket) { insertTx(ns, recB, &b101) }, - state: newState, + desc: "mine tx B", + updates: func(ns walletdb.ReadWriteBucket) error { + 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) - tst.state.compare(t, s, ns, tst.desc) - return nil + if err := tst.updates(ns); err != nil { + return err + } + 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", - updates: func(ns walletdb.ReadWriteBucket) { insertTx(ns, recB, &b100) }, - state: newState, + desc: "move tx B to block 100", + updates: func(ns walletdb.ReadWriteBucket) error { + 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", - updates: func(ns walletdb.ReadWriteBucket) { rollback(ns, b100.Height) }, - state: newState, + desc: "rollback block 100", + updates: func(ns walletdb.ReadWriteBucket) error { + 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", - updates: func(ns walletdb.ReadWriteBucket) { insertTx(ns, recA, &b100); insertTx(ns, recA, nil) }, - state: newState, - }) - newState = lastState.deepCopy() - newState.blocks = [][]TxDetails{ - newState.blocks[0], - {newState.blocks[1][0]}, - {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, + desc: "insert duplicate tx A", + updates: func(ns walletdb.ReadWriteBucket) error { + if err := s.InsertTx(ns, recA, &b100); err != nil { + return err + } + return s.InsertTx(ns, recA, nil) + }, + state: newState, }) for _, tst := range tests { err := walletdb.Update(db, func(tx walletdb.ReadWriteTx) error { ns := tx.ReadWriteBucket(namespaceKey) - tst.updates(ns) - tst.state.compare(t, s, ns, tst.desc) - return nil + if err := tst.updates(ns); err != nil { + return err + } + return tst.state.compare(s, ns, tst.desc) }) if err != nil { t.Fatal(err)