Skip to content

Commit

Permalink
test: use pointer receivers to avoid lock copies (cosmos#15217)
Browse files Browse the repository at this point in the history
## 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
  • Loading branch information
mark-rushakoff authored Feb 28, 2023
1 parent d297747 commit 261af67
Show file tree
Hide file tree
Showing 9 changed files with 33 additions and 32 deletions.
2 changes: 1 addition & 1 deletion server/grpc/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())

Expand Down
34 changes: 17 additions & 17 deletions tests/e2e/tx/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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())

Expand Down
6 changes: 3 additions & 3 deletions types/module/module_int_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
}
Expand All @@ -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},
}
Expand All @@ -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"},
Expand Down
1 change: 1 addition & 0 deletions x/authz/keeper/grpc_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion x/capability/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion x/group/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions x/upgrade/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand All @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions x/upgrade/plan/downloader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,15 +94,15 @@ 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
}

// 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)
Expand Down
10 changes: 5 additions & 5 deletions x/upgrade/plan/info_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}}`
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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"
}
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 261af67

Please sign in to comment.