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

test(gov): Refactor x/gov keeper tests to use mocks #12988

Merged
merged 26 commits into from
Aug 25, 2022
Merged

test(gov): Refactor x/gov keeper tests to use mocks #12988

merged 26 commits into from
Aug 25, 2022

Conversation

amaury1093
Copy link
Contributor

@amaury1093 amaury1093 commented Aug 22, 2022

Description

ref: #12752


Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • included the correct type prefix in the PR title
  • added ! to the type prefix if API or client breaking change
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • followed the guidelines for building modules
  • included the necessary unit and integration tests
  • added a changelog entry to CHANGELOG.md
  • included comments for documenting Go code
  • updated the relevant documentation or specification
  • reviewed "Files changed" and left comments if necessary
  • confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed ! in the type prefix if API or client breaking change
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic
  • reviewed API design and naming
  • reviewed documentation is accurate
  • reviewed tests and test coverage
  • manually tested (if applicable)

@codecov
Copy link

codecov bot commented Aug 24, 2022

Codecov Report

Merging #12988 (0ae0cdc) into main (4fe7797) will decrease coverage by 0.29%.
The diff coverage is 58.24%.

❗ Current head 0ae0cdc differs from pull request most recent head 4aa3b92. Consider uploading reports for the commit 4aa3b92 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #12988      +/-   ##
==========================================
- Coverage   55.87%   55.58%   -0.30%     
==========================================
  Files         646      644       -2     
  Lines       54895    54935      +40     
==========================================
- Hits        30675    30535     -140     
- Misses      21762    21932     +170     
- Partials     2458     2468      +10     
Impacted Files Coverage Δ
baseapp/abci.go 64.09% <0.00%> (-0.33%) ⬇️
client/flags/flags.go 19.35% <0.00%> (-0.32%) ⬇️
client/rpc/status.go 66.66% <ø> (ø)
client/utils.go 34.92% <0.00%> (ø)
server/config/config.go 38.00% <0.00%> (-1.59%) ⬇️
server/rosetta/client_online.go 1.35% <0.00%> (ø)
server/swagger.go 0.00% <0.00%> (ø)
testutil/list.go 0.00% <0.00%> (ø)
types/result.go 76.74% <ø> (ø)
x/auth/tx/query.go 0.00% <ø> (ø)
... and 69 more

@amaury1093 amaury1093 mentioned this pull request Aug 25, 2022
2 tasks
@amaury1093 amaury1093 marked this pull request as ready for review August 25, 2022 13:44
@amaury1093 amaury1093 requested a review from a team as a code owner August 25, 2022 13:44
Comment on lines +19 to +23
// KeeperTestSuite only tests gov's keeper logic around tallying, since it
// relies on complex interactions with x/staking.
//
// It also uses simapp (and not a depinjected app) because we manually set a
// new app.StakingKeeper in `createValidators`.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only tallying tests have been moved here. I don't think it's easy to mock them.

Comment on lines +20 to +25
// BondDenomProvider is a subset of the staking keeper's public interface that
// provides the staking bond denom. It is used in arguments in this package's
// functions so that a mock staking keeper can be passed instead of the real one.
type BondDenomProvider interface {
BondDenom(ctx sdk.Context) string
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any objections to this? Or better ideas?

)

