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

Refactor keeper tests in x/mint #1952

Merged
merged 9 commits into from
Jul 7, 2022
Merged

Refactor keeper tests in x/mint #1952

merged 9 commits into from
Jul 7, 2022

Conversation

vuong177
Copy link
Contributor

@vuong177 vuong177 commented Jul 4, 2022

Closes: #1915

What is the purpose of the change

  • Refactor keeper tests in x/mint

Brief Changelog

  • Refactor TestMintCoinsToFeeCollectorAndGetProportions and TestDistrAssetToDeveloperRewardsAddrWhenNotEmpty

Testing and Verifying

  • This change is a trivial rework / code cleanup without any test coverage.

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

@vuong177 vuong177 marked this pull request as ready for review July 5, 2022 16:13
@vuong177 vuong177 requested a review from a team July 5, 2022 16:13
@vuong177 vuong177 changed the title Vuong/mintkeepers Refactor keeper tests in pool-incentives Jul 5, 2022
@vuong177 vuong177 changed the title Refactor keeper tests in pool-incentives Refactor keeper tests in x/mint Jul 5, 2022
Copy link
Member

@p0mvn p0mvn left a comment

Choose a reason for hiding this comment

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

Great work so far! I would like to ask if we could avoid using a callback in the first test?

x/mint/keeper/keeper_test.go Outdated Show resolved Hide resolved
x/mint/keeper/keeper_test.go Show resolved Hide resolved
x/mint/keeper/keeper_test.go Outdated Show resolved Hide resolved
Copy link
Member

@mattverse mattverse left a comment

Choose a reason for hiding this comment

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

Looking good! Added some comments! Please take a look!

x/mint/keeper/keeper_test.go Show resolved Hide resolved
x/mint/keeper/keeper_test.go Outdated Show resolved Hide resolved
x/mint/keeper/keeper_test.go Outdated Show resolved Hide resolved
x/mint/keeper/keeper_test.go Show resolved Hide resolved
Copy link
Member

@mattverse mattverse left a comment

Choose a reason for hiding this comment

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

🌮

x/mint/keeper/keeper_test.go Outdated Show resolved Hide resolved
Copy link
Member

@p0mvn p0mvn left a comment

Choose a reason for hiding this comment

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

🚀

@p0mvn p0mvn merged commit ed55aeb into osmosis-labs:main Jul 7, 2022
@p0mvn p0mvn added the A:backport/v10.x backport patches to v10.x branch label Jul 19, 2022
mergify bot pushed a commit that referenced this pull request Jul 19, 2022
* TestMintCoinsToFeeCollectorAndGetProportions refactor

* TestDistrAssetToDeveloperRewardsAddrWhenNotEmpty refactor

* add feePool and feeCollector check

* comment

* use hasPreExistingSupply and better readable mintKeeper

* Update x/mint/keeper/keeper_test.go

Co-authored-by: Matt, Park <[email protected]>

* Update x/mint/keeper/keeper_test.go

Co-authored-by: Matt, Park <[email protected]>

* add testcase when nil dev reward address

* use shorter keeper name for better readability

Co-authored-by: Matt, Park <[email protected]>
(cherry picked from commit ed55aeb)
p0mvn pushed a commit that referenced this pull request Jul 19, 2022
* TestMintCoinsToFeeCollectorAndGetProportions refactor

* TestDistrAssetToDeveloperRewardsAddrWhenNotEmpty refactor

* add feePool and feeCollector check

* comment

* use hasPreExistingSupply and better readable mintKeeper

* Update x/mint/keeper/keeper_test.go

Co-authored-by: Matt, Park <[email protected]>

* Update x/mint/keeper/keeper_test.go

Co-authored-by: Matt, Park <[email protected]>

* add testcase when nil dev reward address

* use shorter keeper name for better readability

Co-authored-by: Matt, Park <[email protected]>
(cherry picked from commit ed55aeb)

Co-authored-by: vuong <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:backport/v10.x backport patches to v10.x branch C:x/mint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

x/mint: understand and refactor keeper tests
3 participants