lint tests (#2926)

* lint tests

* don't use prealloc for golangci

* fix unnecessary conversions

* var-declaration lints

* fix ineffectual assignments

* fix composite literal lints

* address copylocks lints from govet

* error checks in tests

* handshake_test.go error checks

* packet_test.go error checks

* error checks

* msg_server_test.go

* errcheck in upgrade_test.go

* goconsts & linting complete

* Update CHANGELOG.md

* golangci-lint run ./... --fix

* last lint

* fix lints

* tidy

* ignore legacy ica api

* ignore icacontrollersendtx

* golangci lint fixes

* fix test

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
Co-authored-by: Carlos Rodriguez <crodveg@gmail.com>

.golangci.yml

@@ -1,27 +1,24 @@
 run:
-  tests: false
+  tests: true
   #   # timeout for analysis, e.g. 30s, 5m, default is 1m
   timeout: 5m
 
 linters:
   disable-all: true
   enable:
-    - deadcode
     - depguard
     - dogsled
     - exportloopref
     - errcheck
     - goconst
     - gocritic
     - gofumpt
-    - goimports
     - gosec
     - gosimple
     - govet
     - ineffassign
     - misspell
     - nakedret
-    - prealloc
     - staticcheck
     - stylecheck
     - revive
@@ -33,6 +30,9 @@ linters:
 
 issues:
   exclude-rules:
+    - text: "SA1019: suite.chainA.GetSimApp().ICAControllerKeeper.SendTx is deprecated: this is a legacy API that is only intended to function correctly in workflows where an underlying application has been set. Prior to to v6.x.x of ibc-go, the controller module was only functional as middleware, with authentication performed by the underlying application. For a full summary of the changes in v6.x.x, please see ADR009. This API will be removed in later releases."
+      linters:
+        - staticcheck
     - text: "Use of weak random number generator"
       linters:
         - gosec

CHANGELOG.md

@@ -68,6 +68,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Improvements
 
+* (tests) [\#2926](https://github.com/cosmos/ibc-go/pull/2926) Lint tests
 * (apps/transfer) [\#2643](https://github.com/cosmos/ibc-go/pull/2643) Add amount, denom, and memo to transfer event emission.
 * (core) [\#2746](https://github.com/cosmos/ibc-go/pull/2746) Allow proof height to be zero for all core IBC `sdk.Msg` types that contain proofs.
 * (light-clients/06-solomachine) [\#2746](https://github.com/cosmos/ibc-go/pull/2746) Discard proofHeight for solo machines and use the solo machine sequence instead.

cmd/build_test_matrix/main_test.go

@@ -102,7 +102,7 @@ func SuiteTwo(t *testing.T) {
 type FeeMiddlewareTestSuite struct {}
 `
 
-		err := os.WriteFile(path.Join(testingDir, goTestFileNameOne), []byte(fileWithTwoSuites), os.FileMode(777))
+		err := os.WriteFile(path.Join(testingDir, goTestFileNameOne), []byte(fileWithTwoSuites), os.FileMode(0o777))
 		assert.NoError(t, err)
 
 		_, err = getGithubActionMatrixForTests(testingDir, "", nil)
@@ -156,6 +156,6 @@ func helper() {}
 
 func createFileWithTestSuiteAndTests(t *testing.T, suiteName, fn1Name, fn2Name, dir, filename string) {
 	goFileContents := goTestFileContents(suiteName, fn1Name, fn2Name)
-	err := os.WriteFile(path.Join(dir, filename), []byte(goFileContents), os.FileMode(777))
+	err := os.WriteFile(path.Join(dir, filename), []byte(goFileContents), os.FileMode(0o777))
 	assert.NoError(t, err)
 }

go.mod

@@ -3,6 +3,7 @@ go 1.19
 module github.com/cosmos/ibc-go/v6
 
 require (
+	cosmossdk.io/errors v1.0.0-beta.7
 	cosmossdk.io/math v1.0.0-beta.4
 	cosmossdk.io/simapp v0.0.0-20221216140705-ee8890cf30e7
 	cosmossdk.io/tools/rosetta v0.2.0
@@ -36,7 +37,6 @@ require (
 	cosmossdk.io/api v0.2.6 // indirect
 	cosmossdk.io/core v0.3.2 // indirect
 	cosmossdk.io/depinject v1.0.0-alpha.3 // indirect
-	cosmossdk.io/errors v1.0.0-beta.7 // indirect
 	filippo.io/edwards25519 v1.0.0-rc.1 // indirect
 	github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4 // indirect
 	github.com/99designs/keyring v1.2.1 // indirect

modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go

@@ -209,7 +209,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenInit() {
 			suite.Require().NoError(err)
 
 			portCap := suite.chainA.GetSimApp().IBCKeeper.PortKeeper.BindPort(suite.chainA.GetContext(), portID)
-			suite.chainA.GetSimApp().ICAControllerKeeper.ClaimCapability(suite.chainA.GetContext(), portCap, host.PortPath(portID))
+			suite.chainA.GetSimApp().ICAControllerKeeper.ClaimCapability(suite.chainA.GetContext(), portCap, host.PortPath(portID)) //nolint:errcheck // checking this error isn't needed for the test
 
 			path.EndpointA.ChannelConfig.PortID = portID
 			path.EndpointA.ChannelID = ibctesting.FirstChannelID
@@ -276,7 +276,9 @@ func (suite *InterchainAccountsTestSuite) TestChanOpenTry() {
 	err = RegisterInterchainAccount(path.EndpointB, TestOwnerAddress)
 	suite.Require().NoError(err)
 
-	path.EndpointA.UpdateClient()
+	err = path.EndpointA.UpdateClient()
+	suite.Require().NoError(err)
+
 	channelKey := host.ChannelKey(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
 	proofInit, proofHeight := path.EndpointB.Chain.QueryProof(channelKey)
 
@@ -420,7 +422,8 @@ func (suite *InterchainAccountsTestSuite) TestChanOpenConfirm() {
 	// commit state changes so proof can be created
 	suite.chainB.NextBlock()
 
-	path.EndpointA.UpdateClient()
+	err = path.EndpointA.UpdateClient()
+	suite.Require().NoError(err)
 
 	// query proof from ChainB
 	channelKey := host.ChannelKey(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)

modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go

@@ -103,7 +103,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() {
 		{
 			"invalid port ID",
 			func() {
-				path.EndpointA.ChannelConfig.PortID = "invalid-port-id"
+				path.EndpointA.ChannelConfig.PortID = "invalid-port-id" //nolint:goconst
 			},
 			false,
 		},
@@ -237,7 +237,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() {
 			suite.Require().NoError(err)
 
 			portCap := suite.chainA.GetSimApp().IBCKeeper.PortKeeper.BindPort(suite.chainA.GetContext(), portID)
-			suite.chainA.GetSimApp().ICAControllerKeeper.ClaimCapability(suite.chainA.GetContext(), portCap, host.PortPath(portID))
+			suite.chainA.GetSimApp().ICAControllerKeeper.ClaimCapability(suite.chainA.GetContext(), portCap, host.PortPath(portID)) //nolint:errcheck // this error check isn't needed for tests
 			path.EndpointA.ChannelConfig.PortID = portID
 
 			// default values

modules/apps/27-interchain-accounts/controller/keeper/keeper_test.go

@@ -157,8 +157,8 @@ func (suite *KeeperTestSuite) TestGetInterchainAccountAddress() {
 
 func (suite *KeeperTestSuite) TestGetAllActiveChannels() {
 	var (
-		expectedChannelID string = "test-channel"
-		expectedPortID    string = "test-port"
+		expectedChannelID = "test-channel"
+		expectedPortID    = "test-port"
 	)
 
 	suite.SetupTest()
@@ -193,8 +193,8 @@ func (suite *KeeperTestSuite) TestGetAllActiveChannels() {
 
 func (suite *KeeperTestSuite) TestGetAllInterchainAccounts() {
 	var (
-		expectedAccAddr string = "test-acc-addr"
-		expectedPortID  string = "test-port"
+		expectedAccAddr = "test-acc-addr"
+		expectedPortID  = "test-port"
 	)
 
 	suite.SetupTest()
@@ -245,8 +245,8 @@ func (suite *KeeperTestSuite) TestIsActiveChannel() {
 
 func (suite *KeeperTestSuite) TestSetInterchainAccountAddress() {
 	var (
-		expectedAccAddr string = "test-acc-addr"
-		expectedPortID  string = "test-port"
+		expectedAccAddr = "test-acc-addr"
+		expectedPortID  = "test-port"
 	)
 
 	suite.chainA.GetSimApp().ICAControllerKeeper.SetInterchainAccountAddress(suite.chainA.GetContext(), ibctesting.FirstConnectionID, expectedPortID, expectedAccAddr)

modules/apps/27-interchain-accounts/host/ibc_module_test.go

@@ -251,7 +251,8 @@ func (suite *InterchainAccountsTestSuite) TestChanOpenAck() {
 	// commit state changes so proof can be created
 	suite.chainA.NextBlock()
 
-	path.EndpointB.UpdateClient()
+	err = path.EndpointB.UpdateClient()
+	suite.Require().NoError(err)
 
 	// query proof from ChainA
 	channelKey := host.ChannelKey(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
@@ -668,7 +669,8 @@ func (suite *InterchainAccountsTestSuite) TestControlAccountAfterChannelClose()
 
 	_, err = suite.chainA.GetSimApp().ICAControllerKeeper.SendTx(suite.chainA.GetContext(), nil, ibctesting.FirstConnectionID, path.EndpointA.ChannelConfig.PortID, icaPacketData, ^uint64(0))
 	suite.Require().NoError(err)
-	path.EndpointB.UpdateClient()
+	err = path.EndpointB.UpdateClient()
+	suite.Require().NoError(err)
 
 	// relay the packet
 	packetRelay := channeltypes.NewPacket(icaPacketData.GetBytes(), 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, clienttypes.ZeroHeight(), ^uint64(0))
@@ -694,7 +696,8 @@ func (suite *InterchainAccountsTestSuite) TestControlAccountAfterChannelClose()
 
 	_, err = suite.chainA.GetSimApp().ICAControllerKeeper.SendTx(suite.chainA.GetContext(), nil, ibctesting.FirstConnectionID, path.EndpointA.ChannelConfig.PortID, icaPacketData, ^uint64(0))
 	suite.Require().NoError(err)
-	path.EndpointB.UpdateClient()
+	err = path.EndpointB.UpdateClient()
+	suite.Require().NoError(err)
 
 	// relay the packet
 	packetRelay = channeltypes.NewPacket(icaPacketData.GetBytes(), 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, clienttypes.ZeroHeight(), ^uint64(0))

modules/apps/27-interchain-accounts/host/keeper/handshake_test.go

@@ -167,7 +167,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenTry() {
 		{
 			"invalid port ID",
 			func() {
-				path.EndpointB.ChannelConfig.PortID = "invalid-port-id"
+				path.EndpointB.ChannelConfig.PortID = "invalid-port-id" //nolint:goconst
 			},
 			false,
 		},

modules/apps/27-interchain-accounts/host/keeper/keeper_test.go

@@ -141,8 +141,8 @@ func (suite *KeeperTestSuite) TestGetInterchainAccountAddress() {
 
 func (suite *KeeperTestSuite) TestGetAllActiveChannels() {
 	var (
-		expectedChannelID string = "test-channel"
-		expectedPortID    string = "test-port"
+		expectedChannelID = "test-channel"
+		expectedPortID    = "test-port"
 	)
 
 	suite.SetupTest()
@@ -175,8 +175,8 @@ func (suite *KeeperTestSuite) TestGetAllActiveChannels() {
 
 func (suite *KeeperTestSuite) TestGetAllInterchainAccounts() {
 	var (
-		expectedAccAddr string = "test-acc-addr"
-		expectedPortID  string = "test-port"
+		expectedAccAddr = "test-acc-addr"
+		expectedPortID  = "test-port"
 	)
 
 	suite.SetupTest()
@@ -225,8 +225,8 @@ func (suite *KeeperTestSuite) TestIsActiveChannel() {
 
 func (suite *KeeperTestSuite) TestSetInterchainAccountAddress() {
 	var (
-		expectedAccAddr string = "test-acc-addr"
-		expectedPortID  string = "test-port"
+		expectedAccAddr = "test-acc-addr"
+		expectedPortID  = "test-port"
 	)
 
 	suite.chainB.GetSimApp().ICAHostKeeper.SetInterchainAccountAddress(suite.chainB.GetContext(), ibctesting.FirstConnectionID, expectedPortID, expectedAccAddr)

modules/apps/27-interchain-accounts/types/codec_test.go

@@ -11,13 +11,6 @@ import (
 	"github.com/cosmos/ibc-go/v6/testing/simapp"
 )
 
-// caseRawBytes defines a helper struct, used for testing codec operations
-type caseRawBytes struct {
-	name    string
-	bz      []byte
-	expPass bool
-}
-
 // mockSdkMsg defines a mock struct, used for testing codec error scenarios
 type mockSdkMsg struct{}
 

modules/apps/29-fee/ibc_middleware_test.go

@@ -172,7 +172,8 @@ func (suite *FeeTestSuite) TestOnChanOpenTry() {
 			// reset suite
 			suite.SetupTest()
 			suite.coordinator.SetupConnections(suite.path)
-			suite.path.EndpointB.ChanOpenInit()
+			err := suite.path.EndpointB.ChanOpenInit()
+			suite.Require().NoError(err)
 
 			// setup mock callback
 			suite.chainA.GetSimApp().FeeMockModule.IBCApp.OnChanOpenTry = func(ctx sdk.Context, order channeltypes.Order, connectionHops []string,
@@ -188,7 +189,6 @@ func (suite *FeeTestSuite) TestOnChanOpenTry() {
 			var (
 				chanCap *capabilitytypes.Capability
 				ok      bool
-				err     error
 			)
 
 			chanCap, err = suite.chainA.App.GetScopedIBCKeeper().NewCapability(suite.chainA.GetContext(), host.ChannelCapabilityPath(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID))
@@ -286,8 +286,10 @@ func (suite *FeeTestSuite) TestOnChanOpenAck() {
 			// malleate test case
 			tc.malleate(suite)
 
-			suite.path.EndpointA.ChanOpenInit()
-			suite.path.EndpointB.ChanOpenTry()
+			err := suite.path.EndpointA.ChanOpenInit()
+			suite.Require().NoError(err)
+			err = suite.path.EndpointB.ChanOpenTry()
+			suite.Require().NoError(err)
 
 			module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), ibctesting.MockFeePort)
 			suite.Require().NoError(err)

modules/apps/29-fee/keeper/events_test.go

@@ -40,15 +40,15 @@ func (suite *KeeperTestSuite) TestIncentivizePacketEvent() {
 	}
 
 	for _, attr := range incentivizedPacketEvent.Attributes {
-		switch string(attr.Key) {
+		switch attr.Key {
 		case types.AttributeKeyRecvFee:
-			suite.Require().Equal(expRecvFees.String(), string(attr.Value))
+			suite.Require().Equal(expRecvFees.String(), attr.Value)
 
 		case types.AttributeKeyAckFee:
-			suite.Require().Equal(expAckFees.String(), string(attr.Value))
+			suite.Require().Equal(expAckFees.String(), attr.Value)
 
 		case types.AttributeKeyTimeoutFee:
-			suite.Require().Equal(expTimeoutFees.String(), string(attr.Value))
+			suite.Require().Equal(expTimeoutFees.String(), attr.Value)
 		}
 	}
 
@@ -69,15 +69,15 @@ func (suite *KeeperTestSuite) TestIncentivizePacketEvent() {
 	}
 
 	for _, attr := range incentivizedPacketEvent.Attributes {
-		switch string(attr.Key) {
+		switch attr.Key {
 		case types.AttributeKeyRecvFee:
-			suite.Require().Equal(expRecvFees.String(), string(attr.Value))
+			suite.Require().Equal(expRecvFees.String(), attr.Value)
 
 		case types.AttributeKeyAckFee:
-			suite.Require().Equal(expAckFees.String(), string(attr.Value))
+			suite.Require().Equal(expAckFees.String(), attr.Value)
 
 		case types.AttributeKeyTimeoutFee:
-			suite.Require().Equal(expTimeoutFees.String(), string(attr.Value))
+			suite.Require().Equal(expTimeoutFees.String(), attr.Value)
 		}
 	}
 }

modules/apps/29-fee/keeper/grpc_query_test.go

@@ -425,14 +425,14 @@ func (suite *KeeperTestSuite) TestQueryPayee() {
 		{
 			"payee address not found: invalid channel",
 			func() {
-				req.ChannelId = "invalid-channel-id"
+				req.ChannelId = "invalid-channel-id" //nolint:goconst
 			},
 			false,
 		},
 		{
 			"payee address not found: invalid relayer address",
 			func() {
-				req.Relayer = "invalid-addr"
+				req.Relayer = "invalid-addr" //nolint:goconst
 			},
 			false,
 		},

modules/apps/29-fee/keeper/keeper_test.go

@@ -237,14 +237,14 @@ func (suite *KeeperTestSuite) TestGetAllIdentifiedPacketFees() {
 }
 
 func (suite *KeeperTestSuite) TestGetAllFeeEnabledChannels() {
-	validPortId := "ibcmoduleport"
+	validPortID := "ibcmoduleport"
 	// set two channels enabled
 	suite.chainA.GetSimApp().IBCFeeKeeper.SetFeeEnabled(suite.chainA.GetContext(), ibctesting.MockFeePort, ibctesting.FirstChannelID)
-	suite.chainA.GetSimApp().IBCFeeKeeper.SetFeeEnabled(suite.chainA.GetContext(), validPortId, ibctesting.FirstChannelID)
+	suite.chainA.GetSimApp().IBCFeeKeeper.SetFeeEnabled(suite.chainA.GetContext(), validPortID, ibctesting.FirstChannelID)
 
 	expectedCh := []types.FeeEnabledChannel{
 		{
-			PortId:    validPortId,
+			PortId:    validPortID,
 			ChannelId: ibctesting.FirstChannelID,
 		},
 		{