Restructure E2Es to setup chain in SetupSuite and create channel in S…

…etupTest #1 (#6629) * wip: refactoring tests to create chains in SetupSuite * chore: refactoring memo test to use new setup fns * chore: replace calls to setup chains and relayer with calls to get relayer and get channel * chore: fix wiring in ThreeChainSetup * chore: remove unused fn * chore: add modification function based on chain inputs * chore: update grandpa test with new structure * chore: remove single chain methods * chore: move upgrade tests into different files * chore: fixing client tests * chore: use paths associated with the test * chore: add getter to fetch paths * chore: fix client tests * chore: fix misbehaviour test * chore: fix misbehaviour test * chore: mereg main * chore: pr feedback
cosmos · Jun 19, 2024 · 10aa0ef · 10aa0ef
1 parent 3f6521e
commit 10aa0ef
Show file tree

Hide file tree

Showing 24 changed files with 750 additions and 631 deletions.
diff --git a/e2e/dockerutil/dockerutil.go b/e2e/dockerutil/dockerutil.go
@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"io"
 	"path"
-	"testing"
 
 	dockertypes "github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/filters"
@@ -16,15 +15,15 @@ import (
 const testLabel = "ibc-test"
 
 // GetTestContainers returns all docker containers that have been created by interchain test.
-func GetTestContainers(ctx context.Context, t *testing.T, dc *dockerclient.Client) ([]dockertypes.Container, error) {
-	t.Helper()
-
+// note: the test suite name must be passed as the chains are created in SetupSuite which will label
+// them with the name of the test suite rather than the test.
+func GetTestContainers(ctx context.Context, suiteName string, dc *dockerclient.Client) ([]dockertypes.Container, error) {
 	testContainers, err := dc.ContainerList(ctx, dockertypes.ContainerListOptions{
 		All: true,
 		Filters: filters.NewArgs(
 			// see: https://github.com/strangelove-ventures/interchaintest/blob/0bdc194c2aa11aa32479f32b19e1c50304301981/internal/dockerutil/setup.go#L31-L36
-			// for the label needed to identify test containers.
-			filters.Arg("label", testLabel+"="+t.Name()),
+			// for the suiteName needed to identify test containers.
+			filters.Arg("label", testLabel+"="+suiteName),
 		),
 	})
 	if err != nil {

diff --git a/e2e/tests/core/02-client/client_test.go b/e2e/tests/core/02-client/client_test.go
@@ -65,7 +65,6 @@ func (s *ClientTestSuite) TestScheduleIBCUpgrade_Succeeds() {
 	t := s.T()
 	ctx := context.TODO()
 
-	_, _ = s.SetupChainsRelayerAndChannel(ctx, nil)
 	chainA, chainB := s.GetChains()
 	chainAWallet := s.CreateUserOnChainA(ctx, testvalues.StartingTokenAmount)
 
@@ -164,23 +163,20 @@ func (s *ClientTestSuite) TestClientUpdateProposal_Succeeds() {
 
 	var (
 		pathName           string
-		relayer            ibc.Relayer
 		subjectClientID    string
 		substituteClientID string
 		// set the trusting period to a value which will still be valid upon client creation, but invalid before the first update
 		badTrustingPeriod = time.Second * 10
 	)
 
-	t.Run("create substitute client with correct trusting period", func(t *testing.T) {
-		relayer, _ = s.SetupChainsRelayerAndChannel(ctx, nil)
+	relayer := s.GetRelayer()
 
+	t.Run("create substitute client with correct trusting period", func(t *testing.T) {
 		// TODO: update when client identifier created is accessible
 		// currently assumes first client is 07-tendermint-0
 		substituteClientID = clienttypes.FormatClientIdentifier(ibcexported.Tendermint, 0)
 
-		// TODO: replace with better handling of path names
-		pathName = fmt.Sprintf("%s-path-%d", s.T().Name(), 0)
-		pathName = strings.ReplaceAll(pathName, "/", "-")
+		pathName = s.GetPaths()[0]
 	})
 
 	chainA, chainB := s.GetChains()
@@ -247,23 +243,20 @@ func (s *ClientTestSuite) TestRecoverClient_Succeeds() {
 
 	var (
 		pathName           string
-		relayer            ibc.Relayer
 		subjectClientID    string
 		substituteClientID string
 		// set the trusting period to a value which will still be valid upon client creation, but invalid before the first update
 		badTrustingPeriod = time.Second * 10
 	)
 
-	t.Run("create substitute client with correct trusting period", func(t *testing.T) {
-		relayer, _ = s.SetupChainsRelayerAndChannel(ctx, nil)
+	relayer := s.GetRelayer()
 
+	t.Run("create substitute client with correct trusting period", func(t *testing.T) {
 		// TODO: update when client identifier created is accessible
 		// currently assumes first client is 07-tendermint-0
 		substituteClientID = clienttypes.FormatClientIdentifier(ibcexported.Tendermint, 0)
 
-		// TODO: replace with better handling of path names
-		pathName = fmt.Sprintf("%s-path-%d", s.T().Name(), 0)
-		pathName = strings.ReplaceAll(pathName, "/", "-")
+		pathName = s.GetPaths()[0]
 	})
 
 	chainA, chainB := s.GetChains()
@@ -341,13 +334,13 @@ func (s *ClientTestSuite) TestClient_Update_Misbehaviour() {
 		err             error
 	)
 
-	relayer, _ := s.SetupChainsRelayerAndChannel(ctx, nil)
+	relayer := s.GetRelayer()
 	chainA, chainB := s.GetChains()
 
 	s.Require().NoError(test.WaitForBlocks(ctx, 10, chainA, chainB))
 
 	t.Run("update clients", func(t *testing.T) {
-		err := relayer.UpdateClients(ctx, s.GetRelayerExecReporter(), s.GetPathName(0))
+		err := relayer.UpdateClients(ctx, s.GetRelayerExecReporter(), s.GetPaths()[0])
 		s.Require().NoError(err)
 
 		clientState, err = query.ClientState(ctx, chainA, ibctesting.FirstClientID)
@@ -363,7 +356,7 @@ func (s *ClientTestSuite) TestClient_Update_Misbehaviour() {
 	})
 
 	t.Run("update clients", func(t *testing.T) {
-		err := relayer.UpdateClients(ctx, s.GetRelayerExecReporter(), s.GetPathName(0))
+		err := relayer.UpdateClients(ctx, s.GetRelayerExecReporter(), s.GetPaths()[0])
 		s.Require().NoError(err)
 
 		clientState, err = query.ClientState(ctx, chainA, ibctesting.FirstClientID)
@@ -397,20 +390,28 @@ func (s *ClientTestSuite) TestClient_Update_Misbehaviour() {
 
 		t.Run("extract validator private keys", func(t *testing.T) {
 			privateKeys := s.extractChainPrivateKeys(ctx, chainB)
+			s.Require().NotEmpty(privateKeys, "private keys are empty")
+
 			for i, pv := range privateKeys {
 				pubKey, err := pv.GetPubKey()
 				s.Require().NoError(err)
 
 				validator := cmttypes.NewValidator(pubKey, validators[i].VotingPower)
+				err = validator.ValidateBasic()
+				s.Require().NoError(err, "invalid validator: %s", err)
 
 				validatorSet = append(validatorSet, validator)
 				signers = append(signers, pv)
 			}
 		})
 	})
 
+	s.Require().NotEmpty(validatorSet, "validator set is empty")
+
 	t.Run("create malicious header", func(t *testing.T) {
 		valSet := cmttypes.NewValidatorSet(validatorSet)
+		err := valSet.ValidateBasic()
+		s.Require().NoError(err, "invalid validator set: %s", err)
 		maliciousHeader, err = createMaliciousTMHeader(chainB.Config().ChainID, int64(latestHeight.GetRevisionHeight()), trustedHeight,
 			header.GetTime(), valSet, valSet, signers, header)
 		s.Require().NoError(err)
@@ -437,8 +438,6 @@ func (s *ClientTestSuite) TestAllowedClientsParam() {
 	t := s.T()
 	ctx := context.TODO()
 
-	_, _ = s.SetupChainsRelayerAndChannel(ctx, s.TransferChannelOptions())
-
 	chainA, chainB := s.GetChains()
 	chainAVersion := chainA.Config().Images[0].Version
 	chainAWallet := s.CreateUserOnChainA(ctx, testvalues.StartingTokenAmount)
@@ -494,15 +493,17 @@ func (s *ClientTestSuite) TestAllowedClientsParam() {
 // extractChainPrivateKeys returns a slice of cmttypes.PrivValidator which hold the private keys for all validator
 // nodes for a given chain.
 func (s *ClientTestSuite) extractChainPrivateKeys(ctx context.Context, chain ibc.Chain) []cmttypes.PrivValidator {
-	testContainers, err := dockerutil.GetTestContainers(ctx, s.T(), s.DockerClient)
+	testContainers, err := dockerutil.GetTestContainers(ctx, s.SuiteName(), s.DockerClient)
 	s.Require().NoError(err)
+	s.Require().NotEmpty(testContainers, "no test containers found")
 
 	var filePvs []privval.FilePVKey
 	var pvs []cmttypes.PrivValidator
 	for _, container := range testContainers {
 		isNodeForDifferentChain := !strings.Contains(container.Names[0], chain.Config().ChainID)
 		isFullNode := strings.Contains(container.Names[0], fmt.Sprintf("%s-fn", chain.Config().ChainID))
 		if isNodeForDifferentChain || isFullNode {
+			s.T().Logf("skipping container %s for chain %s", container.Names[0], chain.Config().ChainID)
 			continue
 		}
 

diff --git a/e2e/tests/core/03-connection/connection_test.go b/e2e/tests/core/03-connection/connection_test.go
@@ -33,6 +33,10 @@ type ConnectionTestSuite struct {
 	testsuite.E2ETestSuite
 }
 
+func (s *ConnectionTestSuite) SetupTest() {
+	s.SetupPath(ibc.DefaultClientOpts(), s.TransferChannelOptions())
+}
+
 // QueryMaxExpectedTimePerBlockParam queries the on-chain max expected time per block param for 03-connection
 func (s *ConnectionTestSuite) QueryMaxExpectedTimePerBlockParam(ctx context.Context, chain ibc.Chain) uint64 {
 	if testvalues.SelfParamsFeatureReleases.IsSupported(chain.Config().Images[0].Version) {
@@ -61,7 +65,7 @@ func (s *ConnectionTestSuite) TestMaxExpectedTimePerBlockParam() {
 	t := s.T()
 	ctx := context.TODO()
 
-	relayer, channelA := s.SetupChainsRelayerAndChannel(ctx, s.TransferChannelOptions())
+	relayer, channelA := s.GetRelayer(), s.GetChainAChannel()
 
 	chainA, chainB := s.GetChains()
 	chainAVersion := chainA.Config().Images[0].Version

diff --git a/e2e/tests/interchain_accounts/base_test.go b/e2e/tests/interchain_accounts/base_test.go
@@ -64,7 +64,7 @@ func (s *InterchainAccountsTestSuite) testMsgSendTxSuccessfulTransfer(order chan
 
 	// setup relayers and connection-0 between two chains
 	// channel-0 is a transfer channel but it will not be used in this test case
-	relayer, _ := s.SetupChainsRelayerAndChannel(ctx, nil)
+	relayer := s.GetRelayer()
 	chainA, chainB := s.GetChains()
 
 	// setup 2 accounts: controller account on chain A, a second chain B account.
@@ -161,9 +161,7 @@ func (s *InterchainAccountsTestSuite) TestMsgSendTx_FailedTransfer_InsufficientF
 	t := s.T()
 	ctx := context.TODO()
 
-	// setup relayers and connection-0 between two chains
-	// channel-0 is a transfer channel but it will not be used in this test case
-	relayer, _ := s.SetupChainsRelayerAndChannel(ctx, nil)
+	relayer := s.GetRelayer()
 	chainA, chainB := s.GetChains()
 
 	// setup 2 accounts: controller account on chain A, a second chain B account.
@@ -251,9 +249,7 @@ func (s *InterchainAccountsTestSuite) TestMsgSendTx_SuccessfulTransfer_AfterReop
 	t := s.T()
 	ctx := context.TODO()
 
-	// setup relayers and connection-0 between two chains
-	// channel-0 is a transfer channel but it will not be used in this test case
-	relayer, _ := s.SetupChainsRelayerAndChannel(ctx, nil)
+	relayer := s.GetRelayer()
 	chainA, chainB := s.GetChains()
 
 	// setup 2 accounts: controller account on chain A, a second chain B account.
@@ -442,7 +438,7 @@ func (s *InterchainAccountsTestSuite) testMsgSendTxSuccessfulGovProposal(order c
 
 	// setup relayers and connection-0 between two chains
 	// channel-0 is a transfer channel but it will not be used in this test case
-	relayer, _ := s.SetupChainsRelayerAndChannel(ctx, nil)
+	relayer := s.GetRelayer()
 	chainA, chainB := s.GetChains()
 
 	// setup 2 accounts: controller account on chain A, a second chain B account.

diff --git a/e2e/tests/interchain_accounts/gov_test.go b/e2e/tests/interchain_accounts/gov_test.go
@@ -40,9 +40,7 @@ func (s *InterchainAccountsGovTestSuite) TestInterchainAccountsGovIntegration()
 	t := s.T()
 	ctx := context.TODO()
 
-	// setup relayers and connection-0 between two chains
-	// channel-0 is a transfer channel but it will not be used in this test case
-	relayer, _ := s.SetupChainsRelayerAndChannel(ctx, nil)
+	relayer := s.GetRelayer()
 	chainA, chainB := s.GetChains()
 	controllerAccount := s.CreateUserOnChainA(ctx, testvalues.StartingTokenAmount)
 

diff --git a/e2e/tests/interchain_accounts/groups_test.go b/e2e/tests/interchain_accounts/groups_test.go
@@ -86,9 +86,7 @@ func (s *InterchainAccountsGroupsTestSuite) TestInterchainAccountsGroupsIntegrat
 		err               error
 	)
 
-	// setup relayers and connection-0 between two chains
-	// channel-0 is a transfer channel but it will not be used in this test case
-	relayer, _ := s.SetupChainsRelayerAndChannel(ctx, nil)
+	relayer := s.GetRelayer()
 	chainA, chainB := s.GetChains()
 
 	chainAWallet := s.CreateUserOnChainA(ctx, testvalues.StartingTokenAmount)

diff --git a/e2e/tests/interchain_accounts/incentivized_test.go b/e2e/tests/interchain_accounts/incentivized_test.go
@@ -40,9 +40,7 @@ func (s *IncentivizedInterchainAccountsTestSuite) TestMsgSendTx_SuccessfulBankSe
 	t := s.T()
 	ctx := context.TODO()
 
-	// setup relayers and connection-0 between two chains
-	// channel-0 is a transfer channel but it will not be used in this test case
-	relayer, _ := s.SetupChainsRelayerAndChannel(ctx, nil)
+	relayer := s.GetRelayer()
 	chainA, chainB := s.GetChains()
 
 	var (
@@ -218,9 +216,7 @@ func (s *IncentivizedInterchainAccountsTestSuite) TestMsgSendTx_FailedBankSend_I
 	t := s.T()
 	ctx := context.TODO()
 
-	// setup relayers and connection-0 between two chains
-	// channel-0 is a transfer channel but it will not be used in this test case
-	relayer, _ := s.SetupChainsRelayerAndChannel(ctx, nil)
+	relayer := s.GetRelayer()
 	chainA, chainB := s.GetChains()
 
 	var (

diff --git a/e2e/tests/interchain_accounts/localhost_test.go b/e2e/tests/interchain_accounts/localhost_test.go
@@ -42,7 +42,6 @@ func (s *LocalhostInterchainAccountsTestSuite) TestInterchainAccounts_Localhost(
 	t := s.T()
 	ctx := context.TODO()
 
-	_, _ = s.SetupChainsRelayerAndChannel(ctx, nil)
 	chainA, _ := s.GetChains()
 
 	chainADenom := chainA.Config().Denom
@@ -196,8 +195,6 @@ func (s *LocalhostInterchainAccountsTestSuite) TestInterchainAccounts_ReopenChan
 	t := s.T()
 	ctx := context.TODO()
 
-	// relayer and channel output is discarded, only a single chain is required
-	_, _ = s.SetupChainsRelayerAndChannel(ctx, nil)
 	chainA, _ := s.GetChains()
 
 	chainADenom := chainA.Config().Denom

diff --git a/e2e/tests/interchain_accounts/params_test.go b/e2e/tests/interchain_accounts/params_test.go
@@ -60,9 +60,6 @@ func (s *InterchainAccountsParamsTestSuite) TestControllerEnabledParam() {
 	t := s.T()
 	ctx := context.TODO()
 
-	// setup relayers and connection-0 between two chains
-	// channel-0 is a transfer channel but it will not be used in this test case
-	_, _ = s.SetupChainsRelayerAndChannel(ctx, nil)
 	chainA, _ := s.GetChains()
 	chainAVersion := chainA.Config().Images[0].Version
 
@@ -115,9 +112,7 @@ func (s *InterchainAccountsParamsTestSuite) TestHostEnabledParam() {
 	t := s.T()
 	ctx := context.TODO()
 
-	// setup relayers and connection-0 between two chains
-	// channel-0 is a transfer channel but it will not be used in this test case
-	relayer, _ := s.SetupChainsRelayerAndChannel(ctx, nil)
+	relayer := s.GetRelayer()
 	chainA, chainB := s.GetChains()
 	chainBVersion := chainB.Config().Images[0].Version
 

diff --git a/e2e/tests/interchain_accounts/query_test.go b/e2e/tests/interchain_accounts/query_test.go
@@ -40,7 +40,7 @@ func (s *InterchainAccountsQueryTestSuite) TestInterchainAccountsQuery() {
 
 	// setup relayers and connection-0 between two chains
 	// channel-0 is a transfer channel but it will not be used in this test case
-	relayer, _ := s.SetupChainsRelayerAndChannel(ctx, nil)
+	relayer := s.GetRelayer()
 	chainA, chainB := s.GetChains()
 	chainBVersion := chainB.Config().Images[0].Version
 

diff --git a/e2e/tests/interchain_accounts/upgrades_test.go b/e2e/tests/interchain_accounts/upgrades_test.go
@@ -45,7 +45,7 @@ func (s *InterchainAccountsChannelUpgradesTestSuite) TestMsgSendTx_SuccessfulTra
 
 	// setup relayers and connection-0 between two chains
 	// channel-0 is a transfer channel but it will not be used in this test case
-	relayer, _ := s.SetupChainsRelayerAndChannel(ctx, nil)
+	relayer := s.GetRelayer()
 	chainA, chainB := s.GetChains()
 
 	// setup 2 accounts: controller account on chain A, a second chain B account.
@@ -223,9 +223,7 @@ func (s *InterchainAccountsChannelUpgradesTestSuite) TestChannelUpgrade_ICAChann
 	t := s.T()
 	ctx := context.TODO()
 
-	// setup relayers and connection-0 between two chains
-	// channel-0 is a transfer channel but it will not be used in this test case
-	relayer, _ := s.SetupChainsRelayerAndChannel(ctx, nil)
+	relayer := s.GetRelayer()
 	chainA, chainB := s.GetChains()
 
 	// setup 2 accounts: controller account on chain A, a second chain B account.

diff --git a/e2e/tests/transfer/authz_test.go b/e2e/tests/transfer/authz_test.go
@@ -32,6 +32,10 @@ type AuthzTransferTestSuite struct {
 	testsuite.E2ETestSuite
 }
 
+func (suite *AuthzTransferTestSuite) SetupTest() {
+	suite.SetupPath(ibc.DefaultClientOpts(), suite.TransferChannelOptions())
+}
+
 // QueryGranterGrants returns all GrantAuthorizations for the given granterAddress.
 func (*AuthzTransferTestSuite) QueryGranterGrants(ctx context.Context, chain ibc.Chain, granterAddress string) ([]*authz.GrantAuthorization, error) {
 	res, err := query.GRPCQuery[authz.QueryGranterGrantsResponse](ctx, chain, &authz.QueryGranterGrantsRequest{
@@ -48,7 +52,7 @@ func (suite *AuthzTransferTestSuite) TestAuthz_MsgTransfer_Succeeds() {
 	t := suite.T()
 	ctx := context.TODO()
 
-	relayer, channelA := suite.SetupChainsRelayerAndChannel(ctx, suite.TransferChannelOptions())
+	relayer, channelA := suite.GetRelayer(), suite.GetChainAChannel()
 	chainA, chainB := suite.GetChains()
 	chainADenom := chainA.Config().Denom
 
@@ -206,7 +210,8 @@ func (suite *AuthzTransferTestSuite) TestAuthz_InvalidTransferAuthorizations() {
 	t := suite.T()
 	ctx := context.TODO()
 
-	relayer, channelA := suite.SetupChainsRelayerAndChannel(ctx, suite.TransferChannelOptions())
+	relayer, channelA := suite.GetRelayer(), suite.GetChainAChannel()
+
 	chainA, chainB := suite.GetChains()
 	chainADenom := chainA.Config().Denom
 	chainAVersion := chainA.Config().Images[0].Version