chore: fix system test for v2 #23328
Conversation
📝 WalkthroughWalkthroughThe pull request introduces changes to the testing workflow and configuration for system tests. The modifications primarily focus on simplifying the build and test process for the Changes
Possibly related PRs
Suggested Labels
Suggested Reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (9)
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
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/systemtests/tx_test.go (1)
411-413: Consider adding more specific assertions.The test only verifies the length of transactions. Consider adding assertions for transaction ordering when testing ORDER_BY cases.
require.Equal(t, tc.expLen, len(txs)) + if tc.expLen > 1 { + // Verify ordering based on height or time + for i := 1; i < len(txs); i++ { + prev := gjson.Get(txs[i-1].Raw, "auth_info.fee.amount.0.amount") + curr := gjson.Get(txs[i].Raw, "auth_info.fee.amount.0.amount") + if tc.url == "order_by=1" { // ASC + require.LessOrEqual(t, prev.Int(), curr.Int()) + } else { // DESC + require.GreaterOrEqual(t, prev.Int(), curr.Int()) + } + } + }
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/v2-test.yml(1 hunks)scripts/build/testing.mk(1 hunks)tests/systemtests/tx_test.go(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
tests/systemtests/tx_test.go (3)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern tests/**/*: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"
Pattern **/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: tests (00)
- GitHub Check: test-system-v2
- GitHub Check: Analyze
- GitHub Check: golangci-lint
- GitHub Check: Summary
🔇 Additional comments (4)
scripts/build/testing.mk (1)
27-28: LGTM! The changes streamline v2 testing setup.The modifications improve the testing workflow by:
- Directly using
simdv2binary instead of conditional logic- Explicitly setting
COSMOS_BUILD_OPTIONS=v2for test executionThis change aligns well with the PR objective to fix system testing for v2.
tests/systemtests/tx_test.go (2)
347-351: LGTM! Well-structured test case definition.The test case uses clear, descriptive field names and appropriate error handling expectations.
389-393: LGTM! Correct usage of ORDER_BY enum values.The test cases now properly use the enum values (1 for ASC, 2 for DESC) instead of string literals, which aligns with the fix mentioned in the PR objectives.
Also applies to: 396-400
.github/workflows/v2-test.yml (1)
169-169: LGTM! Simplified workflow command.Removing the environment variable from the workflow is correct since it's now handled within the Makefile target.
Description
TestGetTxEvents_GRPCGatewayfixing the test condition (expected and got were reversed)make test-systemlocally on your machineSummary by CodeRabbit
Summary by CodeRabbit
Chores
Tests