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 initial doc for system tests #22002

Merged
merged 5 commits into from
Oct 2, 2024
Merged

Conversation

alpe
Copy link
Contributor

@alpe alpe commented Oct 1, 2024

Description

See: #21429

Start doc about system tests


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, you can find examples of the prefixes below:
  • confirmed ! in 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
  • reviewed "Files changed" and left comments if necessary
  • included the necessary unit and integration tests
  • added a changelog entry to CHANGELOG.md
  • updated the relevant documentation or specification, including comments for documenting Go code
  • 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.

Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic, API design and naming, documentation is accurate, tests and test coverage

Summary by CodeRabbit

  • New Features

    • Introduced a new document on system tests for blockchain applications, detailing execution frameworks and practical examples.
  • Documentation

    • Updated the "Simulations" section to clarify its focus on fuzz tests for deterministic message execution.
    • Renamed "End-to-end Tests" to "System Tests" to reflect a broader application evaluation perspective.
    • Made minor adjustments for clarity and consistency throughout the testing documentation.

Copy link
Contributor

coderabbitai bot commented Oct 1, 2024

📝 Walkthrough

Walkthrough

The changes include the introduction of a new document titled 06-system-tests.md, which provides an overview of system tests for blockchain applications. It explains the purpose, framework, and execution of black box tests, highlighting the importance of testing business-critical scenarios. Additionally, the document 16-testing.md has been updated to clarify the concept of simulations, which now refer to fuzz tests, and has renamed the section on end-to-end tests to system tests, reflecting a broader application evaluation scope.

Changes

File Path Change Summary
docs/build/building-apps/06-system-tests.md New document added to provide an overview of system tests, including examples and guidelines.
docs/build/building-modules/16-testing.md Section renamed from "End-to-end Tests" to "System Tests"; updated content regarding simulations and examples.

Possibly related PRs

  • test: e2e/client to system tests #21981: The introduction of system tests for the gRPC client functionalities aligns with the main PR's focus on system tests, as both emphasize the importance of testing within the blockchain application context.
  • docs: minor cleanup to simulation docs  #21777: Although primarily a cleanup of simulation documentation, it relates to the broader context of testing and simulations within the Cosmos SDK, which is relevant to the system testing framework discussed in the main PR.

Suggested labels

C:Simulations, backport/v0.52.x

Suggested reviewers

  • julienrbrt
  • tac0turtle

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@alpe alpe changed the title Add initial doc for system tests docs: Add initial doc for system tests Oct 1, 2024
@julienrbrt
Copy link
Member

Unrelated to the content, but you could make your editor follow the formatting rules: https://github.com/cosmos/cosmos-sdk/blob/main/.markdownlint.json

