chore: add upgrade callback overrides for mock ibc app #3812
Conversation
damiannolan
requested review from
colin-axner,
AdityaSripal,
crodriguezvega,
charleenfei and
chatton
as code owners
| @@ -245,6 +245,7 @@ type SimApp struct { | |||
|
|
|||
| // make IBC modules public for test purposes | |||
| // these modules are never directly routed to by the IBC Router | |||
| IBCMockModule ibcmock.IBCModule | |||
There was a problem hiding this comment.
Side question, how come this wasn't present earlier?
There was a problem hiding this comment.
I believe we originally added these overrides to test middleware. One initial point of testing was with interchain accounts (to mimic "icauth" modules, v6 improvement removed notion of "icaauth"). The mock module would be wrapped however as it acts as middleware, thus you reference ICAAuthModule instead of IBCMockModule
Referencing the unwrapped mock module is only required when you don't need to test the mock module as middleware but as a base application. This only occurs when you need to revert state of the IBC app callback. This occurs in OnRecvPacket handling. Because those tests were written before this feature was added, it uses a mock.MockFailPacketData. Then logic was added to mock.OnRecvPacket to return a certain ack based on the packet data. These tests could be updated to use the override app callback. It'd probably be more stable in the long term (as you won't need to pass in a certain packet data to create a certain result)
There was a problem hiding this comment.
looks like I initially added GetMockModule() but I'm not sure when that was removed
There was a problem hiding this comment.
It was removed in this pr #380, but idk why. Might have been something as simple as a merge conflict mistake or unused func
There was a problem hiding this comment.
Yeah, must have been a merge conflict I assume. I was a little surprised when I noticed this was missing from the SimApp struct.
Description
Adds the upgrade callback overrideable functions to mock IBCApp
ref: #3791
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.