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

R4R Run missing invariants during simulations #4080

Merged
merged 8 commits into from
Apr 10, 2019
Merged

Conversation

rigelrozanski
Copy link
Contributor

@rigelrozanski rigelrozanski commented Apr 9, 2019

just noticed there were a bunch of missing simulatulations in some moreless hidden code ;)

  • Targeted PR against correct branch (see CONTRIBUTING.md)

  • Linked to github-issue with discussion and accepted design OR link to spec that describes this work.

  • Wrote tests

  • Updated relevant documentation (docs/)

  • Added a relevant changelog entry: sdkch add [section] [stanza] [message]

  • rereviewed Files changed in the github PR explorer


For Admin Use:

  • Added appropriate labels to PR (ex. wip, ready-for-review, docs)
  • Reviewers Assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@codecov
Copy link

codecov bot commented Apr 9, 2019

Codecov Report

Merging #4080 into develop will decrease coverage by 0.03%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #4080      +/-   ##
===========================================
- Coverage    59.98%   59.95%   -0.04%     
===========================================
  Files          212      211       -1     
  Lines        15123    15111      -12     
===========================================
- Hits          9072     9060      -12     
  Misses        5431     5431              
  Partials       620      620

@rigelrozanski rigelrozanski changed the title Run missing invariants during simulations R4R Run missing invariants during simulations Apr 10, 2019
@@ -39,3 +39,12 @@ func (k *Keeper) RegisterRoute(moduleName, route string, invar sdk.Invariant) {
func (k Keeper) Routes() []InvarRoute {
return k.routes
}

// return all the invariants
func (k Keeper) Invariants() []sdk.Invariant {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we have a unit test for just this function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I gracefully decline, I really can't see how this is useful whatsoever, I'll add a // DONTCOVER though

Copy link
Contributor

@alessio alessio left a comment

Choose a reason for hiding this comment

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

Just a minor remark - looks good otherwise

simulation.PeriodicInvariant(staking.AllInvariants(app.stakingKeeper, app.feeCollectionKeeper,
app.distrKeeper, app.accountKeeper), period, 0),
}
return simulation.PeriodicInvariants(app.crisisKeeper.Invariants(), period, 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

++

x/simulation/util.go Outdated Show resolved Hide resolved
x/crisis/keeper.go Outdated Show resolved Hide resolved
@alessio alessio merged commit 02a0e39 into develop Apr 10, 2019
@alessio alessio deleted the rigel/all-sim-invar branch April 10, 2019 01:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants