e2e: v8.1 upgrade test for capital efficient escrows #5652
Conversation
damiannolan
changed the title
| @@ -59,8 +62,12 @@ func (s *UpgradeTestSuite) UpgradeChain(ctx context.Context, chain *cosmos.Cosmo | |||
| Info: fmt.Sprintf("upgrade version test from %s to %s", currentVersion, upgradeVersion), | |||
| } | |||
|
|
|||
| upgradeProposal := upgradetypes.NewSoftwareUpgradeProposal(fmt.Sprintf("upgrade from %s to %s", currentVersion, upgradeVersion), "upgrade chain E2E test", plan) | |||
| s.ExecuteAndPassGovV1Beta1Proposal(ctx, chain, wallet, upgradeProposal) | |||
There was a problem hiding this comment.
curious when it's possible to completely switch over and remove this
There was a problem hiding this comment.
That would be once v7 reaches end of life, right?
There was a problem hiding this comment.
I think so, but I'm basing it just off one of the feature releases matrices in values.go
damiannolan
requested review from
colin-axner,
AdityaSripal,
crodriguezvega,
charleenfei,
chatton and
DimitrisJim
as code owners
| @@ -12,20 +12,6 @@ on: | |||
|
|
|||
|
|
|||
| jobs: | |||
| upgrade-v5-hermes: | |||
There was a problem hiding this comment.
EOL btw, so removed both hermes and rly tests
| @@ -399,6 +400,22 @@ func (*E2ETestSuite) TransferChannelOptions() func(options *ibc.CreateChannelOpt | |||
| } | |||
| } | |||
|
|
|||
| // FeeMiddlewareChannelOptions configures both of the chains to have fee middleware enabled. | |||
| func (s *E2ETestSuite) FeeMiddlewareChannelOptions() func(options *ibc.CreateChannelOptions) { | |||
There was a problem hiding this comment.
I took the liberty to move this function here.
There was a problem hiding this comment.
Thank you @crodriguezvega!
There was a problem hiding this comment.
Great work! I pushed some changes to address a couple of review comments, and the tests still pass, so giving my approval now.
| return nil, err | ||
| } | ||
|
|
||
| // be ashamed, be very ashamed |
There was a problem hiding this comment.
❤️
This is not actually the worst thing ATM, I think we can improve this, but this ended up being very readable and concise compared to the other options we were playing with.
There was a problem hiding this comment.
yea, agree! let's leave in the comment for comical effect I guess, would be nice to not have to add a new func for every field we need to sanitise, but we can think about that later - string splitting on a jsonPath with "." is a bit gross but I'm not sure I have any better ideas atm!
Description
closes: #5635
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/) or specification (x/<module>/spec/).godoccomments.Files changedin the Github PR explorer.Codecov Reportin the comment section below once CI passes.