-
Notifications
You must be signed in to change notification settings - Fork 119
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
[ISSUE #1321]🧪Add unit test for OperationResult #1322
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,3 +34,48 @@ impl Default for OperationResult { | |
} | ||
} | ||
} | ||
|
||
#[cfg(test)] | ||
mod tests { | ||
use rocketmq_common::common::message::message_ext::MessageExt; | ||
use rocketmq_remoting::code::response_code::ResponseCode; | ||
|
||
use super::*; | ||
|
||
#[test] | ||
fn default_operation_result_has_none_fields() { | ||
let result = OperationResult::default(); | ||
assert!(result.prepare_message.is_none()); | ||
assert!(result.response_remark.is_none()); | ||
assert_eq!(result.response_code, ResponseCode::Success); | ||
} | ||
|
||
#[test] | ||
fn operation_result_with_some_fields() { | ||
let message = MessageExt::default(); | ||
let remark = Some(String::from("Test remark")); | ||
let response_code = ResponseCode::SystemError; | ||
|
||
let result = OperationResult { | ||
prepare_message: Some(message.clone()), | ||
response_remark: remark.clone(), | ||
response_code, | ||
}; | ||
|
||
assert_eq!(result.response_remark, remark); | ||
assert_eq!(result.response_code, response_code); | ||
} | ||
|
||
#[test] | ||
fn operation_result_with_none_fields() { | ||
let result = OperationResult { | ||
prepare_message: None, | ||
response_remark: None, | ||
response_code: ResponseCode::Success, | ||
}; | ||
|
||
assert!(result.prepare_message.is_none()); | ||
assert!(result.response_remark.is_none()); | ||
assert_eq!(result.response_code, ResponseCode::Success); | ||
} | ||
Comment on lines
+69
to
+80
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider removing redundant test This test appears to be redundant with
If you choose option 2, here's a suggested replacement: #[test]
fn operation_result_can_transition_to_none() {
// Start with Some values
let result = OperationResult {
prepare_message: Some(MessageExt::default()),
response_remark: Some("initial".to_string()),
response_code: ResponseCode::Success,
};
// Transition to None
let result = OperationResult {
prepare_message: None,
response_remark: None,
..result
};
assert!(result.prepare_message.is_none(), "prepare_message should transition to None");
assert!(result.response_remark.is_none(), "response_remark should transition to None");
} |
||
} | ||
Comment on lines
+38
to
+81
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Additional test scenarios needed for comprehensive coverage Consider adding tests for the following scenarios:
Would you like me to provide example implementations for these additional test cases? |
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.
Test coverage and efficiency improvements needed
There are several issues to address:
prepare_message
field is not being asserted after initializationclone()
calls can be removed