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.

Sign up for GitHub

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

Jump to bottom

Migrate Keeper staking tests to use simapp #5683

Merged
merged 95 commits into from
Feb 28, 2020
Merged

Migrate Keeper staking tests to use simapp #5683

merged 95 commits into from
Feb 28, 2020

Conversation

jgimeno
Copy link
Contributor

@jgimeno jgimeno commented Feb 20, 2020 •
edited
Loading

ref: #4875

Description

This is only for the staking/keeper migration.

For contributor use:

  • 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.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@jgimeno jgimeno added the WIP label Feb 20, 2020
golangcibot
golangcibot reviewed Feb 20, 2020
View reviewed changes
x/staking/keeper/test_common_test.go Outdated Show resolved Hide resolved
x/staking/keeper/querier_test.go Outdated Show resolved Hide resolved
x/staking/keeper/querier_test.go Outdated Show resolved Hide resolved
x/staking/keeper/querier_test.go Outdated Show resolved Hide resolved
golangcibot
golangcibot reviewed Feb 26, 2020
View reviewed changes
simapp/test_helpers.go Outdated Show resolved Hide resolved
fedekunze
fedekunze approved these changes Feb 27, 2020
View reviewed changes
Copy link
Collaborator

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

LGTM

x/staking/common_test.go Outdated Show resolved Hide resolved
simapp/test_helpers.go Outdated Show resolved Hide resolved
simapp/test_helpers.go Outdated Show resolved Hide resolved
simapp/test_helpers.go Outdated Show resolved Hide resolved
simapp/test_helpers.go Outdated Show resolved Hide resolved
simapp/test_helpers.go Outdated Show resolved Hide resolved
alexanderbez
alexanderbez suggested changes Feb 28, 2020
View reviewed changes
Copy link
Contributor

@alexanderbez alexanderbez left a comment

Choose a reason for hiding this comment

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

Amazing. Nice work @jgimeno! There are just a few tiny changes requested in simapp/test_helpers.go, otherwise it LGTM 👍

simapp/test_helpers.go Outdated
if err != nil {
panic(err)
}
//res, err = crypto.PubKeyFromBytes(pkBytes)
Copy link
Contributor

Choose a reason for hiding this comment

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

bump

simapp/test_helpers.go Show resolved Hide resolved
simapp/test_helpers.go Show resolved Hide resolved
simapp/test_helpers.go Outdated Show resolved Hide resolved
simapp/test_helpers.go Outdated Show resolved Hide resolved
simapp/test_helpers.go Outdated Show resolved Hide resolved
x/staking/keeper/keeper.go Show resolved Hide resolved
alexanderbez
alexanderbez approved these changes Feb 28, 2020
View reviewed changes
Copy link
Contributor

@alexanderbez alexanderbez left a comment

Choose a reason for hiding this comment

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

ACK 🎉

@alexanderbez alexanderbez added the A:automerge Automatically merge PR once all prerequisites pass. label Feb 28, 2020
@alexanderbez alexanderbez merged commit e358089 into master Feb 28, 2020
@alexanderbez alexanderbez deleted the jonathan/4875-staking-test-use-simapp branch February 28, 2020 20:45
@codecov
Copy link

codecov bot commented Feb 28, 2020

Codecov Report

Merging #5683 into master will increase coverage by 0.05%.
The diff coverage is 80%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5683      +/-   ##
==========================================
+ Coverage   35.63%   35.68%   +0.05%     
==========================================
  Files         332      334       +2     
  Lines       32653    32687      +34     
==========================================
+ Hits        11635    11665      +30     
- Misses      19787    19790       +3     
- Partials     1231     1232       +1
Impacted Files Coverage Δ
version/command.go 77.77% <80%> (ø)
version/version.go 100% <0%> (ø)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@golangcibot golangcibot golangcibot left review comments

@alexanderbez alexanderbez alexanderbez approved these changes

@fedekunze fedekunze fedekunze approved these changes

@alessio alessio Awaiting requested review from alessio

@jackzampolin jackzampolin Awaiting requested review from jackzampolin

@hschoenburg hschoenburg Awaiting requested review from hschoenburg

@nylira nylira Awaiting requested review from nylira

Assignees
No one assigned
Labels
A:automerge Automatically merge PR once all prerequisites pass. C:x/staking
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jgimeno @alexanderbez @fedekunze @golangcibot