Repository: coinbase/rosetta-cli
Pull request #130 (2020-09-12 00:39:48)

improved error handling and test coverage
Fixes # .

### Motivation
we've updated the Rosetta SDK to include named error types for better visibility on error handling. we're updating the CLI here to leverage that new functionality to improve coverage in block syncing and balance checking tests.

### Solution
we are using the `Err()` functions that the SDK now exposes to check if an error originated from the `storage` or `syncer` packages.

we also make sure to fail noisily by having check:data hard exit if we encounter an error not handled by one of our tests.

### Open questions
<!--
(optional) Any open questions or feedback on design desired?
-->



Patch:
diff --git a/go.sum b/go.sum
index c468c1f..3f7aace 100644
--- a/go.sum
+++ b/go.sum
@@ -352,6 +352,7 @@ github.com/rs/xhandler v0.0.0-20160618193221-ed27b6fd6521/go.mod h1:RvLn4FgxWubr
 github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR/rfWxYHBV53g=
 github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
 github.com/segmentio/golines v0.0.0-20200808004416-0a9796b248e8/go.mod h1:bQSh5qdVR67XiCKbaVvYO41s50c5hQo+3cY/1CQQ3xQ=
+github.com/segmentio/golines v0.0.0-20200824192126-7f30d3046793 h1:rhR7esJSmty+9ST6Gsp7mlQHkpISw2DiYjuFaz3dRDg=
 github.com/segmentio/golines v0.0.0-20200824192126-7f30d3046793/go.mod h1:bQSh5qdVR67XiCKbaVvYO41s50c5hQo+3cY/1CQQ3xQ=
 github.com/sergi/go-diff v1.0.0/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAmXWZgo=
 github.com/sergi/go-diff v1.1.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM=
@@ -446,6 +447,7 @@ golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9 h1:psW17arqaxU48Z5kZ0CQnk
 golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
 golang.org/x/crypto v0.0.0-20200728195943-123391ffb6de h1:ikNHVSjEfnvz6sxdSPCaPt572qowuyMDMJLLm3Db3ig=
 golang.org/x/crypto v0.0.0-20200728195943-123391ffb6de/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
+golang.org/x/crypto v0.0.0-20200820211705-5c72a883971a h1:vclmkQCjlDX5OydZ9wv8rBCcS0QyQY66Mpf/7BZbInM=
 golang.org/x/crypto v0.0.0-20200820211705-5c72a883971a/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
 golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
 golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=
@@ -454,6 +456,7 @@ golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3 h1:XQyxROzUlZH+WIQwySDgnISg
 golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc=
 golang.org/x/mod v0.0.0-20190513183733-4bf6d317e70e/go.mod h1:mXi4GBBbnImb6dmsKGUJ2LatrhH/nqhxcFungHvyanc=
 golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
+golang.org/x/mod v0.3.0 h1:RM4zey1++hCTbCVQfnWeKs9/IEsaBLA8vTkd0WVtmH4=
 golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/net v0.0.0-20180719180050-a680a1efc54d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
@@ -535,6 +538,7 @@ golang.org/x/tools v0.0.0-20191130070609-6e064ea0cf2d/go.mod h1:b+2E5dAYhXwXZwtn
 golang.org/x/tools v0.0.0-20200626171337-aa94e735be7f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
 golang.org/x/tools v0.0.0-20200729181040-64cdafbe085c/go.mod h1:njjCfa9FT2d7l9Bc6FUM5FLjQPp3cFF28FI3qnDFljA=
 golang.org/x/tools v0.0.0-20200731060945-b5fad4ed8dd6/go.mod h1:njjCfa9FT2d7l9Bc6FUM5FLjQPp3cFF28FI3qnDFljA=
+golang.org/x/tools v0.0.0-20200904185747-39188db58858 h1:xLt+iB5ksWcZVxqc+g9K41ZHy+6MKWfXCDsjSThnsPA=
 golang.org/x/tools v0.0.0-20200904185747-39188db58858/go.mod h1:Cj7w3i3Rnn0Xh82ur9kSqwfTHTeVxaDqrfMjpcNT6bE=
 golang.org/x/tools/gopls v0.4.4/go.mod h1:zhyGzA+CAtREUwwq/btQxEx2FHnGzDwJvGs5YqdVCbE=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
diff --git a/pkg/processor/coordinator_helper.go b/pkg/processor/coordinator_helper.go
index 5935d90..8d31138 100644
--- a/pkg/processor/coordinator_helper.go
+++ b/pkg/processor/coordinator_helper.go
@@ -259,7 +259,12 @@ func (c *CoordinatorHelper) StoreKey(
 	// We optimisically add the interesting address although the dbTx could be reverted.
 	c.balanceStorageHelper.AddInterestingAddress(account.Address)
 
-	_, _ = c.counterStorage.UpdateTransactional(ctx, dbTx, storage.AddressesCreatedCounter, big.NewInt(1))
+	_, _ = c.counterStorage.UpdateTransactional(
+		ctx,
+		dbTx,
+		storage.AddressesCreatedCounter,
+		big.NewInt(1),
+	)
 	return c.keyStorage.StoreTransactional(ctx, account, keyPair, dbTx)
 }
 


Test patch:
diff --git a/pkg/tester/data_results.go b/pkg/tester/data_results.go
index b1886dd..9fe79c7 100644
--- a/pkg/tester/data_results.go
+++ b/pkg/tester/data_results.go
@@ -65,8 +65,10 @@ func (c *CheckDataResults) Print() {
 	}
 
 	fmt.Printf("\n")
-	c.Tests.Print()
-	fmt.Printf("\n")
+	if c.Tests != nil {
+		c.Tests.Print()
+		fmt.Printf("\n")
+	}
 	if c.Stats != nil {
 		c.Stats.Print()
 		fmt.Printf("\n")
@@ -297,18 +299,11 @@ func ResponseAssertionTest(err error) bool {
 // indicating if it was possible to sync
 // blocks.
 func BlockSyncingTest(err error, blocksSynced bool) *bool {
-	relatedErrors := []error{
-		syncer.ErrCannotRemoveGenesisBlock,
-		syncer.ErrOutOfOrder,
-		storage.ErrDuplicateKey,
-		storage.ErrDuplicateTransactionHash,
-	}
 	syncPass := true
-	for _, relatedError := range relatedErrors {
-		if errors.Is(err, relatedError) {
-			syncPass = false
-			break
-		}
+	storageFailed, _ := storage.Err(err)
+	if syncer.Err(err) ||
+		(storageFailed && !errors.Is(err, storage.ErrNegativeBalance)) {
+		syncPass = false
 	}
 
 	if !blocksSynced && syncPass {
@@ -322,12 +317,9 @@ func BlockSyncingTest(err error, blocksSynced bool) *bool {
 // indicating if any balances went negative
 // while syncing.
 func BalanceTrackingTest(cfg *configuration.Configuration, err error, operationsSeen bool) *bool {
-	relatedErrors := []error{
-		storage.ErrNegativeBalance,
-	}
 	balancePass := true
-	for _, relatedError := range relatedErrors {
-		if errors.Is(err, relatedError) {
+	for _, balanceStorageErr := range storage.BalanceStorageErrs {
+		if errors.Is(err, balanceStorageErr) {
 			balancePass = false
 			break
 		}
@@ -431,6 +423,17 @@ func ComputeCheckDataResults(
 	if err != nil {
 		results.Error = err.Error()
 
+		// If all tests pass, but we still encountered an error,
+		// then we hard exit without showing check:data results
+		// because the error falls beyond our test coverage.
+		if tests.RequestResponse &&
+			tests.ResponseAssertion &&
+			(tests.BlockSyncing == nil || *tests.BlockSyncing) &&
+			(tests.BalanceTracking == nil || *tests.BalanceTracking) &&
+			(tests.Reconciliation == nil || *tests.Reconciliation) {
+			results.Tests = nil
+		}
+
 		// We never want to populate an end condition
 		// if there was an error!
 		return results
diff --git a/pkg/tester/data_results_test.go b/pkg/tester/data_results_test.go
index c40ab10..465266e 100644
--- a/pkg/tester/data_results_test.go
+++ b/pkg/tester/data_results_test.go
@@ -16,6 +16,7 @@ package tester
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"math/big"
 	"path"
@@ -289,6 +290,30 @@ func TestComputeCheckDataResults(t *testing.T) {
 				},
 			},
 		},
+		"default configuration, no storage, unknown errors": {
+			cfg:    configuration.DefaultConfiguration(),
+			err:    []error{errors.New("unsure how to handle this error")},
+			result: &CheckDataResults{},
+		},
+		"default configuration, counter storage no blocks, unknown errors": {
+			cfg:                   configuration.DefaultConfiguration(),
+			provideCounterStorage: true,
+			err:                   []error{errors.New("unsure how to handle this error")},
+			result: &CheckDataResults{
+				Stats: &CheckDataStats{},
+			},
+		},
+		"default configuration, counter storage with blocks, unknown errors": {
+			cfg:                   configuration.DefaultConfiguration(),
+			provideCounterStorage: true,
+			blockCount:            100,
+			err:                   []error{errors.New("unsure how to handle this error")},
+			result: &CheckDataResults{
+				Stats: &CheckDataStats{
+					Blocks: 100,
+				},
+			},
+		},
 	}
 
 	for name, test := range tests {