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

amaury1093 · 2022-08-22T11:28:50Z

Description

Move some keeper tests into tests/integration/gov/keeper, see test(gov): Refactor x/gov keeper tests to use mocks #12988 (comment)
Don't use simapp in x/gov/keeper tests, use mocks instead
Find 0 "simapp" occurence in x/gov/keeper. (Note: other occurrences of simapp are removed in test(gov): Remove simapp references from module tests and simulations #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...

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 · 2022-08-24T14:03:59Z

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

@@            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

…-mocks

amaury1093 · 2022-08-25T13:17:25Z

tests/integration/gov/keeper/keeper_test.go

+// 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`.


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

amaury1093 · 2022-08-25T13:17:50Z

testutil/sims/address_helpers.go

+// 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
+}


Any objections to this? Or better ideas?

amaury1093 · 2022-08-25T13:20:04Z

x/gov/keeper/msg_server_test.go

 )

 func (suite *KeeperTestSuite) TestSubmitProposalReq() {
-	govAcct := suite.app.GovKeeper.GetGovernanceAccount(suite.ctx).GetAddress()
+	suite.reset()


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.

julienrbrt

lgtm!

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>

## 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)

amaury1093 added 3 commits August 22, 2022 12:07

Move to tests/integration

74a8a6b

Run mockgen

07dfe45

refactor more tests

1351c60

github-actions bot added C:x/gov Type: Build labels Aug 22, 2022

amaury1093 mentioned this pull request Aug 22, 2022

test: BDD tests with x/gov and mocks #12847

Closed

26 tasks

amaury1093 added 15 commits August 22, 2022 13:31

move to keeper subfolder

16cb13d

more test fixing

08ff2b5

more initializations

e3f8d40

Add some expects

7a1d1e2

Remove non-test files

ba14f42

fix hooks tests

a4e6ddf

Remove hooks test from integration

e5a9d36

Fix deposit test

44411df

delete tally unit test

493fe52

add balance tracking

db19ba4

fix test

54fd0a1

Make tests/integration pass

32b7e8c

Add resetting suites

4433e6e

Make all tests pass?

271bfd7

Add module tests to test/integration

873983f

amaury1093 added 7 commits August 24, 2022 16:06

Remove simapp from last tests

d2339eb

Remove module tests

7942dca

Move other tests around

7928c80

Fix integration keeper tests

ff2d0f4

revert module tests

000d985

Merge branch 'main' of ssh://github.com/cosmos/cosmos-sdk into am/gov…

ea4aeb9

…-mocks

remove non-keeper integration tests

0ae0cdc

amaury1093 mentioned this pull request Aug 25, 2022

Refactor x/gov to use mocks #12752

Closed

2 tasks

amaury1093 marked this pull request as ready for review August 25, 2022 13:44

amaury1093 requested a review from a team as a code owner August 25, 2022 13:44

amaury1093 commented Aug 25, 2022

View reviewed changes

alexanderbez approved these changes Aug 25, 2022

View reviewed changes

julienrbrt approved these changes Aug 25, 2022

View reviewed changes

Merge branch 'main' into am/gov-mocks

4aa3b92

julienrbrt added the A:automerge Automatically merge PR once all prerequisites pass. label Aug 25, 2022

mergify bot merged commit fc7ee0b into main Aug 25, 2022

mergify bot deleted the am/gov-mocks branch August 25, 2022 20:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

amaury1093 commented Aug 22, 2022 •

edited

Loading

codecov bot commented Aug 24, 2022 •

edited

Loading

amaury1093 Aug 25, 2022

amaury1093 Aug 25, 2022

amaury1093 Aug 25, 2022

julienrbrt left a comment

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

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

Conversation

amaury1093 commented Aug 22, 2022 • edited Loading

Description

Author Checklist

Reviewers Checklist

codecov bot commented Aug 24, 2022 • edited Loading

Codecov Report

amaury1093 Aug 25, 2022

Choose a reason for hiding this comment

amaury1093 Aug 25, 2022

Choose a reason for hiding this comment

amaury1093 Aug 25, 2022

Choose a reason for hiding this comment

julienrbrt left a comment

Choose a reason for hiding this comment

amaury1093 commented Aug 22, 2022 •

edited

Loading

codecov bot commented Aug 24, 2022 •

edited

Loading