osmosis icon indicating copy to clipboard operation
osmosis copied to clipboard

x/gamm: Add PoolI mocks and test for inactive pools

Open rrrliu opened this issue 3 years ago • 2 comments

Closes: #2062

What is the purpose of the change

Following up to #2050, add tests to ensure swaps fail when IsActive returns false. Requires mocking out the PoolI interface, so used gomock package for as per suggestion in https://github.com/osmosis-labs/osmosis/issues/1904#issuecomment-1182730437.

Brief Changelog

  • gomock package in go.mod
  • mock_pool.go module for mocked PoolI interface
  • Added new test for inactive pool swaps in swap_tests.go

Testing and Verifying

This change added tests and can be verified as follows:

  • Extended swap_tests.go by adding TestInactivePoolFreezeSwaps

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes? (no)
  • Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md? (no)
  • How is the feature or change documented? (not applicable, also generated mock files are somewhat self-documenting)

rrrliu avatar Jul 20 '22 08:07 rrrliu

@mattverse thanks for the suggestions, addressed your comments! Lmk if you want me to add anything about gomock in any documentation.

rrrliu avatar Aug 02 '22 08:08 rrrliu

Hi @rrrliu . Great work on this PR! Just wanted to check in and see if you have time to address the last 2 suggestions. We can merge this right after :rocket:

p0mvn avatar Aug 10 '22 13:08 p0mvn

Hi @rrrliu . Great work on this PR! Just wanted to check in and see if you have time to address the last 2 suggestions. We can merge this right after 🚀

Just pushed -- sorry for the long turnaround!

rrrliu avatar Aug 15 '22 16:08 rrrliu

@rrrliu thanks and np! Seems that there is a CI failure due to merge conflict, do you mind fixing that test, please? We can merge this right after

p0mvn avatar Aug 15 '22 19:08 p0mvn