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

docs: add module documentation testing #12762

Merged
merged 12 commits into from
Sep 6, 2022
Merged

docs: add module documentation testing #12762

merged 12 commits into from
Sep 6, 2022

Conversation

julienrbrt
Copy link
Member

Description

Explains how the Cosmos SDK is tested and how a module should be tested.
This follow the testing refactoring happening in the SDK.

Ref: #12696, #12398, #11899

cc @marbar3778 @alexanderbez @kocubinski @AmauryM @facundomedica


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)

@julienrbrt
Copy link
Member Author

Waiting for the testing ADR.

@julienrbrt julienrbrt self-assigned this Sep 5, 2022
@github-actions github-actions bot added C:Simulations C:x/distribution distribution module related labels Sep 6, 2022
@julienrbrt
Copy link
Member Author

I wanted to link the distribution simulation as an example in the docs, and I've noticed it was moved to the wrong place (https://github.com/cosmos/cosmos-sdk/pull/12889/files#diff-f096725763ac01d0861d6f6d43cec03997c13b0397e1916a8284ed0d6f4e55be). I've moved them back where they should be.

@codecov
Copy link

codecov bot commented Sep 6, 2022

Codecov Report

Merging #12762 (0ba0f4d) into main (9e0e951) will increase coverage by 0.32%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #12762      +/-   ##
==========================================
+ Coverage   53.73%   54.05%   +0.32%     
==========================================
  Files         645      645              
  Lines       55238    55238              
==========================================
+ Hits        29682    29860     +178     
+ Misses      23178    22999     -179     
- Partials     2378     2379       +1     
Impacted Files Coverage Δ
x/simulation/util.go 0.00% <0.00%> (-38.47%) ⬇️
x/simulation/operation.go 0.00% <0.00%> (-13.05%) ⬇️
x/group/keeper/keeper.go 56.25% <0.00%> (-0.40%) ⬇️
crypto/keys/internal/ecdsa/privkey.go 83.01% <0.00%> (+1.88%) ⬆️
x/distribution/simulation/operations.go 90.32% <0.00%> (+90.32%) ⬆️
x/distribution/simulation/decoder.go 100.00% <0.00%> (+100.00%) ⬆️

@julienrbrt julienrbrt marked this pull request as ready for review September 6, 2022 07:59
@julienrbrt julienrbrt requested a review from a team as a code owner September 6, 2022 07:59
@julienrbrt julienrbrt enabled auto-merge (squash) September 6, 2022 07:59
Copy link
Member

@tac0turtle tac0turtle left a comment

Choose a reason for hiding this comment

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

left a few comments, I think this needs to be expanded quite a bit with short walkthroughs.

docs/building-modules/testing.md Show resolved Hide resolved
docs/building-modules/testing.md Show resolved Hide resolved
docs/building-modules/testing.md Show resolved Hide resolved
docs/building-modules/testing.md Show resolved Hide resolved
@julienrbrt julienrbrt disabled auto-merge September 6, 2022 08:11
order: 1
-->

# Dependency Injection
Copy link
Member Author

Choose a reason for hiding this comment

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

This will be added after #12633. I am for keeping this boilerplate, as it permits me to link the testing documentation to here.

docs/building-chain/README.md Outdated Show resolved Hide resolved
docs/building-chain/README.md Show resolved Hide resolved
docs/building-chain/app-go.md Outdated Show resolved Hide resolved

+++ https://github.com/cosmos/cosmos-sdk/blob/main/simapp/app_config.go

+++ https://github.com/cosmos/cosmos-sdk/blob/main/simapp/app.go
Copy link
Member

Choose a reason for hiding this comment

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

We should open an issue to fill this out. Generally not a fan of just linking code as it doesn't explain why this and why that. This wouldn't provide much info to a new user.

Copy link
Member Author

@julienrbrt julienrbrt Sep 6, 2022

Choose a reason for hiding this comment

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

As docs.cosmos.network default on v0.46, a new user won't see it until he digs.
I can remove the code, but I am really for to keep these files, as this allows me to already link to them.

Copy link
Member

Choose a reason for hiding this comment

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

Totally fine it keep them, mainly thinking it would be useful to also break it down

@julienrbrt julienrbrt merged commit f9e8a04 into main Sep 6, 2022
@julienrbrt julienrbrt deleted the julien/testing branch September 6, 2022 23:34
@julienrbrt
Copy link
Member Author

Live https://docs.cosmos.network/main/building-modules/testing.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants