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

Tests Refactor v1: remove mock #273

Merged
merged 22 commits into from
Dec 10, 2019
Merged

Tests Refactor v1: remove mock #273

merged 22 commits into from
Dec 10, 2019

Conversation

rhuairahrighairidh
Copy link
Member

@rhuairahrighairidh rhuairahrighairidh commented Dec 3, 2019

This updates the tests to the almost-latest sdk testing style (cosmos/cosmos-sdk#4875).
Main aim was to remove mock to help with the move to module accounts (mock doesn't have a supply keeper).
Leaving update to using TestSuites (cosmos/cosmos-sdk#5257) to a future refactor.

Main changes:

  • Moved tests over to using TestApp (a wrapper around the main app) rather than setting up all the keepers they need in a test_common.go.
  • Changed initialization of TestApps in tests to use genesis states, rather than purely calling keeper methods. Not entirely sold on this change, as tests get more complex it might be easier to go back to initialization through keeper methods.
  • converted auction/app_test.go to a keeper test

TODOs for future refactors:

  • consider removing app_test.go style tests, they test things covered by keeper, handler, and cli tests
  • consider moving TestApp and helper funcs to a new package to split out of app/
  • maybe update validator-vesting tests

Progress:

  • cdp
  • liquidator
  • pricefeed
  • auction

@karzak karzak added the WIP PR is a work in progress and not ready for review label Dec 4, 2019
@rhuairahrighairidh rhuairahrighairidh added R4R When a PR is ready for review and removed WIP PR is a work in progress and not ready for review labels Dec 7, 2019
Copy link
Member

@karzak karzak left a comment

Choose a reason for hiding this comment

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

Note. The use of package_test and explicit importing of public methods on package in tests is good hygiene, but I don't think we need to strictly follow that pattern in cases where we just want simple unit tests (including unit tests of private functions).

@karzak karzak merged commit c5db0ff into develop Dec 10, 2019
@karzak karzak deleted the ro-remove-mock-from-tests branch January 15, 2020 10:42
denalimarsh pushed a commit that referenced this pull request Apr 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R4R When a PR is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants