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

fix(tests): Fix post-upgrade transactions in automated upgrade tests #2311

Merged
merged 12 commits into from
Feb 2, 2024

Conversation

MalteHerrmann
Copy link
Contributor

@MalteHerrmann MalteHerrmann commented Feb 1, 2024

Description

This PR fixes the post-upgrade transaction that is being sent in the automated upgrade tests.
There are a bunch of improvements and refactors bundled with this main change:

  • Create QueryArgs type and refactor query calls in queryexec.go
  • Create tx.go which holds all methods related to the post-upgrade transactions
  • Create balances.go with utilities to parse the balances query response

Closes ENG-2492

Summary by CodeRabbit

  • Tests
    • Enhanced the readability and maintainability of integration test code by adopting structured argument passing.
    • Added functionality for executing and verifying sample transactions in end-to-end tests.
    • Introduced methods and tests for retrieving and unpacking account balances from CLI queries.
    • Adjusted transaction execution behavior in tests by specifying output format, adjusting fees, and enabling automatic confirmation.
    • Updated the test suite to utilize a new approach for module query executions, improving test clarity and structure.

@MalteHerrmann MalteHerrmann requested a review from a team as a code owner February 1, 2024 07:40
@MalteHerrmann MalteHerrmann requested review from Vvaradinov and facs95 and removed request for a team February 1, 2024 07:40
Copy link

linear bot commented Feb 1, 2024

Copy link
Contributor

coderabbitai bot commented Feb 1, 2024

Warning

Rate Limit Exceeded

@GAtom22 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 37 seconds before requesting another review.

How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.
Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.
Please see our FAQ for further information.

Commits Files that changed from the base of the PR and between d88629d and 6f3ae65.

Walkthrough

The recent updates to the end-to-end (e2e) test suite primarily focus on enhancing code readability and maintainability through structured data usage and command execution improvements. These changes include adopting a new struct for query arguments, adding functionality for executing and testing transactions, refining the process of retrieving and testing account balances, adjusting command flags, and introducing ProtoCodec for serialization. These updates aim to optimize test operations and data handling within the suite.

Changes

Files Change Summary
tests/e2e/e2e_suite_test.go, tests/e2e/upgrade/exec.go, tests/e2e/upgrade/queryexec.go Adopted QueryArgs struct for module query executions, improving readability and maintainability.
tests/e2e/tx.go, tests/e2e/tx_test.go Added functionality for executing and verifying transactions pre and post-upgrade.
tests/e2e/upgrade/balances.go, tests/e2e/upgrade/balances_test.go Introduced methods for retrieving and unpacking account balances.
tests/e2e/upgrade/manager.go Enhanced Manager with ProtoCodec initialization and optimized query execution.

"In the realm of code, where logic intertwines,
A rabbit hopped through, refining the lines.
Structs aligned, with transactions they danced,
A leap towards clarity, in tests, it advanced. 🐇✨"

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>.
    • Generate unit-tests for this file.
  • 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 tests 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 generate interesting stats about this repository from git and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit tests.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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 as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

@github-actions github-actions bot added the tests label Feb 1, 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.

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 755945a and df072d5.
Files selected for processing (7)
  • tests/e2e/e2e_suite_test.go (3 hunks)
  • tests/e2e/tx.go (1 hunks)
  • tests/e2e/upgrade/balances.go (1 hunks)
  • tests/e2e/upgrade/balances_test.go (1 hunks)
  • tests/e2e/upgrade/exec.go (1 hunks)
  • tests/e2e/upgrade/manager.go (4 hunks)
  • tests/e2e/upgrade/queryexec.go (1 hunks)
Files skipped from review due to trivial changes (1)
  • tests/e2e/e2e_suite_test.go
Additional comments: 9
tests/e2e/upgrade/exec.go (1)
  • 31-35: The addition of --output=json, --fees=500000000000aevmos, and --yes flags to the CreateModuleTxExec method enhances the command's functionality by specifying the output format, adjusting the fees, and enabling automatic confirmation. These changes align with best practices for clarity and explicit command behavior in scripts.
tests/e2e/upgrade/balances.go (2)
  • 12-36: The GetBalance function correctly implements the process of constructing a query command using the QueryArgs struct and executing it to retrieve balance information. The error handling is thorough, ensuring that any execution or parsing errors are caught and reported. The use of QueryArgs for encapsulating query parameters is a good practice for maintainability and readability.
  • 38-45: The UnpackBalancesResponse function demonstrates good practice in error handling and code clarity by checking for errors during the unmarshalling process and returning a clear error message if needed. This function is a good example of separating concerns by handling the specific task of parsing the balance query response.
