From 574bbe5eba299b31bfc00d804bd9c3d951cfd24e Mon Sep 17 00:00:00 2001 From: Andras Banki-Horvath Date: Fri, 5 Jun 2020 15:02:06 +0200 Subject: [PATCH 1/2] invoices+test: replace DeepEqual + spew dump with testify This commit replaces reflect.DeepEqual tests and spew.Dump prints with testify's require.Equal to make diffs smaller and test outputs more readable. --- channeldb/invoice_test.go | 94 +++++++++++++++++++-------------------- 1 file changed, 47 insertions(+), 47 deletions(-) diff --git a/channeldb/invoice_test.go b/channeldb/invoice_test.go index 537480e97..b658f36a6 100644 --- a/channeldb/invoice_test.go +++ b/channeldb/invoice_test.go @@ -2,16 +2,16 @@ package channeldb import ( "crypto/rand" + "fmt" "math" - "reflect" "testing" "time" - "github.com/davecgh/go-spew/spew" "github.com/lightningnetwork/lnd/lntypes" "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/record" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) var ( @@ -185,10 +185,11 @@ func testInvoiceWorkflow(t *testing.T, test invWorkflowTest) { } return } - if !reflect.DeepEqual(*fakeInvoice, dbInvoice) { - t.Fatalf("invoice fetched from db doesn't match original %v vs %v", - spew.Sdump(fakeInvoice), spew.Sdump(dbInvoice)) - } + + require.Equal(t, + *fakeInvoice, dbInvoice, + "invoice fetched from db doesn't match original", + ) // The add index of the invoice retrieved from the database should now // be fully populated. As this is the first index written to the DB, @@ -280,11 +281,10 @@ func testInvoiceWorkflow(t *testing.T, test invWorkflowTest) { // order (and the primary key should be incremented with each // insertion). for i := 0; i < len(invoices); i++ { - if !reflect.DeepEqual(*invoices[i], response.Invoices[i]) { - t.Fatalf("retrieved invoices don't match %v vs %v", - spew.Sdump(invoices[i]), - spew.Sdump(response.Invoices[i])) - } + assert.Equal(t, + *invoices[i], response.Invoices[i], + "retrieved invoice doesn't match", + ) } } @@ -501,9 +501,13 @@ func TestInvoiceAddTimeSeries(t *testing.T) { t.Fatalf("unable to query: %v", err) } - if !reflect.DeepEqual(query.resp, resp) { - t.Fatalf("test #%v: expected %v, got %v", i, - spew.Sdump(query.resp), spew.Sdump(resp)) + require.Equal(t, len(query.resp), len(resp)) + + for j := 0; j < len(query.resp); j++ { + require.Equal(t, + query.resp[j], resp[j], + fmt.Sprintf("test: #%v, item: #%v", i, j), + ) } } @@ -566,9 +570,13 @@ func TestInvoiceAddTimeSeries(t *testing.T) { t.Fatalf("unable to query: %v", err) } - if !reflect.DeepEqual(query.resp, resp) { - t.Fatalf("test #%v: expected %v, got %v", i, - spew.Sdump(query.resp), spew.Sdump(resp)) + require.Equal(t, len(query.resp), len(resp)) + + for j := 0; j < len(query.resp); j++ { + require.Equal(t, + query.resp[j], resp[j], + fmt.Sprintf("test: #%v, item: #%v", i, j), + ) } } } @@ -654,14 +662,11 @@ func TestFetchAllInvoicesWithPaymentHash(t *testing.T) { pendingInvoices[i].PaymentHash) } - // Zero out add index to not confuse DeepEqual. + // Zero out add index to not confuse require.Equal. pendingInvoices[i].Invoice.AddIndex = 0 expected.AddIndex = 0 - if !reflect.DeepEqual(*expected, pendingInvoices[i].Invoice) { - t.Fatalf("expected: %v, got: %v", - spew.Sdump(expected), spew.Sdump(pendingInvoices[i].Invoice)) - } + require.Equal(t, *expected, pendingInvoices[i].Invoice) } for i := range allInvoices { @@ -671,14 +676,11 @@ func TestFetchAllInvoicesWithPaymentHash(t *testing.T) { allInvoices[i].PaymentHash) } - // Zero out add index to not confuse DeepEqual. + // Zero out add index to not confuse require.Equal. allInvoices[i].Invoice.AddIndex = 0 expected.AddIndex = 0 - if !reflect.DeepEqual(*expected, allInvoices[i].Invoice) { - t.Fatalf("expected: %v, got: %v", - spew.Sdump(expected), spew.Sdump(allInvoices[i].Invoice)) - } + require.Equal(t, *expected, allInvoices[i].Invoice) } } @@ -732,10 +734,7 @@ func TestDuplicateSettleInvoice(t *testing.T) { } // We should get back the exact same invoice that we just inserted. - if !reflect.DeepEqual(dbInvoice, invoice) { - t.Fatalf("wrong invoice after settle, expected %v got %v", - spew.Sdump(invoice), spew.Sdump(dbInvoice)) - } + require.Equal(t, invoice, dbInvoice, "wrong invoice after settle") // If we try to settle the invoice again, then we should get the very // same invoice back, but with an error this time. @@ -749,10 +748,7 @@ func TestDuplicateSettleInvoice(t *testing.T) { } invoice.SettleDate = dbInvoice.SettleDate - if !reflect.DeepEqual(dbInvoice, invoice) { - t.Fatalf("wrong invoice after second settle, expected %v got %v", - spew.Sdump(invoice), spew.Sdump(dbInvoice)) - } + require.Equal(t, invoice, dbInvoice, "wrong invoice after second settle") } // TestQueryInvoices ensures that we can properly query the invoice database for @@ -1020,11 +1016,13 @@ func TestQueryInvoices(t *testing.T) { t.Fatalf("unable to query invoice database: %v", err) } - if !reflect.DeepEqual(response.Invoices, testCase.expected) { - t.Fatalf("test #%d: query returned incorrect set of "+ - "invoices: expcted %v, got %v", i, - spew.Sdump(response.Invoices), - spew.Sdump(testCase.expected)) + require.Equal(t, len(testCase.expected), len(response.Invoices)) + + for j, expected := range testCase.expected { + require.Equal(t, + expected, response.Invoices[j], + fmt.Sprintf("test: #%v, item: #%v", i, j), + ) } } } @@ -1118,9 +1116,11 @@ func TestCustomRecords(t *testing.T) { if len(dbInvoice.Htlcs) != 1 { t.Fatalf("expected the htlc to be added") } - if !reflect.DeepEqual(records, dbInvoice.Htlcs[key].CustomRecords) { - t.Fatalf("invalid custom records") - } + + require.Equal(t, + records, dbInvoice.Htlcs[key].CustomRecords, + "invalid custom records", + ) } // TestInvoiceRef asserts that the proper identifiers are returned from an @@ -1132,12 +1132,12 @@ func TestInvoiceRef(t *testing.T) { // An InvoiceRef by hash should return the provided hash and a nil // payment addr. refByHash := InvoiceRefByHash(payHash) - assert.Equal(t, payHash, refByHash.PayHash()) - assert.Equal(t, (*[32]byte)(nil), refByHash.PayAddr()) + require.Equal(t, payHash, refByHash.PayHash()) + require.Equal(t, (*[32]byte)(nil), refByHash.PayAddr()) // An InvoiceRef by hash and addr should return the payment hash and // payment addr passed to the constructor. refByHashAndAddr := InvoiceRefByHashAndAddr(payHash, payAddr) - assert.Equal(t, payHash, refByHashAndAddr.PayHash()) - assert.Equal(t, &payAddr, refByHashAndAddr.PayAddr()) + require.Equal(t, payHash, refByHashAndAddr.PayHash()) + require.Equal(t, &payAddr, refByHashAndAddr.PayAddr()) } From 80012c3936baf242621ecd0c1175180e5196a167 Mon Sep 17 00:00:00 2001 From: Andras Banki-Horvath Date: Fri, 5 Jun 2020 15:12:01 +0200 Subject: [PATCH 2/2] invoice+test: changing testitfy asserts to specific requires --- channeldb/invoice_test.go | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/channeldb/invoice_test.go b/channeldb/invoice_test.go index b658f36a6..eea9df034 100644 --- a/channeldb/invoice_test.go +++ b/channeldb/invoice_test.go @@ -10,7 +10,6 @@ import ( "github.com/lightningnetwork/lnd/lntypes" "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/record" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -281,7 +280,7 @@ func testInvoiceWorkflow(t *testing.T, test invWorkflowTest) { // order (and the primary key should be incremented with each // insertion). for i := 0; i < len(invoices); i++ { - assert.Equal(t, + require.Equal(t, *invoices[i], response.Invoices[i], "retrieved invoice doesn't match", ) @@ -293,25 +292,25 @@ func testInvoiceWorkflow(t *testing.T, test invWorkflowTest) { func TestAddDuplicatePayAddr(t *testing.T) { db, cleanUp, err := makeTestDB() defer cleanUp() - assert.Nil(t, err) + require.NoError(t, err) // Create two invoices with the same payment addr. invoice1, err := randInvoice(1000) - assert.Nil(t, err) + require.NoError(t, err) invoice2, err := randInvoice(20000) - assert.Nil(t, err) + require.NoError(t, err) invoice2.Terms.PaymentAddr = invoice1.Terms.PaymentAddr // First insert should succeed. inv1Hash := invoice1.Terms.PaymentPreimage.Hash() _, err = db.AddInvoice(invoice1, inv1Hash) - assert.Nil(t, err) + require.NoError(t, err) // Second insert should fail with duplicate payment addr. inv2Hash := invoice2.Terms.PaymentPreimage.Hash() _, err = db.AddInvoice(invoice2, inv2Hash) - assert.Equal(t, ErrDuplicatePayAddr, err) + require.Error(t, err, ErrDuplicatePayAddr) } // TestInvRefEquivocation asserts that retrieving or updating an invoice using @@ -319,29 +318,29 @@ func TestAddDuplicatePayAddr(t *testing.T) { func TestInvRefEquivocation(t *testing.T) { db, cleanUp, err := makeTestDB() defer cleanUp() - assert.Nil(t, err) + require.NoError(t, err) // Add two random invoices. invoice1, err := randInvoice(1000) - assert.Nil(t, err) + require.NoError(t, err) inv1Hash := invoice1.Terms.PaymentPreimage.Hash() _, err = db.AddInvoice(invoice1, inv1Hash) - assert.Nil(t, err) + require.NoError(t, err) invoice2, err := randInvoice(2000) - assert.Nil(t, err) + require.NoError(t, err) inv2Hash := invoice2.Terms.PaymentPreimage.Hash() _, err = db.AddInvoice(invoice2, inv2Hash) - assert.Nil(t, err) + require.NoError(t, err) // Now, query using invoice 1's payment address, but invoice 2's payment // hash. We expect an error since the invref points to multiple // invoices. ref := InvoiceRefByHashAndAddr(inv2Hash, invoice1.Terms.PaymentAddr) _, err = db.LookupInvoice(ref) - assert.Equal(t, ErrInvRefEquivocation, err) + require.Error(t, err, ErrInvRefEquivocation) // The same error should be returned when updating an equivocating // reference. @@ -349,7 +348,7 @@ func TestInvRefEquivocation(t *testing.T) { return nil, nil } _, err = db.UpdateInvoice(ref, nop) - assert.Equal(t, ErrInvRefEquivocation, err) + require.Error(t, err, ErrInvRefEquivocation) } // TestInvoiceCancelSingleHtlc tests that a single htlc can be canceled on the @@ -438,7 +437,7 @@ func TestInvoiceAddTimeSeries(t *testing.T) { } _, err = db.InvoicesAddedSince(0) - assert.Nil(t, err) + require.NoError(t, err) // We'll start off by creating 20 random invoices, and inserting them // into the database. @@ -512,7 +511,7 @@ func TestInvoiceAddTimeSeries(t *testing.T) { } _, err = db.InvoicesSettledSince(0) - assert.Nil(t, err) + require.NoError(t, err) var settledInvoices []Invoice var settleIndex uint64 = 1