* main:
  docs: amend docs for 52 changes  (#21992)
  test: migrate e2e/authz to system tests (#21819)
  refactor(runtime/v2): use StoreBuilder (#21989)
  feat(schema): add API descriptors, struct, oneof & list types, and wire encoding spec (#21482)
  docs: add instructions to change DefaultGenesis (#21680)
  feat(x/staking)!: Add metadata field to validator info (#21315)
  chore(x/authz)!: Remove account keeper dependency (#21632)
@alpe alpe marked this pull request as ready for review October 1, 2024 14:17
@alpe alpe requested a review from a team as a code owner October 1, 2024 14:17
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.

This is looking good thanks! Could you delete some section about e2e in the build-modules?

@julienrbrt julienrbrt added the backport/v0.52.x PR scheduled for inclusion in the v0.52's next stable release label Oct 2, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 and nitpick comments (7)
docs/build/building-apps/06-system-tests.md (4)

11-25: LGTM with a minor suggestion: Well-explained implementation details

The section effectively explains the implementation of system tests using Go, introducing key components and providing useful examples and resources.

Consider removing the line break between "genesis or" and "transactions" on lines 24-25 to improve readability:

 contains a step-by-step guide to building and running your first system test. It covers setting chain state via genesis
-or
-transactions and validation via transaction response or queries.
+or transactions and validation via transaction response or queries.

27-42: LGTM with a formatting suggestion: Comprehensive design principles and guidelines

The section provides valuable guidance for writing effective system tests. The principles are well-explained and cover important aspects of test design.

To adhere to the project's markdown linting configuration, please update the unordered list style from dashes to asterisks:

-  - **Perspective:** Tests should mimic a human interacting with the chain from the outside. Initial states can be set via
+  * **Perspective:** Tests should mimic a human interacting with the chain from the outside. Initial states can be set via
   genesis or transactions to support a test scenario.
-  - **Roles:** The user can have multiple roles such as validator, delegator, granter, or group admin.
+  * **Roles:** The user can have multiple roles such as validator, delegator, granter, or group admin.
-  - **Focus:** Tests should concentrate on happy paths or business-critical workflows. Unit and integration tests are
+  * **Focus:** Tests should concentrate on happy paths or business-critical workflows. Unit and integration tests are
   better suited for more fine-grained testing.
-  - **Workflows:** Test workflows and scenarios, not individual units. Given the high setup costs, it is reasonable to
+  * **Workflows:** Test workflows and scenarios, not individual units. Given the high setup costs, it is reasonable to
   combine multiple steps and assertions in a single test method.
-  - **Genesis Mods:** Genesis modifications can incur additional time costs for resetting dirty states. Reuse existing
+  * **Genesis Mods:** Genesis modifications can incur additional time costs for resetting dirty states. Reuse existing
   accounts (node0..n) whenever possible.
-  - **Framework:** Continuously improve the framework for better readability and reusability.
+  * **Framework:** Continuously improve the framework for better readability and reusability.
🧰 Tools
🪛 LanguageTool

[typographical] ~40-~40: Two consecutive dots
Context: ...states. Reuse existing accounts (node0..n) whenever possible. - Framework: C...

(DOUBLE_PUNCTUATION)

🪛 Markdownlint

32-32: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


34-34: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


35-35: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


37-37: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


39-39: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


41-41: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


43-48: LGTM with a minor suggestion: Helpful debugging information

This section provides valuable information on logging and debugging system tests.

To improve readability, consider using curly braces for the range in "node0..n":

- accounts (node0..n) whenever possible.
+ accounts (node{0..n}) whenever possible.

This change makes the notation consistent with the node{0..n}.out format used earlier in the document.

🧰 Tools
🪛 LanguageTool

[typographical] ~45-~45: Two consecutive dots
Context: ... and Debugging All output is logged to systemtests/testnet/node{0..n}.out. Usually, node0.out is very no...

(DOUBLE_PUNCTUATION)


49-58: LGTM with a formatting suggestion: Clear debugging instructions

The step-by-step debugging instructions are clear and helpful for developers.

To adhere to the project's markdown linting configuration, please update the unordered list style from dashes to asterisks:

- - Start the test with one node only and verbose output:
+ * Start the test with one node only and verbose output:

   ```sh
   go test -v -tags=system_test ./ --run TestAccountCreation --verbose --nodes-count=1
    • Copy the CLI command for the transaction and modify the test to stop before the command
    • Start the node with --home=<project-home>/tests/systemtests/testnet/node0/<binary-name>/ in debug mode
    • Execute CLI command from shell and enter breakpoints
    • Copy the CLI command for the transaction and modify the test to stop before the command
    • Start the node with --home=<project-home>/tests/systemtests/testnet/node0/<binary-name>/ in debug mode
    • Execute CLI command from shell and enter breakpoints

<details>
<summary>🧰 Tools</summary>

<details>
<summary>🪛 Markdownlint</summary><blockquote>

50-50: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

56-56: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

57-57: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

58-58: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

</blockquote></details>

</details>

</blockquote></details>
<details>
<summary>docs/build/building-modules/16-testing.md (3)</summary><blockquote>

`89-100`: **LGTM! Consider adding a brief explanation of message factories.**

The updates to the Simulations section provide a clearer and more accurate description of the current state of simulations in the SDK. The change from `x/gov/` to `x/bank/` for the example is also appropriate.



Consider adding a brief explanation of what message factories are and how they relate to simulations. This would help readers better understand the concept, especially those who might be new to the SDK.

---

`101-107`: **LGTM! Consider clarifying the use of `simapp` vs. custom binaries.**

The updates to the System Tests section (formerly End-to-end Tests) provide a more accurate and comprehensive description of these tests. The change in terminology and test location is consistent with the current SDK structure.



The sentence about using the `simapp` binary might be confusing. Consider clarifying that while the SDK uses `simapp` for its own testing, users should use their own application binary when testing their specific blockchain application. This would help prevent any misunderstanding about which binary to use in different contexts.

---

Line range hint `1-116`: **LGTM! Consider adding a brief introduction to the changes.**

The document maintains a consistent structure and now provides a more comprehensive overview of testing methodologies in the Cosmos SDK. The updates to the Simulations and System Tests sections integrate well with the existing content.



Consider adding a brief introduction at the beginning of the document highlighting the recent changes to the testing methodologies. This would help readers quickly identify what's new or updated in the document.

</blockquote></details>

</blockquote></details>

<details>
<summary>📜 Review details</summary>

**Configuration used: .coderabbit.yml**
**Review profile: CHILL**

<details>
<summary>📥 Commits</summary>

Files that changed from the base of the PR and between 52d8b2eb43df267d80a6bbbd2b7067879466377d and 1026a68c2b18e59689dfbccd5cc9fc9b390d8c9d.

</details>

<details>
<summary>📒 Files selected for processing (2)</summary>

* docs/build/building-apps/06-system-tests.md (1 hunks)
* docs/build/building-modules/16-testing.md (1 hunks)

</details>

<details>
<summary>🧰 Additional context used</summary>

<details>
<summary>📓 Path-based instructions (2)</summary><blockquote>

<details>
<summary>docs/build/building-apps/06-system-tests.md (1)</summary>

Pattern `**/*.md`: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"


</details>
<details>
<summary>docs/build/building-modules/16-testing.md (1)</summary>

Pattern `**/*.md`: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"


</details>

</blockquote></details>

<details>
<summary>🪛 LanguageTool</summary><blockquote>

<details>
<summary>docs/build/building-apps/06-system-tests.md</summary><blockquote>

[typographical] ~40-~40: Two consecutive dots
Context: ...states. Reuse existing   accounts (node0..n) whenever possible. - **Framework:** C...

(DOUBLE_PUNCTUATION)

---

[typographical] ~45-~45: Two consecutive dots
Context: ... and Debugging  All output is logged to `systemtests/testnet/node{0..n}.out`. Usually, `node0.out` is very no...

(DOUBLE_PUNCTUATION)

</blockquote></details>

</blockquote></details>
<details>
<summary>🪛 Markdownlint</summary><blockquote>

<details>
<summary>docs/build/building-apps/06-system-tests.md</summary><blockquote>

32-32: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

34-34: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

35-35: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

37-37: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

39-39: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

41-41: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

50-50: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

56-56: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

57-57: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

58-58: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

</blockquote></details>

</blockquote></details>

</details>

<details>
<summary>🔇 Additional comments (3)</summary><blockquote>

<details>
<summary>docs/build/building-apps/06-system-tests.md (3)</summary><blockquote>

`1-6`: **LGTM: Frontmatter and title are well-formatted**

The frontmatter is correctly structured, and the title "System Tests" is clear and concise.

---

`7-10`: **LGTM: Clear and informative introduction**

The introduction effectively explains the purpose and benefits of system tests, providing a good context for readers.

---

`1-58`: **Excellent introduction to system tests**

This document provides a comprehensive and well-structured introduction to system tests for blockchain applications. It covers key aspects such as the purpose of system tests, implementation details, design principles, and debugging techniques.

The content is informative and valuable for developers working on blockchain applications. With the minor formatting suggestions implemented, this document will serve as an excellent resource for the project.

<details>
<summary>🧰 Tools</summary>

<details>
<summary>🪛 LanguageTool</summary><blockquote>

[typographical] ~40-~40: Two consecutive dots
Context: ...states. Reuse existing   accounts (node0..n) whenever possible. - **Framework:** C...

(DOUBLE_PUNCTUATION)

---

[typographical] ~45-~45: Two consecutive dots
Context: ... and Debugging  All output is logged to `systemtests/testnet/node{0..n}.out`. Usually, `node0.out` is very no...

(DOUBLE_PUNCTUATION)

</blockquote></details>
<details>
<summary>🪛 Markdownlint</summary><blockquote>

32-32: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

34-34: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

35-35: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

37-37: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

39-39: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

41-41: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

50-50: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

56-56: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

57-57: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

58-58: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

</blockquote></details>

</details>

</blockquote></details>

</blockquote></details>

</details>

<!-- This is an auto-generated comment by CodeRabbit for review status -->

@alpe
Copy link
Contributor Author

alpe commented Oct 2, 2024

I assume the dead link in https://github.com/cosmos/cosmos-sdk/actions/runs/11143215342/job/30967800126?pr=22002#step:5:472 is fixed when this is published

@alpe alpe added this pull request to the merge queue Oct 2, 2024
Merged via the queue into main with commit 5c4f4ac Oct 2, 2024
72 of 73 checks passed
@alpe alpe deleted the alex/21429_build_systemtest branch October 2, 2024 12:04
mergify bot pushed a commit that referenced this pull request Oct 2, 2024
julienrbrt pushed a commit that referenced this pull request Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/v0.52.x PR scheduled for inclusion in the v0.52's next stable release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants