testing (rpc/modules): migrate current tests to integration tests, write new unit tests (ChainSafe#1957)

* fix TestAuthorModule_InsertKey_Valid

* fix TestAuthorModule_InsertKey_Valid_Gran_Keytype

* fix rpc/module/author_integration_test

* migrate rpc/module tests to integration tests TODO:write unit tests

* remove skip author integration tests from golangci.yml

* lint

* rebase and fix errors

* license

* testing (rpc/modules): write new unit tests to increase code coverage (ChainSafe#1995)

* migrate rpc/module tests to integration tests TODO:write unit tests

* fix TestAuthorModule_InsertKey_Valid

* fix TestAuthorModule_InsertKey_Valid_Gran_Keytype

* migrate rpc/module tests to integration tests TODO:write unit tests

* use mockery to generate BlockProducerAPI

* WIP/test for dev control function

* 100% test coverage for rpc/module/dev unit tests

* test cov for getKeys in childState

* 100% cover for getStorage size

* 100% cover for getStorage hash

* 100% coverage for childstate unit tests

* format childState

* lint

* tests for offchain get

* full coverage for offchain unit tests

* unit tests for payment

* 100% for grandpa proveFinality

* grandpa unit tests

* GenSyncSpec coverage

* sync state unit tests

* GetBlock

* getBlockHash tests

* chain tests

* Wip/system tests

* AccountNextIndex test

* syncState test

* tes localListenAndAddresses

* test localPeerId

* system tests

* state getPairs test

* getKeysPages test

* test GetMetadata

* test GetReadProof

* test GetRuntime Version

* test GetStorage

* test GetStorageHash

* test GetStorageSize

* test queryStorage

* state tests

* remove subscribe func tests

* format

* rebase

* fix naming and remove unnecessary variable inits

* remove log

* push test data

* remove AnythingOfType from chain test

* remove AnythingOfType from childState test

* remove AnythingOfType from Grandpa test

* remove AnythingOfType from offchain test

* remove AnythingOfType from payment test

* remove mocking anything in state test

* remove mocking anything in sync state test

* WIP/Finish removing mock.AnythingOfType

* WIP/author test

* author test

* add exp cases to chain test

* WIP/figure out error assertion issue

* WIP/fix errors still

* fix chain test error handling

* author module tests fix

* childstate test CR feedback

* dev tests

* wip/grandpa tests

* grandpa tests

* offchain tests

* payment tests

* syc state test updates

* state test CR feedback

* system tests

* fix inconsistent test to use reflect on maps

* use zip file for test data

* delete test data hex file

* fix up tests

* close to final changes

* finish CR feedback

* clean up error checking

* CR feedback TODO/Fix integration test error

* move response init into subtests

* remove panic from code

* WIP/clean up tests

* change res variable to be non pointer

* WIP/add expected response check for error case

* add response check for error case

* finish first round of CR feedback

* hasSessionKey tests

* remove old author module tests

* rename err to expErr

* use system module constructor instead of fields

* rename getter for test metadata

* wip/CR feedback

* separate out system getter tests

* fix naming

* clean up imports

* wip/cr feedback

* remove wantErr from rpc/module tests

* finalize feedback

* rebase

* CR feedback

* remove apimocks alias

* final CR feedback

* finalize CR changes

* fix failing http test

* fix LocalPeerId naming

Author: jimjbrettj

.golangci.yml

@@ -27,7 +27,6 @@ run:
   skip-files:
     - .*mock_.*\.go
     - .*mocks\/.*\.go
-    - author_integration_test\.go # TODO remove once fixed
 
 # all available settings of specific linters
 linters-settings:

dot/rpc/modules/api.go

@@ -147,6 +147,8 @@ type RuntimeStorageAPI interface {
 	GetPersistent(k []byte) ([]byte, error)
 }
 
+//go:generate mockery --name SyncStateAPI --structname SyncStateAPI --case underscore --keeptree
+
 // SyncStateAPI is the interface to interact with sync state.
 type SyncStateAPI interface {
 	GenSyncSpec(raw bool) (*genesis.Genesis, error)

dot/rpc/modules/api_mocks.go

@@ -16,7 +16,8 @@ import (
 func NewMockStorageAPI() *modulesmocks.StorageAPI {
 	m := new(modulesmocks.StorageAPI)
 	m.On("GetStorage", mock.AnythingOfType("*common.Hash"), mock.AnythingOfType("[]uint8")).Return(nil, nil)
-	m.On("GetStorageFromChild", mock.AnythingOfType("*common.Hash"), mock.AnythingOfType("[]uint8"), mock.AnythingOfType("[]uint8")).Return(nil, nil)
+	m.On("GetStorageFromChild", mock.AnythingOfType("*common.Hash"), mock.AnythingOfType("[]uint8"),
+		mock.AnythingOfType("[]uint8")).Return(nil, nil)
 	m.On("Entries", mock.AnythingOfType("*common.Hash")).Return(nil, nil)
 	m.On("GetStorageByBlockHash", mock.AnythingOfType("common.Hash"), mock.AnythingOfType("[]uint8")).Return(nil, nil)
 	m.On("RegisterStorageObserver", mock.Anything)
@@ -41,7 +42,8 @@ func NewMockBlockAPI() *modulesmocks.BlockAPI {
 	m.On("FreeFinalisedNotifierChannel", mock.AnythingOfType("chan *types.FinalisationInfo"))
 	m.On("GetJustification", mock.AnythingOfType("common.Hash")).Return(make([]byte, 10), nil)
 	m.On("HasJustification", mock.AnythingOfType("common.Hash")).Return(true, nil)
-	m.On("SubChain", mock.AnythingOfType("common.Hash"), mock.AnythingOfType("common.Hash")).Return(make([]common.Hash, 0), nil)
+	m.On("SubChain", mock.AnythingOfType("common.Hash"), mock.AnythingOfType("common.Hash")).
+		Return(make([]common.Hash, 0), nil)
 	m.On("RegisterRuntimeUpdatedChannel", mock.AnythingOfType("chan<- runtime.Version")).Return(uint32(0), nil)
 
 	return m

dot/rpc/modules/author.go

@@ -191,7 +191,7 @@ func (am *AuthorModule) PendingExtrinsics(r *http.Request, req *EmptyRequest, re
 }
 
 // RemoveExtrinsic Remove given extrinsic from the pool and temporarily ban it to prevent reimporting
-func (am *AuthorModule) RemoveExtrinsic(r *http.Request, req *ExtrinsicOrHashRequest, res *RemoveExtrinsicsResponse) error {
+func (am *AuthorModule) RemoveExtrinsic(r *http.Request, _ *ExtrinsicOrHashRequest, _ *RemoveExtrinsicsResponse) error {
 	return nil
 }
 

dot/rpc/modules/author_integration_test.go

@@ -8,6 +8,7 @@ package modules
 
 import (
 	"fmt"
+	"io"
 	"os"
 	"reflect"
 	"testing"
@@ -16,6 +17,8 @@ import (
 	"github.com/ChainSafe/gossamer/dot/types"
 	"github.com/ChainSafe/gossamer/internal/log"
 	"github.com/ChainSafe/gossamer/lib/common"
+	"github.com/ChainSafe/gossamer/lib/crypto/ed25519"
+	"github.com/ChainSafe/gossamer/lib/crypto/sr25519"
 	"github.com/ChainSafe/gossamer/lib/keystore"
 	"github.com/ChainSafe/gossamer/lib/runtime"
 	"github.com/ChainSafe/gossamer/lib/runtime/wasmer"
@@ -24,7 +27,9 @@ import (
 )
 
 // https://github.com/paritytech/substrate/blob/5420de3face1349a97eb954ae71c5b0b940c31de/core/transaction-pool/src/tests.rs#L95
-var testExt = common.MustHexToBytes("0x410284ffd43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d01f8efbe48487e57a22abf7e3acd491b7f3528a33a111b1298601554863d27eb129eaa4e718e1365414ff3d028b62bebc651194c6b5001e5c2839b982757e08a8c0000000600ff8eaf04151687736326c9fea17e25fc5287613693c912909cb226aa4794f26a480b00c465f14670")
+var testExt = common.MustHexToBytes("0x410284ffd43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d01f8e" +
+	"fbe48487e57a22abf7e3acd491b7f3528a33a111b1298601554863d27eb129eaa4e718e1365414ff3d028b62bebc651194c6b5001e5c2839b98" +
+	"2757e08a8c0000000600ff8eaf04151687736326c9fea17e25fc5287613693c912909cb226aa4794f26a480b00c465f14670")
 
 // invalid transaction (above tx, with last byte changed)
 //nolint
@@ -33,7 +38,7 @@ var testInvalidExt = []byte{1, 212, 53, 147, 199, 21, 253, 211, 28, 97, 20, 26,
 func TestMain(m *testing.M) {
 	wasmFilePaths, err := runtime.GenerateRuntimeWasmFile()
 	if err != nil {
-		log.Error("failed to generate runtime wasm file", err)
+		log.Errorf("failed to generate runtime wasm file: %s", err)
 		os.Exit(1)
 	}
 
@@ -46,7 +51,7 @@ func TestMain(m *testing.M) {
 
 func TestAuthorModule_Pending(t *testing.T) {
 	txQueue := state.NewTransactionState()
-	auth := NewAuthorModule(nil, nil, nil, txQueue)
+	auth := NewAuthorModule(log.New(log.SetWriter(io.Discard)), nil, txQueue)
 
 	res := new(PendingExtrinsicsResponse)
 	err := auth.PendingExtrinsics(nil, nil, res)
@@ -178,35 +183,48 @@ func TestAuthorModule_SubmitExtrinsic_InQueue(t *testing.T) {
 }
 
 func TestAuthorModule_InsertKey_Valid(t *testing.T) {
+	seed := "0xb7e9185065667390d2ad952a5324e8c365c9bf503dcf97c67a5ce861afe97309"
+	kp, err := sr25519.NewKeypairFromSeed(common.MustHexToBytes(seed))
+	require.NoError(t, err)
+
 	auth := setupAuthModule(t, nil)
-	req := &KeyInsertRequest{"babe", "0xb7e9185065667390d2ad952a5324e8c365c9bf503dcf97c67a5ce861afe97309", "0x6246ddf254e0b4b4e7dffefc8adf69d212b98ac2b579c362b473fec8c40b4c0a"}
+	req := &KeyInsertRequest{"babe", seed, kp.Public().Hex()}
 	res := &KeyInsertResponse{}
-	err := auth.InsertKey(nil, req, res)
+	err = auth.InsertKey(nil, req, res)
 	require.Nil(t, err)
 	require.Len(t, *res, 0) // zero len result on success
 }
 
-func TestAuthorModule_InsertKey_Valid_gran_keytype(t *testing.T) {
+func TestAuthorModule_InsertKey_Valid_Gran_Keytype(t *testing.T) {
+	seed := "0xb7e9185065667390d2ad952a5324e8c365c9bf503dcf97c67a5ce861afe97309"
+	kp, err := ed25519.NewKeypairFromSeed(common.MustHexToBytes(seed))
+	require.NoError(t, err)
+
 	auth := setupAuthModule(t, nil)
-	req := &KeyInsertRequest{"gran", "0xb7e9185065667390d2ad952a5324e8c365c9bf503dcf97c67a5ce861afe97309b7e9185065667390d2ad952a5324e8c365c9bf503dcf97c67a5ce861afe97309", "0xb7e9185065667390d2ad952a5324e8c365c9bf503dcf97c67a5ce861afe97309"}
+	req := &KeyInsertRequest{"gran", seed, kp.Public().Hex()}
 	res := &KeyInsertResponse{}
-	err := auth.InsertKey(nil, req, res)
+	err = auth.InsertKey(nil, req, res)
 	require.Nil(t, err)
 
 	require.Len(t, *res, 0) // zero len result on success
 }
 
 func TestAuthorModule_InsertKey_InValid(t *testing.T) {
 	auth := setupAuthModule(t, nil)
-	req := &KeyInsertRequest{"babe", "0xb7e9185065667390d2ad952a5324e8c365c9bf503dcf97c67a5ce861afe97309", "0x0000000000000000000000000000000000000000000000000000000000000000"}
+	req := &KeyInsertRequest{
+		"babe",
+		"0xb7e9185065667390d2ad952a5324e8c365c9bf503dcf97c67a5ce861afe97309",
+		"0x0000000000000000000000000000000000000000000000000000000000000000"}
 	res := &KeyInsertResponse{}
 	err := auth.InsertKey(nil, req, res)
 	require.EqualError(t, err, "generated public key does not equal provide public key")
 }
 
 func TestAuthorModule_InsertKey_UnknownKeyType(t *testing.T) {
 	auth := setupAuthModule(t, nil)
-	req := &KeyInsertRequest{"mack", "0xb7e9185065667390d2ad952a5324e8c365c9bf503dcf97c67a5ce861afe97309", "0x6246ddf254e0b4b4e7dffefc8adf69d212b98ac2b579c362b473fec8c40b4c0a"}
+	req := &KeyInsertRequest{"mack",
+		"0xb7e9185065667390d2ad952a5324e8c365c9bf503dcf97c67a5ce861afe97309",
+		"0x6246ddf254e0b4b4e7dffefc8adf69d212b98ac2b579c362b473fec8c40b4c0a"}
 	res := &KeyInsertResponse{}
 	err := auth.InsertKey(nil, req, res)
 	require.EqualError(t, err, "cannot decode key: invalid key type")
@@ -267,5 +285,5 @@ func setupAuthModule(t *testing.T, txq *state.TransactionState) *AuthorModule {
 	t.Cleanup(func() {
 		rt.Stop()
 	})
-	return NewAuthorModule(nil, cs, rt, txq)
+	return NewAuthorModule(log.New(log.SetWriter(io.Discard)), cs, txq)
 }