add integration tests for feetiers module governance messages #459
Conversation
WalkthroughThe changes primarily revolve around enhancing the testing suite for the governance module of the protocol. New test cases have been added to verify the behavior of various functions under different scenarios. A significant addition across multiple test functions is the Changes
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Files selected for processing (1)
- protocol/testing/e2e/gov/feetiers_test.go (1 hunks)
Additional comments (Suppressed): 1
protocol/testing/e2e/gov/feetiers_test.go (1)
- 1-102: The test cases for
MsgUpdatePerpetualFeeParamsinx/feetiersare well structured and cover both success and failure scenarios. The use of table-driven tests makes it easy to add new test cases in the future. However, there is a potential issue with the way the test setup is done.In line 76, you're creating a new instance of the test application for each test case. This could potentially slow down your tests if the number of test cases grows. Consider setting up a single instance of the test application before running the test cases and resetting the state as needed between test cases.
Also, consider adding more test cases to cover other edge cases or failure modes, such as invalid fee parameters or issues with the governance proposal process itself.
tqin7
force-pushed
the
tq/core-567-feetiers
branch
from
4d741fb
to
836bad7
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 3
Files selected for processing (7)
- protocol/testing/e2e/gov/add_new_market_test.go (2 hunks)
- protocol/testing/e2e/gov/bridge_test.go (6 hunks)
- protocol/testing/e2e/gov/feetiers_test.go (1 hunks)
- protocol/testing/e2e/gov/sending_test.go (2 hunks)
- protocol/testing/e2e/gov/stats_test.go (2 hunks)
- protocol/testing/e2e/gov/vest_test.go (1 hunks)
- protocol/testutil/app/gov.go (2 hunks)
Files skipped from review due to trivial changes (1)
- protocol/testing/e2e/gov/add_new_market_test.go
Additional comments (Suppressed): 12
protocol/testing/e2e/gov/sending_test.go (2)
20-26: The new field
expectCheckTxFailshas been added to the test struct. Ensure that this field is properly set in all test cases and that its value correctly reflects whether thecheckTxfunction should fail or not.105-111: The
expectCheckTxFailsfield is now being passed as an argument to theSubmitAndTallyProposalfunction. Make sure that this function handles this new argument correctly, especially in terms of error handling and expected outcomes.protocol/testing/e2e/gov/stats_test.go (2)
17-23: The new field
expectCheckTxFailshas been added to the test struct. Ensure that all instances of this struct in the codebase have been updated accordingly.71-77: The function call now includes an additional argument
tc.expectCheckTxFails. Make sure that the function signature has been updated everywhere it is defined and called.protocol/testing/e2e/gov/bridge_test.go (6)
20-25: The new field
expectCheckTxFailshas been added to the test struct. Ensure that this field is properly initialized in all test cases and its implications are correctly handled in the test logic.68-73: The function call now includes an additional argument
tc.expectCheckTxFails. Make sure that the function definition has been updated to handle this new argument and it's used appropriately within the function.87-92: Similar to the previous comment, the new field
expectCheckTxFailshas been added to the test struct. Ensure that this field is properly initialized in all test cases and its implications are correctly handled in the test logic.136-142: The function call now includes an additional argument
tc.expectCheckTxFails. Make sure that the function definition has been updated to handle this new argument and it's used appropriately within the function.155-161: Again, the new field
expectCheckTxFailshas been added to the test struct. Ensure that this field is properly initialized in all test cases and its implications are correctly handled in the test logic.201-207: The function call now includes an additional argument
tc.expectCheckTxFails. Make sure that the function definition has been updated to handle this new argument and it's used appropriately within the function.protocol/testutil/app/gov.go (2)
35-41: The function signature has been updated to include a new parameter
expectCheckTxFails. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.62-71: The new hunk introduces a conditional check for
expectCheckTxFailswhich determines whether theCheckTxfunction should fail or not. This is a good practice as it allows for more granular control over the expected behavior of the test, improving its robustness.
tqin7
force-pushed
the
tq/core-567-feetiers
branch
from
836bad7
to
8a38ffb
Compare
tqin7
force-pushed
the
tq/core-567-feetiers
branch
from
8a38ffb
to
686d737
Compare
No description provided.