tests/e2e/upgrade/queryexec.go (2)
  • 10-34: The introduction of the QueryArgs struct and its Validate method is a positive change, enhancing the robustness of the query execution process by ensuring that all necessary fields are populated before executing a query. This approach improves code maintainability and readability.
  • 36-59: Refactoring CreateModuleQueryExec to accept a QueryArgs struct simplifies the function's signature and makes the code more readable. The validation step before building the command is a good practice, ensuring that the function operates on a valid set of arguments.
tests/e2e/upgrade/balances_test.go (1)
  • 15-69: The test cases for UnpackBalancesResponse are well-structured and cover various scenarios, including success, empty balances, and invalid output. The use of table-driven tests is a best practice in Go, facilitating the addition of new test cases and ensuring that the function behaves correctly under different conditions.
tests/e2e/tx.go (1)
  • 15-64: The executeTransactions and sendBankTransfer methods demonstrate a clear and logical flow for executing transactions and verifying their effects. The use of context for cancellation, error handling, and the regex check for the gas estimate message are all best practices. However, ensure that the hardcoded receiver address and the mykey string are appropriate for all testing scenarios or consider making them configurable.
tests/e2e/upgrade/manager.go (2)
  • 63-79: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [20-76]

The addition of the ProtoCodec field to the Manager struct and its initialization in the NewManager function is a significant improvement, enabling the use of protobuf for encoding and decoding. This change aligns with modern best practices for handling data serialization in distributed systems.

  • 341-345: The modification of the getVotingPeriod method to use the CreateModuleQueryExec with a QueryArgs struct instead of separate string arguments is a good practice. It simplifies the method signature and leverages the QueryArgs struct's validation, enhancing code readability and maintainability.

Copy link

codecov bot commented Feb 1, 2024

Codecov Report

Attention: 63 lines in your changes are missing coverage. Please review.

Comparison is base (f097735) 70.45% compared to head (d7fc541) 70.29%.
Report is 5 commits behind head on main.

❗ Current head d7fc541 differs from pull request most recent head 6f3ae65. Consider uploading reports for the commit 6f3ae65 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2311      +/-   ##
==========================================
- Coverage   70.45%   70.29%   -0.16%     
==========================================
  Files         293      294       +1     
  Lines       22559    22617      +58     
==========================================
+ Hits        15893    15899       +6     
- Misses       5800     5852      +52     
  Partials      866      866              
Files Coverage Δ
tests/e2e/upgrade/exec.go 0.00% <0.00%> (ø)
tests/e2e/upgrade/manager.go 0.00% <0.00%> (ø)
tests/e2e/upgrade/balances.go 23.07% <23.07%> (ø)
tests/e2e/upgrade/queryexec.go 0.00% <0.00%> (ø)

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.

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between df072d5 and d7fc541.
Files selected for processing (3)
  • CHANGELOG.md (1 hunks)
  • tests/e2e/upgrade/balances.go (1 hunks)
  • tests/e2e/upgrade/manager.go (4 hunks)
Files skipped from review as they are similar to previous changes (2)
  • tests/e2e/upgrade/balances.go
  • tests/e2e/upgrade/manager.go

Copy link
Contributor

@GAtom22 GAtom22 left a comment

Choose a reason for hiding this comment

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

LGTM!! Great work @MalteHerrmann!!

Copy link
Contributor

@ramacarlucho ramacarlucho left a comment

Choose a reason for hiding this comment

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

LGTM. Need to fix the github actions

tests/e2e/tx.go Outdated Show resolved Hide resolved
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.

Review Status

Actionable comments generated: 2

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between d7fc541 and d88629d.
Files selected for processing (1)
  • tests/e2e/tx_test.go (1 hunks)

tests/e2e/tx_test.go Show resolved Hide resolved
tests/e2e/tx_test.go Show resolved Hide resolved
@MalteHerrmann MalteHerrmann enabled auto-merge (squash) February 2, 2024 09:16
@MalteHerrmann MalteHerrmann merged commit eec726e into main Feb 2, 2024
49 checks passed
@MalteHerrmann MalteHerrmann deleted the malte/fix-e2e-tests branch February 2, 2024 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants