(test) Test that tokens are successfully forwarded from A to C through B#6758
(test) Test that tokens are successfully forwarded from A to C through B#6758
Conversation
bznein
requested review from
AdityaSripal,
DimitrisJim,
chatton,
colin-axner,
crodriguezvega,
damiannolan and
gjermundgaraba
as code owners
| channelBToCCallback := func() ibc.ChannelOutput { | ||
| ctx := context.TODO() | ||
| // Creating a new path between chain B and chain C with a ICS20-v1 channel | ||
| opts := s.TransferChannelOptions() | ||
| opts.Version = transfertypes.V1 | ||
| chains := s.GetAllChains() | ||
| channelBtoC, _ := s.CreatePath(ctx, chains[1], chains[2], ibc.DefaultClientOpts(), opts) | ||
| s.Require().Equal(transfertypes.V1, channelBtoC.Version, "the channel version is not ics20-1") | ||
| return channelBtoC | ||
| } | ||
| s.testForwardingThreeChains(channelBToCCallback) | ||
| } |
There was a problem hiding this comment.
These two tests are identical, they only differ in the fact that the channel from B to C in one is the regular one we create in the setup, while in this one is a v1 channel.
I extracted all to a common function which accepts a parameter that specifies how the channel from B to C is obtained. This test overrides it, while the other one (below) simply returns the one already created.
| type ChainChannelPair struct { | ||
| ChainIdx int | ||
| ChannelIdx int | ||
| } | ||
|
|
There was a problem hiding this comment.
This might bloat a bit the file, but I'm not a fan of passing magic numbers around (it would be easy to switch them by mistake)
There was a problem hiding this comment.
agree. don't know if we have consistency on this atm, though. might need replacements elsewhere (admittedly mixed usage would be worse)
crodriguezvega
left a comment
crodriguezvega
left a comment
There was a problem hiding this comment.
Good thinking by reusing the existing test with a small modification!
| }) | ||
| } | ||
|
|
||
| func (s *TransferForwardingTestSuite) testForwardingThreeChains(channelBToCCallback func() ibc.ChannelOutput) { |
There was a problem hiding this comment.
I feel like callback is more appropriate when you want to execute something based on something that has happened. But this is just an option, happy to go with it:
| func (s *TransferForwardingTestSuite) testForwardingThreeChains(channelBToCCallback func() ibc.ChannelOutput) { | |
| func (s *TransferForwardingTestSuite) testForwardingThreeChains(createChannelBToC func() ibc.ChannelOutput) { |
There was a problem hiding this comment.
It makes sense, though I was talking to Cian on Thursday and it does not actually make sense to over-complicate by passing a function, we can leave the logic in the test (see newest commit)
e2e/testsuite/testsuite.go
Outdated
| ChainIdx int | ||
| ChannelIdx int |
There was a problem hiding this comment.
think negative values aren't handled atm (plus uints just make sense as the type for indices)
| ChainIdx int | |
| ChannelIdx int | |
| ChainIdx uint64 | |
| ChannelIdx uint64 |
| type ChainChannelPair struct { | ||
| ChainIdx int | ||
| ChannelIdx int | ||
| } | ||
|
|
There was a problem hiding this comment.
agree. don't know if we have consistency on this atm, though. might need replacements elsewhere (admittedly mixed usage would be worse)
|
…h B (#6758) * Create new forwarding test * Use a struct rather than raw fields * better docstring * Change a type and remove callback * Fix type
3 participants
Description
This was very similar to the test that performs the same but with C being
ics20-1.Since the two tests are almost identical, I refactored in a single function and just wrap them into two tests that differ slightly.
ref: #6578
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/).godoccomments.Files changedin the GitHub PR explorer.SonarCloud Reportin the comment section below once CI passes.