-
Notifications
You must be signed in to change notification settings - Fork 0
test: add unit tests for ProtoTimestamp
#6
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe changes add a new test module in Changes
Sequence Diagram(s)sequenceDiagram
participant T as Test Module
participant PT as ProtoTimestamp
participant P as Panic Handler
%% Normal conversion and extension flow
T->>PT: Create ProtoTimestamp from Timestamp
PT-->>T: Return valid ProtoTimestamp
T->>PT: Call extend_by_seconds(valid_increment)
PT-->>T: Return extended ProtoTimestamp
%% Overflow scenario flow
T->>PT: Call extend_by_seconds(overflow_value)
PT->>P: Trigger panic with error message
P-->>T: Return error notification
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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
CodeRabbit Configuration File (
|
There was a problem hiding this 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
🧹 Nitpick comments (2)
src/cosmos/proto_timestamp.rs (2)
102-112
: Consider adding expected error message for conversion error test.While this test correctly validates that conversion from u64::MAX to i64 fails, it would be more specific to include the expected error message in the
should_panic
attribute, similar to other tests.#[test] -#[should_panic] +#[should_panic(expected = "out of range integral type conversion attempted")] fn test_extend_by_seconds_conversion_error() {
139-149
: Consider adding expected error message for addition overflow test.This test correctly verifies overflow handling during nanosecond addition, but would be more specific with an expected error message in the
should_panic
attribute, matching the error from line 34.#[test] -#[should_panic] +#[should_panic(expected = "failed to add current nanos")] fn test_to_nanos_addition_overflow() {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/cosmos/proto_timestamp.rs
(1 hunks)
🔇 Additional comments (9)
src/cosmos/proto_timestamp.rs (9)
57-61
: Good test module structure with proper imports.The test module is well-organized with appropriate imports and use statements, allowing tests to access both the module under test and required external dependencies.
62-73
: Comprehensive conversion test.Good implementation of the basic conversion test from
Timestamp
toProtoTimestamp
. The test verifies both seconds and nanos fields are correctly preserved during conversion.
75-87
: Solid happy path test for timestamp extension.This test properly validates that extending a timestamp by seconds works correctly in the normal case, ensuring both the seconds field is incremented and the nanos field remains unchanged.
89-100
: Good overflow test with specific error message validation.The test effectively checks that attempting to extend beyond the maximum i64 value triggers the expected error. Using
should_panic
with the expected error message provides strong validation of the error handling logic.
114-125
: Effective nanosecond conversion test.This test correctly validates the timestamp to nanoseconds conversion logic, using a well-chosen example that includes both seconds and partial seconds (as nanoseconds).
127-137
: Beneficial overflow test for maximum seconds.Good test for verifying that converting a timestamp with maximum seconds to nanoseconds correctly handles the overflow condition.
151-167
: Good Header conversion test.This test effectively validates the conversion from a Header with a timestamp to a ProtoTimestamp, ensuring all fields are correctly preserved.
169-178
: Well-defined error case test for missing timestamp.Good test for verifying the error handling when attempting to convert a Header without a timestamp. The expected error message validates the specific error condition.
57-179
: Excellent test coverage for ProtoTimestamp struct.The test suite provides comprehensive coverage of all the public functionality of the ProtoTimestamp struct, including both happy paths and error cases. The tests verify:
- Conversions from different types
- Timestamp extension logic
- Nanosecond conversion
- Multiple overflow conditions
- Error handling
This robust test coverage will help ensure the reliability of the timestamp handling code.
closes #5
description
this PR adds unit tests for the
ProtoTimestamp
struct to ensure all functionality is working correctly. coverage includes:Summary by CodeRabbit