func (suite *KeeperTestSuite) TestSubmitProposalReq() {
govAcct := suite.app.GovKeeper.GetGovernanceAccount(suite.ctx).GetAddress()
suite.reset()
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 added this line in some tests. If I don't, I have an off-by-1 error in proposal ID, only when running the whole suite. Like some other test got run before and created a proposal or something

Resetting makes each test more independent.

Copy link
Member

@julienrbrt julienrbrt left a comment

Choose a reason for hiding this comment

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

lgtm!

@julienrbrt julienrbrt added the A:automerge Automatically merge PR once all prerequisites pass. label Aug 25, 2022
@mergify mergify bot merged commit fc7ee0b into main Aug 25, 2022
@mergify mergify bot deleted the am/gov-mocks branch August 25, 2022 20:15
Eengineer1 added a commit to cheqd/cosmos-sdk that referenced this pull request Aug 26, 2022
commit d11196a
Author: Robert Zaremba <robert@zaremba.ch>
Date:   Fri Aug 26 20:05:39 2022 +0200

    deps: bump gin-gonic (cosmos#13061)

    Co-authored-by: Julien Robert <julien@rbrt.fr>

commit 20b947e
Author: Daniel Wedul <github@wedul.com>
Date:   Fri Aug 26 11:29:58 2022 -0600

    fix(x/gov): Return ErrInvalidProposalContent in SubmitProposal when legacy handler returns an error. (cosmos#13051)

    * [13030]: When a legacy gov proposal fails it's handler check, wrap the error in a ErrInvalidProposalContent (instead of ErrNoProposalHandlerExists).

    * [13030]: Add changelog entry.

    * [13030]: If the error is already a ErrNoProposalHandlerExists, just return it. Otherwise, wrap it as a ErrInvalidProposalContent.

    Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>

commit fc7ee0b
Author: Amaury <1293565+amaurym@users.noreply.github.com>
Date:   Thu Aug 25 22:15:44 2022 +0200

    test(gov): Refactor x/gov keeper tests to use mocks (cosmos#12988)

    ## Description

    ref: cosmos#12752

    - [x] Move some keeper tests into `tests/integration/gov/keeper`, see cosmos#12988 (comment)
    - [x] Don't use simapp in `x/gov/keeper` tests, use mocks instead
    - [x] Find 0 "simapp" occurence in x/gov/keeper. (Note: other occurrences of simapp are removed in  cosmos#13043)

    ---

    ### Author Checklist

    *All items are required. Please add a note to the item if the item is not applicable and
    please add links to any relevant follow up issues.*

    I have...

    - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
    - [ ] added `!` to the type prefix if API or client breaking change
    - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#pr-targeting))
    - [ ] provided a link to the relevant issue or specification
    - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/main/docs/building-modules)
    - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#testing)
    - [ ] added a changelog entry to `CHANGELOG.md`
    - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
    - [ ] updated the relevant documentation or specification
    - [ ] reviewed "Files changed" and left comments if necessary
    - [ ] confirmed all CI checks have passed

    ### Reviewers Checklist

    *All items are required. Please add a note if the item is not applicable and please add
    your handle next to the items reviewed if you only reviewed selected items.*

    I have...

    - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
    - [ ] confirmed `!` in the type prefix if API or client breaking change
    - [ ] confirmed all author checklist items have been addressed
    - [ ] reviewed state machine logic
    - [ ] reviewed API design and naming
    - [ ] reviewed documentation is accurate
    - [ ] reviewed tests and test coverage
    - [ ] manually tested (if applicable)

commit 1fb6f0b
Author: Daniel Wedul <github@wedul.com>
Date:   Thu Aug 25 13:43:40 2022 -0600

    feat(x/auth): Add auth sim decoder case for AccountNumberStoreKeyPrefix (cosmos#13048)

    * [13028]: Add auth sim decoder case for AccountNumberStoreKeyPrefix (and a unit test case for it).

    * [13028]: Add changelog entry.

commit 5e4651e
Author: Daniel Wedul <github@wedul.com>
Date:   Thu Aug 25 13:17:46 2022 -0600

    feat(x/authz): Add the GetAuthorization function. (cosmos#13047)

    * [13027]: Create the GetAuthorization function (in the authz module).

    * [13027]: Add unit tests for the new GetAuthorization function.

    * [13027]: Add changelog entry.

commit c73171f
Author: Julien Robert <julien@rbrt.fr>
Date:   Thu Aug 25 17:54:50 2022 +0200

    fix: missing return statement in BaseApp.Query (cosmos#13046)

    ## Description

    Closes: cosmos#13040

    ---

    ### Author Checklist

    *All items are required. Please add a note to the item if the item is not applicable and
    please add links to any relevant follow up issues.*

    I have...

    - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
    - [ ] added `!` to the type prefix if API or client breaking change
    - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#pr-targeting))
    - [ ] provided a link to the relevant issue or specification
    - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/main/docs/building-modules)
    - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#testing)
    - [ ] added a changelog entry to `CHANGELOG.md`
    - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
    - [ ] updated the relevant documentation or specification
    - [ ] reviewed "Files changed" and left comments if necessary
    - [ ] confirmed all CI checks have passed

    ### Reviewers Checklist

    *All items are required. Please add a note if the item is not applicable and please add
    your handle next to the items reviewed if you only reviewed selected items.*

    I have...

    - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
    - [ ] confirmed `!` in the type prefix if API or client breaking change
    - [ ] confirmed all author checklist items have been addressed
    - [ ] reviewed state machine logic
    - [ ] reviewed API design and naming
    - [ ] reviewed documentation is accurate
    - [ ] reviewed tests and test coverage
    - [ ] manually tested (if applicable)

commit e1999e4
Author: JayT106 <JayT106@users.noreply.github.com>
Date:   Thu Aug 25 17:09:52 2022 +0800

    fix: exporting the blockParams regression (cosmos#13029)

    ## Description

    Closes: cosmos#13024

    ---

    ### Author Checklist

    *All items are required. Please add a note to the item if the item is not applicable and
    please add links to any relevant follow up issues.*

    I have...

    - [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
    - [x] added `!` to the type prefix if API or client breaking change
    - [x] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#pr-targeting))
    - [x] provided a link to the relevant issue or specification
    - [x] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/main/docs/building-modules)
    - [x] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#testing)
    - [x] added a changelog entry to `CHANGELOG.md`
    - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
    - [ ] updated the relevant documentation or specification
    - [x] reviewed "Files changed" and left comments if necessary
    - [ ] confirmed all CI checks have passed

    ### Reviewers Checklist

    *All items are required. Please add a note if the item is not applicable and please add
    your handle next to the items reviewed if you only reviewed selected items.*

    I have...

    - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
    - [ ] confirmed `!` in the type prefix if API or client breaking change
    - [ ] confirmed all author checklist items have been addressed
    - [ ] reviewed state machine logic
    - [ ] reviewed API design and naming
    - [ ] reviewed documentation is accurate
    - [ ] reviewed tests and test coverage
    - [ ] manually tested (if applicable)

commit 6af89d6
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Wed Aug 24 23:13:11 2022 +0200

    build(deps): Bump github.com/golangci/golangci-lint from 1.48.0 to 1.49.0 (cosmos#13013)

    * build(deps): Bump github.com/golangci/golangci-lint

    Bumps [github.com/golangci/golangci-lint](https://github.com/golangci/golangci-lint) from 1.48.0 to 1.49.0.
    - [Release notes](https://github.com/golangci/golangci-lint/releases)
    - [Changelog](https://github.com/golangci/golangci-lint/blob/master/CHANGELOG.md)
    - [Commits](golangci/golangci-lint@v1.48.0...v1.49.0)

    ---
    updated-dependencies:
    - dependency-name: github.com/golangci/golangci-lint
      dependency-type: direct:production
      update-type: version-update:semver-minor
    ...

    Signed-off-by: dependabot[bot] <support@github.com>

    * updates

    Signed-off-by: dependabot[bot] <support@github.com>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
    Co-authored-by: Julien Robert <julien@rbrt.fr>

commit d8a7dbd
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Wed Aug 24 21:47:14 2022 +0200

    build(deps): Bump google.golang.org/grpc in /client/v2 (cosmos#13021)

    Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.48.0 to 1.49.0.
    - [Release notes](https://github.com/grpc/grpc-go/releases)
    - [Commits](grpc/grpc-go@v1.48.0...v1.49.0)

    ---
    updated-dependencies:
    - dependency-name: google.golang.org/grpc
      dependency-type: direct:production
      update-type: version-update:semver-minor
    ...

    Signed-off-by: dependabot[bot] <support@github.com>

    Signed-off-by: dependabot[bot] <support@github.com>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

commit 4fbd246
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Wed Aug 24 21:31:50 2022 +0200

    build(deps): Bump github.com/cosmos/cosmos-sdk in /store/tools/ics23 (cosmos#13020)

    Bumps [github.com/cosmos/cosmos-sdk](https://github.com/cosmos/cosmos-sdk) from 0.46.0 to 0.46.1.
    - [Release notes](https://github.com/cosmos/cosmos-sdk/releases)
    - [Changelog](https://github.com/cosmos/cosmos-sdk/blob/v0.46.1/CHANGELOG.md)
    - [Commits](cosmos/cosmos-sdk@v0.46.0...v0.46.1)

    ---
    updated-dependencies:
    - dependency-name: github.com/cosmos/cosmos-sdk
      dependency-type: direct:production
      update-type: version-update:semver-patch
    ...

    Signed-off-by: dependabot[bot] <support@github.com>

    Signed-off-by: dependabot[bot] <support@github.com>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

commit 55307d6
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Wed Aug 24 21:02:14 2022 +0200

    build(deps): Bump google.golang.org/grpc in /errors (cosmos#13017)

commit 4d2fc82
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Wed Aug 24 20:48:36 2022 +0200

    build(deps): Bump google.golang.org/grpc from 1.48.0 to 1.49.0 in /orm (cosmos#13016)

    Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.48.0 to 1.49.0.
    - [Release notes](https://github.com/grpc/grpc-go/releases)
    - [Commits](grpc/grpc-go@v1.48.0...v1.49.0)

    ---
    updated-dependencies:
    - dependency-name: google.golang.org/grpc
      dependency-type: direct:production
      update-type: version-update:semver-minor
    ...

    Signed-off-by: dependabot[bot] <support@github.com>

    Signed-off-by: dependabot[bot] <support@github.com>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

commit 0a5647e
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Wed Aug 24 19:23:13 2022 +0200

    build(deps): Bump google.golang.org/grpc from 1.48.0 to 1.49.0 in /api (cosmos#13015)

    Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.48.0 to 1.49.0.
    - [Release notes](https://github.com/grpc/grpc-go/releases)
    - [Commits](grpc/grpc-go@v1.48.0...v1.49.0)

    ---
    updated-dependencies:
    - dependency-name: google.golang.org/grpc
      dependency-type: direct:production
      update-type: version-update:semver-minor
    ...

    Signed-off-by: dependabot[bot] <support@github.com>

    Signed-off-by: dependabot[bot] <support@github.com>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

commit 149787b
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Wed Aug 24 18:27:37 2022 +0200

    build(deps): Bump google.golang.org/grpc from 1.48.0 to 1.49.0 (cosmos#13014)

    Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.48.0 to 1.49.0.
    - [Release notes](https://github.com/grpc/grpc-go/releases)
    - [Commits](grpc/grpc-go@v1.48.0...v1.49.0)

    ---
    updated-dependencies:
    - dependency-name: google.golang.org/grpc
      dependency-type: direct:production
      update-type: version-update:semver-minor
    ...

    Signed-off-by: dependabot[bot] <support@github.com>

    Signed-off-by: dependabot[bot] <support@github.com>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Wryhder pushed a commit to Wryhder/cosmos-sdk that referenced this pull request Oct 26, 2022
## Description

ref: cosmos#12752 

- [x] Move some keeper tests into `tests/integration/gov/keeper`, see cosmos#12988 (comment)
- [x] Don't use simapp in `x/gov/keeper` tests, use mocks instead
- [x] Find 0 "simapp" occurence in x/gov/keeper. (Note: other occurrences of simapp are removed in  cosmos#13043)



---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/main/docs/building-modules)
- [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:automerge Automatically merge PR once all prerequisites pass. C:x/gov Type: Build
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants