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

testing: e2e support for sdkv50 #4174

Closed

Conversation

faddat
Copy link
Contributor

@faddat faddat commented Jul 25, 2023

This PR begins work to make the e2e tests support sdk 50,
based on this PR to interchaintest:

strangelove-ventures/interchaintest#674

@charleenfei charleenfei changed the title faddat/e2e ibc8 testing: e2e support for sdkv50 Aug 24, 2023
@charleenfei charleenfei requested a review from srdtrk as a code owner August 24, 2023 19:16
@@ -271,8 +271,8 @@ func (s *E2ETestSuite) QueryProposalV1(ctx context.Context, chain ibc.Chain, pro

// GetBlockHeaderByHeight fetches the block header at a given height.
func (s *E2ETestSuite) GetBlockHeaderByHeight(ctx context.Context, chain ibc.Chain, height uint64) (Header, error) {
cmtservice := s.GetChainGRCPClients(chain).ConsensusServiceClient
res, err := cmtservice.GetBlockByHeight(ctx, &ccmtservice.GetBlockByHeightRequest{
ccmtservice := s.GetChainGRCPClients(chain).ConsensusServiceClient
Copy link
Contributor

Choose a reason for hiding this comment

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

are the diffs here just for the rename? any reason why we do them?

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, conflicts with the import for the client I'd guess "github.com/cosmos/cosmos-sdk/client/grpc/cmtservice" and linter is muy triste about it, leaving comment here so others don't ask same thing

Comment on lines -294 to -295
// GetExpectedEvent returns the expected event for a callback.
func GetExpectedEvent(
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be added back, looks like maybe it got removed accidentally?

Copy link
Contributor

Choose a reason for hiding this comment

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

is it actually being used anywhere?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yea, I will add back. The usage was deleted :)

@mergify mergify bot mentioned this pull request Aug 28, 2023
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants