Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add fee middleware test suite functions (E2E #5) #1710

Merged
merged 47 commits into from
Jul 19, 2022
Merged
Show file tree
Hide file tree
Changes from 44 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
47be6be
feat(e2e): adding script to dynamically generate list of e2e tests
chatton Jul 4, 2022
ce80411
feat(e2e): adding github CI to run E2E tests
chatton Jul 4, 2022
bfbba03
feat(e2e) adding placeholder test for github action to execute
chatton Jul 4, 2022
4569fa5
wip: extracting correct tag for image
chatton Jul 4, 2022
272ad61
wip: extracting correct tag for image
chatton Jul 4, 2022
6afe90c
wip: corrected workflow syntax
chatton Jul 4, 2022
7497574
wip: corrected workflow syntax
chatton Jul 4, 2022
7091fbc
removed unreferenced job
chatton Jul 4, 2022
6ebcc6d
wip: adding makefile
chatton Jul 4, 2022
05d9a45
wip: displaying env vars in placeholder test
chatton Jul 4, 2022
308eb0f
wip: adding go script to determine simd tag
chatton Jul 4, 2022
9ebacd7
fix: corrected outputs name in test.yml
chatton Jul 4, 2022
c431850
fix: specifying correct output name
chatton Jul 4, 2022
3c4f62b
fix: updating go script to accept a single argument
chatton Jul 4, 2022
b5fff2a
chore: greatly simplifying determine_simd_tag.go
chatton Jul 4, 2022
abb0eca
chore: adding docstring to determine_simd_tag.go
chatton Jul 4, 2022
f36025b
chore: extract output variable
chatton Jul 4, 2022
9039137
chore: adding echo to display the tag in the workflow
chatton Jul 4, 2022
30591c9
feat(e2e): adding E2ETestSuite
chatton Jul 4, 2022
b43d07e
wip: temporarily remove entrypoint in dockerfile
chatton Jul 4, 2022
b1bcc12
chore: removing SkipPathCreation
chatton Jul 4, 2022
d740296
removed Req and eRep member fields
chatton Jul 4, 2022
e94d35b
merge main
chatton Jul 6, 2022
848f888
chore: adding docstrings and cleaning up E2ETestSuite
chatton Jul 6, 2022
99328b1
merge main
chatton Jul 8, 2022
35e454f
chore: correcting docstring
chatton Jul 8, 2022
ccd9fd5
chore: updating test.yml to pass only the pr
chatton Jul 8, 2022
e154f09
chore: upgrading go to 1.18 for e2e test
chatton Jul 8, 2022
236eb9f
Merge branch 'main' into cian/issue#1648-add-baseline-e2e-test-suite-…
chatton Jul 12, 2022
11227eb
Merge branch 'main' into cian/issue#1648-add-baseline-e2e-test-suite-…
chatton Jul 13, 2022
7541beb
Merge branch 'main' into cian/issue#1648-add-baseline-e2e-test-suite-…
chatton Jul 13, 2022
e768a46
chore: addressing PR feedback
chatton Jul 14, 2022
ce2828e
Merge branch 'cian/issue#1648-add-baseline-e2e-test-suite-type-(e2e-#…
chatton Jul 14, 2022
2f77cd3
chore: adding separate go.mod for e2e tests
chatton Jul 14, 2022
0cfac25
chore: renaming module name to e2e
chatton Jul 14, 2022
5753fa4
chore: remving commented out line from Dockerfile
chatton Jul 14, 2022
9ebbed4
chore: adding separate Makefile for e2e tests
chatton Jul 14, 2022
58a0fd5
chore: adding newline
chatton Jul 14, 2022
9b2ffba
chore: merge main
chatton Jul 14, 2022
1c2e239
chore: handling merge conflicts
chatton Jul 14, 2022
da2c8b9
chore: uncommenting line from Dockerfile
chatton Jul 14, 2022
3b87fd8
chore(e2e): adding fee middleware test functions and broadcast functi…
chatton Jul 15, 2022
b253b8d
chore: merge main
chatton Jul 15, 2022
42e96be
Merge branch 'main' into cian/add-fee-middleware-test-suite-functions
chatton Jul 15, 2022
196cfc7
chore: addressing PR feedback
chatton Jul 19, 2022
24abfa1
Merge branch 'cian/add-fee-middleware-test-suite-functions' of https:…
chatton Jul 19, 2022
b6a6d01
Merge branch 'main' into cian/add-fee-middleware-test-suite-functions
chatton Jul 19, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 45 additions & 1 deletion e2e/fee_middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,16 @@ import (
"context"
"testing"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/strangelove-ventures/ibctest/broadcast"
"github.com/strangelove-ventures/ibctest/chain/cosmos"
"github.com/strangelove-ventures/ibctest/ibc"
"github.com/stretchr/testify/suite"

"e2e/testsuite"

feetypes "github.com/cosmos/ibc-go/v4/modules/apps/29-fee/types"
channeltypes "github.com/cosmos/ibc-go/v4/modules/core/04-channel/types"
)

func TestFeeMiddlewareTestSuite(t *testing.T) {
Expand All @@ -18,13 +24,51 @@ type FeeMiddlewareTestSuite struct {
testsuite.E2ETestSuite
}

// RegisterCounterPartyPayee broadcasts a MsgRegisterCounterpartyPayee message.
func (s *FeeMiddlewareTestSuite) RegisterCounterPartyPayee(ctx context.Context, chain *cosmos.CosmosChain, user broadcast.User, portID, channelID, relayerAddr, counterpartyPayeeAddr string) (sdk.TxResponse, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With so many arguments the line becomes quite long, should we split it in multiple lines for better readability?

msg := feetypes.NewMsgRegisterCounterpartyPayee(portID, channelID, relayerAddr, counterpartyPayeeAddr)
return s.BroadcastMessages(ctx, chain, user, msg)
}

// QueryCounterPartyPayee queries the counterparty payee of the given chain and relayer address on the specified channel.
func (s *FeeMiddlewareTestSuite) QueryCounterPartyPayee(ctx context.Context, chain ibc.Chain, relayerAddress, channelID string) (string, error) {
queryClient := s.GetChainGRCPClientSet(chain).FeeQueryClient
res, err := queryClient.CounterpartyPayee(ctx, &feetypes.QueryCounterpartyPayeeRequest{
ChannelId: channelID,
Relayer: relayerAddress,
})

if err != nil {
return "", err
}
return res.CounterpartyPayee, nil
}

// PayPacketFeeAsync broadcasts a MsgPayPacketFeeAsync message.
func (s *FeeMiddlewareTestSuite) PayPacketFeeAsync(ctx context.Context, chain *cosmos.CosmosChain, user broadcast.User, packetID channeltypes.PacketId, packetFee feetypes.PacketFee) (sdk.TxResponse, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto on @crodriguezvega's point

msg := feetypes.NewMsgPayPacketFeeAsync(packetID, packetFee)
return s.BroadcastMessages(ctx, chain, user, msg)
}

// QueryIncentivizedPacketsForChannel queries the incentivized packets on the specified channel.
func (s *FeeMiddlewareTestSuite) QueryIncentivizedPacketsForChannel(ctx context.Context, chain *cosmos.CosmosChain, portId, channelId string) ([]*feetypes.IdentifiedPacketFees, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto on @crodriguezvega's point

queryClient := s.GetChainGRCPClientSet(chain).FeeQueryClient
res, err := queryClient.IncentivizedPacketsForChannel(ctx, &feetypes.QueryIncentivizedPacketsForChannelRequest{
PortId: portId,
ChannelId: channelId,
})
if err != nil {
return nil, err
}
return res.IncentivizedPackets, err
}

func (s *FeeMiddlewareTestSuite) TestPlaceholder() {
ctx := context.Background()
r := s.SetupChainsRelayerAndChannel(ctx, feeMiddlewareChannelOptions())
s.T().Run("start relayer", func(t *testing.T) {
s.StartRelayer(r)
})

}

// feeMiddlewareChannelOptions configures both of the chains to have fee middleware enabled.
Expand Down
62 changes: 61 additions & 1 deletion e2e/testsuite/testsuite.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,23 @@ import (
"strings"
"time"

"e2e/testconfig"

sdk "github.com/cosmos/cosmos-sdk/types"
dockerclient "github.com/docker/docker/client"
"github.com/strangelove-ventures/ibctest"
"github.com/strangelove-ventures/ibctest/broadcast"
"github.com/strangelove-ventures/ibctest/chain/cosmos"
"github.com/strangelove-ventures/ibctest/ibc"
"github.com/strangelove-ventures/ibctest/test"
"github.com/strangelove-ventures/ibctest/testreporter"
"github.com/stretchr/testify/suite"
"go.uber.org/zap"
"go.uber.org/zap/zaptest"
"google.golang.org/grpc"
"google.golang.org/grpc/credentials/insecure"

"e2e/testconfig"
feetypes "github.com/cosmos/ibc-go/v4/modules/apps/29-fee/types"
)

const (
Expand All @@ -29,13 +36,18 @@ const (
// E2ETestSuite has methods and functionality which can be shared among all test suites.
type E2ETestSuite struct {
suite.Suite
grpcClientSets map[string]GRPCClientSet
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mega nit but should we add a line break after suite.Suite...?

paths map[string]path
logger *zap.Logger
DockerClient *dockerclient.Client
network string
startRelayerFn func(relayer ibc.Relayer)
}

type GRPCClientSet struct {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think we should add godocs to all exported types

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reasoning behind the naming of having the Set suffix? Initially when I read this I thought it was supposed to be a set data structure but I'm slightly confused

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intention is just that is a group of clients, however I can see where the confusion is coming from.

After some searching it looks like the term clientSet isn't that common (used a lot in k8s ecosystem) I can rename to maybe just GRPCClients does that sound good to you?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep sounds good to me! :) Thanks 🤝

FeeQueryClient feetypes.QueryClient
}

// path is a pairing of two chains which will be used in a test.
type path struct {
chainA, chainB *cosmos.CosmosChain
Expand Down Expand Up @@ -102,6 +114,9 @@ func (s *E2ETestSuite) SetupChainsRelayerAndChannel(ctx context.Context, channel
time.Sleep(time.Second * 10)
}

s.initClientSet(chainA)
s.initClientSet(chainB)

return r
}

Expand Down Expand Up @@ -129,6 +144,20 @@ func (s *E2ETestSuite) GetChains(chainOpts ...testconfig.ChainOptionConfiguratio
return path.chainA, path.chainB
}

// BroadcastMessages broadcasts the provided messages to the given chain and signs them on behalf of the provided user.
// Once the broadcast response is returned, we wait for a few blocks to be created on both chain A and chain B.
func (s *E2ETestSuite) BroadcastMessages(ctx context.Context, chain *cosmos.CosmosChain, user broadcast.User, msgs ...sdk.Msg) (sdk.TxResponse, error) {
broadcaster := cosmos.NewBroadcaster(s.T(), chain)
resp, err := ibctest.BroadcastTx(ctx, broadcaster, user, msgs...)
if err != nil {
return sdk.TxResponse{}, err
}

chainA, chainB := s.GetChains()
err = test.WaitForBlocks(ctx, 2, chainA, chainB)
return resp, err
}

// GetRelayerWallets returns the relayer wallets associated with the chains.
func (s *E2ETestSuite) GetRelayerWallets(relayer ibc.Relayer) (ibc.RelayerWallet, ibc.RelayerWallet, error) {
chainA, chainB := s.GetChains()
Expand Down Expand Up @@ -196,6 +225,37 @@ func (s *E2ETestSuite) GetChainBNativeBalance(ctx context.Context, user *ibctest
return GetNativeChainBalance(ctx, chainB, user)
}

// GetChainGRCPClientSet gets the GRPC clientset associated with the given chain.
func (s *E2ETestSuite) GetChainGRCPClientSet(chain ibc.Chain) GRPCClientSet {
cs, ok := s.grpcClientSets[chain.Config().ChainID]
s.Require().True(ok, "chain %s does not have a GRPC clientset", chain.Config().ChainID)
return cs
}

// initClientSet establishes GRPC clients with the given chain.
// The created GRPCClientSet can be retreived with GetChainGRCPClientSet.
func (s *E2ETestSuite) initClientSet(chain *cosmos.CosmosChain) {
// Create a connection to the gRPC server.
grpcConn, err := grpc.Dial(
chain.GetHostGRPCAddress(),
grpc.WithTransportCredentials(insecure.NewCredentials()),
)
s.Require().NoError(err)
s.T().Cleanup(func() {
if err := grpcConn.Close(); err != nil {
s.T().Logf("failed closing GRPC connection to chain %s: %s", chain.Config().ChainID, err)
}
})

if s.grpcClientSets == nil {
s.grpcClientSets = map[string]GRPCClientSet{}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason that we should not use make(map[string]GRPCClientSet) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope! I can change it, I think make(...) syntax is more common in the codebase.

}

s.grpcClientSets[chain.Config().ChainID] = GRPCClientSet{
FeeQueryClient: feetypes.NewQueryClient(grpcConn),
}
}

// createCosmosChains creates two separate chains in docker containers.
// test and can be retrieved with GetChains.
func (s *E2ETestSuite) createCosmosChains(chainOptions testconfig.ChainOptions) (*cosmos.CosmosChain, *cosmos.CosmosChain) {
Expand Down