From 261af6721b84a45286bcdf812b0006d2d192f5c4 Mon Sep 17 00:00:00 2001 From: Mark Rushakoff Date: Tue, 28 Feb 2023 13:27:57 -0500 Subject: [PATCH] test: use pointer receivers to avoid lock copies (#15217) ## Description There were several test suite methods that had a value receiver, and which were being ignored for linting. Instead of ignoring the error, use pointer receivers to properly avoid lock copying. And use a local copy of one loop variable in a possible closure. --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [x] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#pr-targeting)) - [ ] ~~provided a link to the relevant issue or specification~~ - [ ] ~~reviewed "Files changed" and left comments if necessary~~ - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed all author checklist items have been addressed - [ ] confirmed that this PR does not change production code --- server/grpc/server_test.go | 2 +- tests/e2e/tx/service_test.go | 34 +++++++++++++++--------------- types/module/module_int_test.go | 6 +++--- x/authz/keeper/grpc_query_test.go | 1 + x/capability/keeper/keeper_test.go | 2 +- x/group/keeper/keeper_test.go | 2 +- x/upgrade/abci_test.go | 4 ++-- x/upgrade/plan/downloader_test.go | 4 ++-- x/upgrade/plan/info_test.go | 10 ++++----- 9 files changed, 33 insertions(+), 32 deletions(-) diff --git a/server/grpc/server_test.go b/server/grpc/server_test.go index ed6272b34f..07e2bb85c3 100644 --- a/server/grpc/server_test.go +++ b/server/grpc/server_test.go @@ -239,7 +239,7 @@ func (s *IntegrationTestSuite) TestGRPCUnpacker() { } // mkTxBuilder creates a TxBuilder containing a signed tx from validator 0. -func (s IntegrationTestSuite) mkTxBuilder() client.TxBuilder { //nolint:govet +func (s *IntegrationTestSuite) mkTxBuilder() client.TxBuilder { val := s.network.Validators[0] s.Require().NoError(s.network.WaitForNextBlock()) diff --git a/tests/e2e/tx/service_test.go b/tests/e2e/tx/service_test.go index b72ba03321..87ea04308a 100644 --- a/tests/e2e/tx/service_test.go +++ b/tests/e2e/tx/service_test.go @@ -147,7 +147,7 @@ func (s *E2ETestSuite) TestQueryBySig() { s.Require().Equal(res.Txs[0].Signatures[0], sig.Signature) } -func (s E2ETestSuite) TestSimulateTx_GRPC() { +func (s *E2ETestSuite) TestSimulateTx_GRPC() { val := s.network.Validators[0] txBuilder := s.mkTxBuilder() // Convert the txBuilder to a tx.Tx. @@ -194,7 +194,7 @@ func (s E2ETestSuite) TestSimulateTx_GRPC() { } } -func (s E2ETestSuite) TestSimulateTx_GRPCGateway() { +func (s *E2ETestSuite) TestSimulateTx_GRPCGateway() { val := s.network.Validators[0] txBuilder := s.mkTxBuilder() // Convert the txBuilder to a tx.Tx. @@ -236,7 +236,7 @@ func (s E2ETestSuite) TestSimulateTx_GRPCGateway() { } } -func (s E2ETestSuite) TestGetTxEvents_GRPC() { +func (s *E2ETestSuite) TestGetTxEvents_GRPC() { testCases := []struct { name string req *tx.GetTxsEventRequest @@ -328,7 +328,7 @@ func (s E2ETestSuite) TestGetTxEvents_GRPC() { } } -func (s E2ETestSuite) TestGetTxEvents_GRPCGateway() { +func (s *E2ETestSuite) TestGetTxEvents_GRPCGateway() { val := s.network.Validators[0] testCases := []struct { name string @@ -405,7 +405,7 @@ func (s E2ETestSuite) TestGetTxEvents_GRPCGateway() { } } -func (s E2ETestSuite) TestGetTx_GRPC() { +func (s *E2ETestSuite) TestGetTx_GRPC() { testCases := []struct { name string req *tx.GetTxRequest @@ -432,7 +432,7 @@ func (s E2ETestSuite) TestGetTx_GRPC() { } } -func (s E2ETestSuite) TestGetTx_GRPCGateway() { +func (s *E2ETestSuite) TestGetTx_GRPCGateway() { val := s.network.Validators[0] testCases := []struct { name string @@ -479,7 +479,7 @@ func (s E2ETestSuite) TestGetTx_GRPCGateway() { } } -func (s E2ETestSuite) TestBroadcastTx_GRPC() { +func (s *E2ETestSuite) TestBroadcastTx_GRPC() { val := s.network.Validators[0] txBuilder := s.mkTxBuilder() txBytes, err := val.ClientCtx.TxConfig.TxEncoder()(txBuilder.GetTx()) @@ -517,7 +517,7 @@ func (s E2ETestSuite) TestBroadcastTx_GRPC() { } } -func (s E2ETestSuite) TestBroadcastTx_GRPCGateway() { +func (s *E2ETestSuite) TestBroadcastTx_GRPCGateway() { val := s.network.Validators[0] txBuilder := s.mkTxBuilder() txBytes, err := val.ClientCtx.TxConfig.TxEncoder()(txBuilder.GetTx()) @@ -662,7 +662,7 @@ func (s *E2ETestSuite) TestSimMultiSigTx() { s.Require().Greater(res.GasInfo.GasUsed, uint64(0)) } -func (s E2ETestSuite) TestGetBlockWithTxs_GRPC() { +func (s *E2ETestSuite) TestGetBlockWithTxs_GRPC() { testCases := []struct { name string req *tx.GetBlockWithTxsRequest @@ -700,7 +700,7 @@ func (s E2ETestSuite) TestGetBlockWithTxs_GRPC() { } } -func (s E2ETestSuite) TestGetBlockWithTxs_GRPCGateway() { +func (s *E2ETestSuite) TestGetBlockWithTxs_GRPCGateway() { val := s.network.Validators[0] testCases := []struct { name string @@ -741,7 +741,7 @@ func (s E2ETestSuite) TestGetBlockWithTxs_GRPCGateway() { } } -func (s E2ETestSuite) TestTxEncode_GRPC() { +func (s *E2ETestSuite) TestTxEncode_GRPC() { val := s.network.Validators[0] txBuilder := s.mkTxBuilder() protoTx, err := txBuilderToProtoTx(txBuilder) @@ -816,7 +816,7 @@ func (s *E2ETestSuite) TestTxEncode_GRPCGateway() { } } -func (s E2ETestSuite) TestTxDecode_GRPC() { +func (s *E2ETestSuite) TestTxDecode_GRPC() { val := s.network.Validators[0] txBuilder := s.mkTxBuilder() @@ -858,7 +858,7 @@ func (s E2ETestSuite) TestTxDecode_GRPC() { } } -func (s E2ETestSuite) TestTxDecode_GRPCGateway() { +func (s *E2ETestSuite) TestTxDecode_GRPCGateway() { val := s.network.Validators[0] txBuilder := s.mkTxBuilder() @@ -901,7 +901,7 @@ func (s E2ETestSuite) TestTxDecode_GRPCGateway() { } } -func (s E2ETestSuite) TestTxEncodeAmino_GRPC() { +func (s *E2ETestSuite) TestTxEncodeAmino_GRPC() { val := s.network.Validators[0] txBuilder := s.mkTxBuilder() stdTx, err := clienttx.ConvertTxToStdTx(val.ClientCtx.LegacyAmino, txBuilder.GetTx()) @@ -984,7 +984,7 @@ func (s *E2ETestSuite) TestTxEncodeAmino_GRPCGateway() { } } -func (s E2ETestSuite) TestTxDecodeAmino_GRPC() { +func (s *E2ETestSuite) TestTxDecodeAmino_GRPC() { val := s.network.Validators[0] txBuilder := s.mkTxBuilder() @@ -1029,7 +1029,7 @@ func (s E2ETestSuite) TestTxDecodeAmino_GRPC() { } } -func (s E2ETestSuite) TestTxDecodeAmino_GRPCGateway() { +func (s *E2ETestSuite) TestTxDecodeAmino_GRPCGateway() { val := s.network.Validators[0] txBuilder := s.mkTxBuilder() @@ -1078,7 +1078,7 @@ func TestE2ETestSuite(t *testing.T) { suite.Run(t, new(E2ETestSuite)) } -func (s E2ETestSuite) mkTxBuilder() client.TxBuilder { +func (s *E2ETestSuite) mkTxBuilder() client.TxBuilder { val := s.network.Validators[0] s.Require().NoError(s.network.WaitForNextBlock()) diff --git a/types/module/module_int_test.go b/types/module/module_int_test.go index afdb4322ef..443ee0bbcb 100644 --- a/types/module/module_int_test.go +++ b/types/module/module_int_test.go @@ -15,7 +15,7 @@ type TestSuite struct { suite.Suite } -func (s TestSuite) TestAssertNoForgottenModules() { //nolint:govet +func (s *TestSuite) TestAssertNoForgottenModules() { m := Manager{ Modules: map[string]interface{}{"a": nil, "b": nil}, } @@ -40,7 +40,7 @@ func (s TestSuite) TestAssertNoForgottenModules() { //nolint:govet } } -func (s TestSuite) TestModuleNames() { //nolint:govet // this is a test +func (s *TestSuite) TestModuleNames() { m := Manager{ Modules: map[string]interface{}{"a": nil, "b": nil}, } @@ -49,7 +49,7 @@ func (s TestSuite) TestModuleNames() { //nolint:govet // this is a test s.Require().Equal([]string{"a", "b"}, ms) } -func (s TestSuite) TestDefaultMigrationsOrder() { //nolint:govet // this is a test +func (s *TestSuite) TestDefaultMigrationsOrder() { require := s.Require() require.Equal( []string{"auth2", "d", "z", "auth"}, diff --git a/x/authz/keeper/grpc_query_test.go b/x/authz/keeper/grpc_query_test.go index 426c74be87..041ec22f6d 100644 --- a/x/authz/keeper/grpc_query_test.go +++ b/x/authz/keeper/grpc_query_test.go @@ -212,6 +212,7 @@ func (suite *TestSuite) TestGRPCQueryGranterGrants() { } for _, tc := range testCases { + tc := tc suite.Run(fmt.Sprintf("Case %s", tc.msg), func() { tc.preRun() result, err := queryClient.GranterGrants(gocontext.Background(), &tc.request) diff --git a/x/capability/keeper/keeper_test.go b/x/capability/keeper/keeper_test.go index daf8d7e205..61d5ddf6b9 100644 --- a/x/capability/keeper/keeper_test.go +++ b/x/capability/keeper/keeper_test.go @@ -274,7 +274,7 @@ func (suite *KeeperTestSuite) TestReleaseCapability() { suite.Require().Error(sk1.ReleaseCapability(suite.ctx, nil)) } -func (suite KeeperTestSuite) TestRevertCapability() { //nolint:govet // this is a test, we can copy locks +func (suite *KeeperTestSuite) TestRevertCapability() { sk := suite.keeper.ScopeToModule(bankModuleName) ms := suite.ctx.MultiStore() diff --git a/x/group/keeper/keeper_test.go b/x/group/keeper/keeper_test.go index 35e160c0c9..40a25f591e 100644 --- a/x/group/keeper/keeper_test.go +++ b/x/group/keeper/keeper_test.go @@ -124,7 +124,7 @@ func (s *TestSuite) SetupTest() { s.bankKeeper.SendCoinsFromModuleToAccount(s.sdkCtx, minttypes.ModuleName, s.groupPolicyAddr, sdk.Coins{sdk.NewInt64Coin("test", 10000)}) } -func (s TestSuite) setNextAccount() { //nolint:govet // this is a test and we're okay with copying locks here. +func (s *TestSuite) setNextAccount() { nextAccVal := s.groupKeeper.GetGroupPolicySeq(s.sdkCtx) + 1 derivationKey := make([]byte, 8) binary.BigEndian.PutUint64(derivationKey, nextAccVal) diff --git a/x/upgrade/abci_test.go b/x/upgrade/abci_test.go index aee58f0202..90a2c6d78a 100644 --- a/x/upgrade/abci_test.go +++ b/x/upgrade/abci_test.go @@ -41,7 +41,7 @@ type TestSuite struct { var s TestSuite -func setupTest(t *testing.T, height int64, skip map[int64]bool) TestSuite { +func setupTest(t *testing.T, height int64, skip map[int64]bool) *TestSuite { s.encCfg = moduletestutil.MakeTestEncodingConfig(upgrade.AppModuleBasic{}) key := storetypes.NewKVStoreKey(types.StoreKey) testCtx := testutil.DefaultContextWithDB(s.T(), key, storetypes.NewTransientStoreKey("transient_test")) @@ -60,7 +60,7 @@ func setupTest(t *testing.T, height int64, skip map[int64]bool) TestSuite { s.module = upgrade.NewAppModule(s.keeper) s.handler = upgrade.NewSoftwareUpgradeProposalHandler(s.keeper) - return s //nolint:govet // this is a test, we can copy locks + return &s } func TestRequireName(t *testing.T) { diff --git a/x/upgrade/plan/downloader_test.go b/x/upgrade/plan/downloader_test.go index b9c6f8740c..7a48275ef6 100644 --- a/x/upgrade/plan/downloader_test.go +++ b/x/upgrade/plan/downloader_test.go @@ -94,7 +94,7 @@ func (z TestZip) SaveAs(path string) error { // saveTestZip saves a TestZip in this test's Home/src directory with the given name. // The full path to the saved archive is returned. -func (s DownloaderTestSuite) saveSrcTestZip(name string, z TestZip) string { //nolint:govet // this is a test, we can copy locks +func (s *DownloaderTestSuite) saveSrcTestZip(name string, z TestZip) string { fullName := filepath.Join(s.Home, "src", name) s.Require().NoError(z.SaveAs(fullName), "saving test zip %s", name) return fullName @@ -102,7 +102,7 @@ func (s DownloaderTestSuite) saveSrcTestZip(name string, z TestZip) string { //n // saveSrcTestFile saves a TestFile in this test's Home/src directory. // The full path to the saved file is returned. -func (s DownloaderTestSuite) saveSrcTestFile(f *TestFile) string { //nolint:govet // this is a test, we can copy locks +func (s *DownloaderTestSuite) saveSrcTestFile(f *TestFile) string { path := filepath.Join(s.Home, "src") fullName, err := f.SaveIn(path) s.Require().NoError(err, "saving test file %s", f.Name) diff --git a/x/upgrade/plan/info_test.go b/x/upgrade/plan/info_test.go index b6fa03bce7..ed5a206762 100644 --- a/x/upgrade/plan/info_test.go +++ b/x/upgrade/plan/info_test.go @@ -26,13 +26,13 @@ func TestInfoTestSuite(t *testing.T) { // saveSrcTestFile saves a TestFile in this test's Home/src directory. // The full path to the saved file is returned. -func (s InfoTestSuite) saveTestFile(f *TestFile) string { //nolint:govet // false positive +func (s *InfoTestSuite) saveTestFile(f *TestFile) string { fullName, err := f.SaveIn(s.Home) s.Require().NoError(err, "saving test file %s", f.Name) return fullName } -func (s InfoTestSuite) TestParseInfo() { //nolint:govet // false positive +func (s *InfoTestSuite) TestParseInfo() { goodJSON := `{"binaries":{"os1/arch1":"url1","os2/arch2":"url2"}}` binariesWrongJSON := `{"binaries":["foo","bar"]}` binariesWrongValueJSON := `{"binaries":{"os1/arch1":1,"os2/arch2":2}}` @@ -129,7 +129,7 @@ func (s InfoTestSuite) TestParseInfo() { //nolint:govet // false positive } } -func (s InfoTestSuite) TestInfoValidateFull() { //nolint:govet // this is a test, we can copy locks +func (s *InfoTestSuite) TestInfoValidateFull() { darwinAMD64File := NewTestFile("darwin_amd64", "#!/usr/bin\necho 'darwin/amd64'\n") linux386File := NewTestFile("linux_386", "#!/usr/bin\necho 'darwin/amd64'\n") darwinAMD64Path := s.saveTestFile(darwinAMD64File) @@ -186,7 +186,7 @@ func (s InfoTestSuite) TestInfoValidateFull() { //nolint:govet // this is a test } } -func (s InfoTestSuite) TestBinaryDownloadURLMapValidateBasic() { //nolint:govet // this is a test, we can copy locks +func (s *InfoTestSuite) TestBinaryDownloadURLMapValidateBasic() { addDummyChecksum := func(url string) string { return url + "?checksum=sha256:b5a2c96250612366ea272ffac6d9744aaf4b45aacd96aa7cfcb931ee3b558259" } @@ -282,7 +282,7 @@ func (s InfoTestSuite) TestBinaryDownloadURLMapValidateBasic() { //nolint:govet } } -func (s InfoTestSuite) TestBinaryDownloadURLMapCheckURLs() { //nolint:govet // this is a test, we can copy locks +func (s *InfoTestSuite) TestBinaryDownloadURLMapCheckURLs() { darwinAMD64File := NewTestFile("darwin_amd64", "#!/usr/bin\necho 'darwin/amd64'\n") linux386File := NewTestFile("linux_386", "#!/usr/bin\necho 'darwin/amd64'\n") darwinAMD64Path := s.saveTestFile(darwinAMD64File)