Add test cases to consensus params validate #1536
Conversation
|
Warning Rate Limit Exceeded@Manav-Aggarwal has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 26 minutes and 44 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. WalkthroughThis update focuses on refining the error handling within the Changes
Assessment against linked issues
Poem
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Manav-Aggarwal
added
T:testing
![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: 3
Configuration used: CodeRabbit UI
Files selected for processing (2)
- types/params.go (1 hunks)
- types/params_test.go (1 hunks)
Additional comments: 7
types/params.go (2)
- 24-33: Use
errors.Newfor static error messages andfmt.Errorffor dynamic error messages. The current usage is correct, but ensure consistency across the codebase.- 38-46: The error wrapping with
%wis correctly used for dynamic error messages, allowing for error unwrapping. This is a good practice.types/params_test.go (5)
- 35-46: The test case "MaxBytes cannot be 0" correctly prepares the test data and expects the appropriate error. This aligns with the validation logic.
- 49-60: The test case "MaxBytes invalid" is well-structured, testing for a specific invalid condition. Ensure that the error message in the assertion matches the one defined in
types/params.go.- 88-101: The test case "VoteExtensionsEnableHeight negative" is correctly set up to test negative values for
VoteExtensionsEnableHeight. This is a good practice for boundary testing.- 104-117: The test case "PubKeyTypes empty" accurately tests the scenario where
PubKeyTypesis an empty slice. This aligns with the validation logic that expects at least one pubkey type.- 120-133: The test case "PubKeyType unknown" tests for an unknown pubkey type, which is a valuable edge case. Ensure that the string "unknownType" does not accidentally become a valid type in the future.
MSevey
deleted the
manav/add_more_test_cases_to_consensus_params_validate
branch
<!-- Please read and fill out this form before submitting your PR. Please make sure you have reviewed our contributors guide before submitting your first PR. --> Closes: rollkit#1379 <!-- Please provide an explanation of the PR, including the appropriate context, background, goal, and rationale. If there is an issue with this information, please provide a tl;dr and link the issue. --> <!-- Please complete the checklist to ensure that the PR is ready to be reviewed. IMPORTANT: PRs should be left in Draft until the below checklist is completed. --> - [x] New and updated code has appropriate documentation - [x] New and updated code has new and/or updated testing - [x] Required CI checks are passing - [ ] Visual proof for any user facing features like CLI or documentation updates - [x] Linked issues closed with keywords <!-- This is an auto-generated comment: release notes by coderabbit.ai --> - **Refactor** - Improved error handling in consensus parameter validation for enhanced clarity in error messages. - **Tests** - Added new test cases for consensus parameter validation to ensure robustness and reliability. <!-- end of auto-generated comment: release notes by coderabbit.ai --> move getRandomBlockWithProposer to utils create single getBlockfunction always returning privkey update to context pattern update getValidator to a single function, remove redundant code change height to pointer, allow passing nil for all three types update GetRanomBytes to use Crypto.read, and check for errors extract out repitition of start-stop node code for each test refactor node-creation for fullnodes and lightnodes, add enum type for nodes instead of hardcoded string pass testing.T pointer to helper functions, instead of directly passing assert (only few remaining instances) refactor initializeAndStartNode to handle node teardown, reduce redundant calls to cleanupnode streamline the setdalc pattern using helper function change to use loop for all test cases update helper name, and return app config created inside it
Overview
Closes: #1379
Checklist
Summary by CodeRabbit