-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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: system test tutorial #20812
docs: system test tutorial #20812
Conversation
WalkthroughWalkthroughThe changes involve updates to the documentation files related to system tests. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TestRunner
participant SystemUnderTest
participant JSONLibrary
User->>TestRunner: Write test case
TestRunner->>SystemUnderTest: Initialize test environment
TestRunner->>SystemUnderTest: Execute query
SystemUnderTest-->>TestRunner: Return response
TestRunner->>JSONLibrary: Validate JSON response
JSONLibrary-->>TestRunner: Return validation results
TestRunner->>User: Provide test results
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
should we merge the readme, the test is fine to remove |
I think we should add the whole readme https://github.com/cosmos/cosmos-sdk/blob/main/docs/build/building-modules/16-testing.md to mention system tests and remove e2e tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (8)
tests/systemtests/getting_started.md (8)
25-25
: Add a comma after "for example" for better readability.-for example `bank_test.go` to begin with: +for example, `bank_test.go` to begin with:Tools
LanguageTool
[typographical] ~25-~25: After the expression ‘for example’ a comma is usually used.
Context: ...e case, start a new test file here. for examplebank_test.go
to begin with: ```go //...(COMMA_FOR_EXAMPLE)
52-52
: Add a comma to improve clarity.-In this example we want to execute `simd q bank total-supply --output json --node tcp://localhost:26657` which queries +In this example, we want to execute `simd q bank total-supply --output json --node tcp://localhost:26657` which queriesTools
LanguageTool
[typographical] ~52-~52: It appears that a comma is missing.
Context: ...r to interact or parse results. In this example we want to execute `simd q bank total-s...(DURING_THAT_TIME_COMMA)
61-61
: Fix the subject-verb agreement.-This give very verbose output. +This gives very verbose output.Tools
LanguageTool
[grammar] ~61-~61: Possible subject-verb agreement error detected.
Context: ...un TestQueryTotalSupply --verbose ``` This give very verbose output. You would see all ...(THIS_THAT_AGR)
95-95
: Add a comma after "For example" for better readability.-For example `gjson.Get(raw, "supply").Array()` gives us all the childs to `supply` as an array. +For example, `gjson.Get(raw, "supply").Array()` gives us all the childs to `supply` as an array.Tools
LanguageTool
[typographical] ~95-~95: After the expression ‘for example’ a comma is usually used.
Context: ...to navigation within the document. For examplegjson.Get(raw, "supply").Array()
give...(COMMA_FOR_EXAMPLE)
97-97
: Consider replacing "In order to" with "To" for conciseness.-In order to test our assumptions in the system test, we modify the code to use `gjson` to fetch the data: +To test our assumptions in the system test, we modify the code to use `gjson` to fetch the data:Tools
LanguageTool
[style] ~97-~97: Consider a shorter alternative to avoid wordiness.
Context: ...ount of the stake token as int64 type. In order to test our assumptions in the system test...(IN_ORDER_TO_PREMIUM)
133-133
: Add a comma after "Usually" for clarity.-Usually a good time to think about extracting helper functions for +Usually, a good time to think about extracting helper functions forTools
LanguageTool
[style] ~133-~133: As an alternative to the over-used intensifier ‘quite’, consider replacing this phrase.
Context: ...t for keynode0
. The setup code looks quite big and unreadable now. Usually a good time...(EN_WEAK_ADJECTIVE)
[typographical] ~133-~133: Consider adding a comma after ‘Usually’ for more clarity.
Context: ...ode looks quite big and unreadable now. Usually a good time to think about extracting h...(RB_LY_COMMA)
27-44
: Add more comments to explain the key steps in the code snippets.Consider adding comments to explain:
- The purpose of each code block
- The expected output or behavior
- Any important considerations or gotchas
This will make the code snippets more beginner-friendly and easier to understand.
Also applies to: 98-113, 137-166, 188-205, 210-215
1-215
: Add a conclusion section to summarize the key takeaways.Consider adding a conclusion section at the end of the guide to:
- Recap the main points covered
- Provide next steps for readers to continue learning about system tests
- Link to additional resources or documentation
This will help reinforce the learnings and provide a clear direction for readers.
Tools
LanguageTool
[typographical] ~25-~25: After the expression ‘for example’ a comma is usually used.
Context: ...e case, start a new test file here. for examplebank_test.go
to begin with: ```go //...(COMMA_FOR_EXAMPLE)
[typographical] ~52-~52: It appears that a comma is missing.
Context: ...r to interact or parse results. In this example we want to execute `simd q bank total-s...(DURING_THAT_TIME_COMMA)
[grammar] ~61-~61: Possible subject-verb agreement error detected.
Context: ...un TestQueryTotalSupply --verbose ``` This give very verbose output. You would see all ...(THIS_THAT_AGR)
[typographical] ~95-~95: After the expression ‘for example’ a comma is usually used.
Context: ...to navigation within the document. For examplegjson.Get(raw, "supply").Array()
give...(COMMA_FOR_EXAMPLE)
[style] ~97-~97: Consider a shorter alternative to avoid wordiness.
Context: ...ount of the stake token as int64 type. In order to test our assumptions in the system test...(IN_ORDER_TO_PREMIUM)
[style] ~133-~133: As an alternative to the over-used intensifier ‘quite’, consider replacing this phrase.
Context: ...t for keynode0
. The setup code looks quite big and unreadable now. Usually a good time...(EN_WEAK_ADJECTIVE)
[typographical] ~133-~133: Consider adding a comma after ‘Usually’ for more clarity.
Context: ...ode looks quite big and unreadable now. Usually a good time to think about extracting h...(RB_LY_COMMA)
[uncategorized] ~167-~167: Possible missing article found.
Context: ...newState }) sut.StartChain(t) ``` Next step is to add the new token to the ass...(AI_HYDRA_LEO_MISSING_THE)
[uncategorized] ~185-~185: Possible missing preposition found.
Context: ...and operations. If we want to burn some our new tokens, we need to submit a bank bu...(AI_HYDRA_LEO_MISSING_OF)
[grammar] ~207-~207: The modal verb ‘can’ requires the verb’s base form.
Context: ...are still more or less readable, it can gets harder the longer they are. I found it ...(MD_BASEFORM)
[typographical] ~207-~207: Consider inserting a comma for improved readability.
Context: ... readable, it can gets harder the longer they are. I found it helpful to add some com...(MISSING_COMMAS)
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (2)
- tests/systemtests/README.md (2 hunks)
- tests/systemtests/getting_started.md (1 hunks)
Files skipped from review due to trivial changes (1)
- tests/systemtests/README.md
Additional context used
Path-based instructions (1)
tests/systemtests/getting_started.md (2)
Pattern
tests/**/*
: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"
Pattern
**/*.md
: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
LanguageTool
tests/systemtests/getting_started.md
[typographical] ~25-~25: After the expression ‘for example’ a comma is usually used.
Context: ...e case, start a new test file here. for examplebank_test.go
to begin with: ```go //...(COMMA_FOR_EXAMPLE)
[typographical] ~52-~52: It appears that a comma is missing.
Context: ...r to interact or parse results. In this example we want to execute `simd q bank total-s...(DURING_THAT_TIME_COMMA)
[grammar] ~61-~61: Possible subject-verb agreement error detected.
Context: ...un TestQueryTotalSupply --verbose ``` This give very verbose output. You would see all ...(THIS_THAT_AGR)
[typographical] ~95-~95: After the expression ‘for example’ a comma is usually used.
Context: ...to navigation within the document. For examplegjson.Get(raw, "supply").Array()
give...(COMMA_FOR_EXAMPLE)
[style] ~97-~97: Consider a shorter alternative to avoid wordiness.
Context: ...ount of the stake token as int64 type. In order to test our assumptions in the system test...(IN_ORDER_TO_PREMIUM)
[style] ~133-~133: As an alternative to the over-used intensifier ‘quite’, consider replacing this phrase.
Context: ...t for keynode0
. The setup code looks quite big and unreadable now. Usually a good time...(EN_WEAK_ADJECTIVE)
[typographical] ~133-~133: Consider adding a comma after ‘Usually’ for more clarity.
Context: ...ode looks quite big and unreadable now. Usually a good time to think about extracting h...(RB_LY_COMMA)
[uncategorized] ~167-~167: Possible missing article found.
Context: ...newState }) sut.StartChain(t) ``` Next step is to add the new token to the ass...(AI_HYDRA_LEO_MISSING_THE)
[uncategorized] ~185-~185: Possible missing preposition found.
Context: ...and operations. If we want to burn some our new tokens, we need to submit a bank bu...(AI_HYDRA_LEO_MISSING_OF)
[grammar] ~207-~207: The modal verb ‘can’ requires the verb’s base form.
Context: ...are still more or less readable, it can gets harder the longer they are. I found it ...(MD_BASEFORM)
[typographical] ~207-~207: Consider inserting a comma for improved readability.
Context: ... readable, it can gets harder the longer they are. I found it helpful to add some com...(MISSING_COMMAS)
Additional comments not posted (1)
tests/systemtests/getting_started.md (1)
1-215
: Excellent guide for writing system tests! 👍This is a comprehensive guide that covers all the key aspects of writing system tests, including:
- Preparation steps
- Writing the first test
- Working with JSON responses
- Setting state via genesis and transactions
The code snippets are clear and easy to follow. Great job!
Tools
LanguageTool
[typographical] ~25-~25: After the expression ‘for example’ a comma is usually used.
Context: ...e case, start a new test file here. for examplebank_test.go
to begin with: ```go //...(COMMA_FOR_EXAMPLE)
[typographical] ~52-~52: It appears that a comma is missing.
Context: ...r to interact or parse results. In this example we want to execute `simd q bank total-s...(DURING_THAT_TIME_COMMA)
[grammar] ~61-~61: Possible subject-verb agreement error detected.
Context: ...un TestQueryTotalSupply --verbose ``` This give very verbose output. You would see all ...(THIS_THAT_AGR)
[typographical] ~95-~95: After the expression ‘for example’ a comma is usually used.
Context: ...to navigation within the document. For examplegjson.Get(raw, "supply").Array()
give...(COMMA_FOR_EXAMPLE)
[style] ~97-~97: Consider a shorter alternative to avoid wordiness.
Context: ...ount of the stake token as int64 type. In order to test our assumptions in the system test...(IN_ORDER_TO_PREMIUM)
[style] ~133-~133: As an alternative to the over-used intensifier ‘quite’, consider replacing this phrase.
Context: ...t for keynode0
. The setup code looks quite big and unreadable now. Usually a good time...(EN_WEAK_ADJECTIVE)
[typographical] ~133-~133: Consider adding a comma after ‘Usually’ for more clarity.
Context: ...ode looks quite big and unreadable now. Usually a good time to think about extracting h...(RB_LY_COMMA)
[uncategorized] ~167-~167: Possible missing article found.
Context: ...newState }) sut.StartChain(t) ``` Next step is to add the new token to the ass...(AI_HYDRA_LEO_MISSING_THE)
[uncategorized] ~185-~185: Possible missing preposition found.
Context: ...and operations. If we want to burn some our new tokens, we need to submit a bank bu...(AI_HYDRA_LEO_MISSING_OF)
[grammar] ~207-~207: The modal verb ‘can’ requires the verb’s base form.
Context: ...are still more or less readable, it can gets harder the longer they are. I found it ...(MD_BASEFORM)
[typographical] ~207-~207: Consider inserting a comma for improved readability.
Context: ... readable, it can gets harder the longer they are. I found it helpful to add some com...(MISSING_COMMAS)
Co-authored-by: Julien Robert <[email protected]> (cherry picked from commit 6b20ef7)
Description
Follow the commits to the see a system test build. Or just read the doc.
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...
!
in the type prefix if API or client breaking changeCHANGELOG.md
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.
Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.
I have...
Summary by CodeRabbit
New Features
Documentation