tests: add tests for more primitive types

[carneiro-cw]

PR Type

Tests, Enhancement

---

Description

• Enhanced serialization and deserialization for multiple types, including BlockFilter, EvmExecution, and ExecutionAccountChanges.
• Added test-specific attributes to various structs and enums to improve testability.
• Implemented DebugAsJson for several types to improve debug output.
• Removed bincode-specific serde tests and focused on JSON serialization.
• Added support for ordered map serialization in test environments.
• Updated and expanded serde tests for primitive types in the mod.rs file.
• Improved case-insensitive deserialization for BlockFilter.

---

Changes walkthrough 📝

	Relevant files
Tests	7 files evm_result.rs Add test attributes to EvmExecutionResult                                src/eth/executor/evm_result.rs Added #[cfg_attr(test, derive(serde::Deserialize, fake::Dummy, PartialEq))] to EvmExecutionResult struct +1/-0      address.rs Add test attributes to Address struct                                        src/eth/primitives/address.rs Added #[cfg_attr(test, derive(PartialOrd, Ord))] to Address struct +1/-0      execution_conflict.rs Add test attributes to ExecutionConflict                                  src/eth/primitives/execution_conflict.rs Added #[cfg_attr(test, derive(serde::Deserialize, fake::Dummy, PartialEq))] to ExecutionConflict enum +1/-0      log_filter.rs Enhance LogFilter for testing                                                        src/eth/primitives/log_filter.rs Added #[cfg_attr(test, derive(serde::Deserialize, fake::Dummy))] to LogFilter struct Modified original_input field serialization for tests +2/-1      log_filter_input.rs Improve LogFilterInput and LogFilterInputTopic for testing src/eth/primitives/log_filter_input.rs Added DebugAsJson and #[cfg_attr(test, derive(fake::Dummy))] to LogFilterInput and LogFilterInputTopic structs +5/-2      mod.rs Update and expand serde tests for primitive types                src/eth/primitives/mod.rs Removed commented-out test generation macros Added new test generation macros for various types Reorganized and expanded the list of types for serde testing +22/-22  transaction_execution.rs Add test attributes to LocalTransactionExecution                  src/eth/primitives/transaction_execution.rs Added #[cfg_attr(test, derive(serde::Deserialize, fake::Dummy, PartialEq))] to LocalTransactionExecution struct +1/-0
Enhancement	5 files block_filter.rs Enhance BlockFilter serialization and deserialization        src/eth/primitives/block_filter.rs Added DebugAsJson and #[cfg_attr(test, derive(fake::Dummy))] to BlockFilter enum Modified serde::Deserialize implementation to handle more cases and be case-insensitive Added support for deserialization from JSON objects +31/-5    execution.rs Improve EvmExecution serialization                                              src/eth/primitives/execution.rs Replaced Debug with DebugAsJson for EvmExecution struct Added #[cfg_attr(test, serde(serialize_with = "ordered_map"))] to changes field +6/-3      execution_account_changes.rs Enhance ExecutionAccountChanges serialization                        src/eth/primitives/execution_account_changes.rs Replaced Debug with DebugAsJson for ExecutionAccountChanges struct Added #[cfg_attr(test, serde(serialize_with = "ordered_map"))] to slots field +6/-1      transaction_mined.rs Use DebugAsJson for TransactionMined                                          src/eth/primitives/transaction_mined.rs Replaced Debug with DebugAsJson for TransactionMined struct +2/-1      ext.rs Add ordered map serialization and update serde tests          src/ext.rs Added ordered_map function for serializing HashMaps as ordered BTrees Removed bincode-specific serde tests from gen_test_serde! macro +12/-19

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit b5254f6)

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Key issues to review Possible Bug The deserialization of BlockFilter now allows for case-insensitive matching of "latest", "pending", and "earliest". This change might introduce unexpected behavior if the system relies on exact string matching elsewhere. Performance Concern The changes field in EvmExecution is now using ordered_map for serialization in test environments. This might impact performance if used in production code. Performance Concern Similar to EvmExecution, the slots field in ExecutionAccountChanges is now using ordered_map for serialization in test environments. This might impact performance if used in production code.

[carneiro-cw]

/describe

[github-actions]

Persistent review updated to latest commit b5254f6

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to b5254f6

Category	Suggestion	Score
Best practice	Remove debug print statement from error handling Remove the debug print statement in the error handling case. Debug prints should not be present in production code. src/eth/primitives/block_filter.rs [103-106] // unhandled type -a => { - println!("{:?}", a); - Err(serde::de::Error::custom("block filter must be a string or integer")) -} +_ => Err(serde::de::Error::custom("block filter must be a string or integer")) Suggestion importance[1-10]: 9 Why: Removing debug print statements from production code is a crucial best practice for security and performance reasons.	9
Enhancement	Simplify the handling of the Object case in the deserialization logic Consider using a more concise and idiomatic approach for handling the serde_json::Value::Object case. The current implementation is verbose and can be simplified. src/eth/primitives/block_filter.rs [82-100] -serde_json::Value::Object(map) => - if let Some((key, value)) = map.iter().next() { - let Some(value_str) = value.as_str() else { - return Err(serde::de::Error::custom("block filter must be a string or integer")); - }; - match key.as_str() { - "Hash" => { - let hash: Hash = value_str.parse().map_err(serde::de::Error::custom)?; - Ok(Self::Hash(hash)) - } - "Number" => { - let number: BlockNumber = value_str.parse().map_err(serde::de::Error::custom)?; - Ok(Self::Number(number)) - } - _ => Err(serde::de::Error::custom("block filter must be a string or integer")), - } - } else { - Err(serde::de::Error::custom("block filter must be a string or integer")) - }, +serde_json::Value::Object(map) => { + let (key, value) = map.iter().next().ok_or_else(|| serde::de::Error::custom("empty object"))?; + let value_str = value.as_str().ok_or_else(|| serde::de::Error::custom("value must be a string"))?; + match key.as_str() { + "Hash" => value_str.parse().map(Self::Hash).map_err(serde::de::Error::custom), + "Number" => value_str.parse().map(Self::Number).map_err(serde::de::Error::custom), + _ => Err(serde::de::Error::custom("invalid key")), + } +}, Suggestion importance[1-10]: 8 Why: The suggestion significantly improves code clarity and reduces verbosity in the Object case handling, making it more idiomatic and easier to maintain.	8
	Simplify the parsing of special keywords using a match expression with lowercase comparison Consider using a match expression instead of multiple if-else statements for parsing special keywords. This can improve readability and maintainability. src/eth/primitives/block_filter.rs [63-72] -match value.as_str() { - // parse special keywords - "latest" | "Latest" => Ok(Self::Latest), - "pending" | "Pending" => Ok(Self::Pending), - "earliest" | "Earliest" => Ok(Self::Earliest), - - // parse hash (64: H256 without 0x prefix; 66: H256 with 0x prefix) +match value.as_str().to_lowercase() { + "latest" => Ok(Self::Latest), + "pending" => Ok(Self::Pending), + "earliest" => Ok(Self::Earliest), s if s.len() == 64 || s.len() == 66 => { let hash: Hash = s.parse().map_err(serde::de::Error::custom)?; + Ok(Self::Hash(hash)) + }, + _ => { + let number: BlockNumber = value.parse().map_err(serde::de::Error::custom)?; + Ok(Self::Number(number)) + } +} Suggestion importance[1-10]: 7 Why: The suggestion improves code readability and maintainability by using a match expression with lowercase comparison, which is more concise and less error-prone.	7
	Use a consistently ordered map type for storage changes Consider using a more specific type for changes instead of HashMap. Since you're using ordered_map for serialization in tests, you might want to use BTreeMap or a custom ordered map implementation to ensure consistent ordering across all use cases. src/eth/primitives/execution.rs [46-48] /// Storage changes that happened during the transaction execution. -#[cfg_attr(test, serde(serialize_with = "ordered_map"))] -pub changes: ExecutionChanges, +pub changes: std::collections::BTreeMap<Address, ExecutionAccountChanges>, Suggestion importance[1-10]: 6 Why: Using a consistently ordered map type like BTreeMap ensures consistent behavior across all use cases, not just in tests, which can prevent subtle bugs.	6

---

Previous suggestions

Suggestions up to commit 67bbd6b

Category	Suggestion	Score
Enhancement	Add error handling and improve assertion messages in the serde_json test function In the serde_json test function, consider adding error handling for the serialization and deserialization operations to provide more informative error messages in case of failure. src/ext.rs [326-342] #[test] pub fn [<serde_json_ $type:snake>]() { // encode let original = <fake::Faker as fake::Fake>::fake::<$type>(&fake::Faker); - let encoded = serde_json::to_value(&original).unwrap(); + let encoded = serde_json::to_value(&original).expect("Failed to encode original value"); // decode - let decoded = serde_json::from_value::<$type>(encoded.clone()).unwrap(); - assert_eq!(decoded, original); + let decoded = serde_json::from_value::<$type>(encoded.clone()).expect("Failed to decode value"); + assert_eq!(decoded, original, "Decoded value doesn't match original"); // re-encode - let reencoded = serde_json::to_value(&decoded).unwrap(); - assert_eq!(reencoded, encoded); + let reencoded = serde_json::to_value(&decoded).expect("Failed to re-encode decoded value"); + assert_eq!(reencoded, encoded, "Re-encoded value doesn't match original encoding"); // re-decode - let redecoded = serde_json::from_value::<$type>(reencoded).unwrap(); - assert_eq!(redecoded, original); + let redecoded = serde_json::from_value::<$type>(reencoded).expect("Failed to re-decode value"); + assert_eq!(redecoded, original, "Re-decoded value doesn't match original"); } Suggestion importance[1-10]: 8 Why: This suggestion significantly improves error handling and provides more informative assertion messages, making debugging easier and tests more robust.	8
	Implement faker::Dummy for external structs or create custom test functions instead of using commented-out gen_test_serde! macro calls Consider removing the commented-out gen_test_serde! macro calls for external structs and internal structs that contain external structs not implementing faker::Dummy. Instead, implement faker::Dummy for these types or create custom test functions for them. src/eth/primitives/mod.rs [117-126] -// TODO: Test external structs and internal structs that contain external strtucts that do no implement faker::Dummy -// gen_test_serde!(ExecutionConflicts); -// gen_test_serde!(ExecutionConflictsBuilder); -// gen_test_serde!(ExternalBlock); -// gen_test_serde!(ExternalReceipt); -// gen_test_serde!(ExternalReceipts); -// gen_test_serde!(ExternalTransaction); -// gen_test_serde!(ExternalTransactionExecution); -// gen_test_serde!(PendingBlock); -// gen_test_serde!(TransactionExecution); +// Implement faker::Dummy for external structs or create custom test functions +// Example: +// impl faker::Dummy<faker::Faker> for ExecutionConflicts { +// fn dummy_with_rng<R: rand::Rng + ?Sized>(_: &faker::Faker, _: &mut R) -> Self { +// // Implement custom dummy generation logic +// } +// } +// +// #[test] +// fn test_execution_conflicts_serde() { +// // Implement custom test logic +// } Suggestion importance[1-10]: 7 Why: This suggestion improves code quality by replacing commented-out code with actionable tasks, enhancing maintainability and test coverage.	7
	Add type annotation and improve assertion message in the serde_debug_json test function In the serde_debug_json test function, consider adding an assertion to compare the original value with the decoded value to ensure that the debug representation can be correctly deserialized back into the original type. src/ext.rs [319-324] #[test] pub fn [<serde_debug_json_ $type:snake>]() { let original = <fake::Faker as fake::Fake>::fake::<$type>(&fake::Faker); let encoded_debug = format!("{:?}", original); - let decoded_debug = serde_json::from_str(&encoded_debug).unwrap(); - assert_eq!(original, decoded_debug); + let decoded_debug: $type = serde_json::from_str(&encoded_debug).unwrap(); + assert_eq!(original, decoded_debug, "Debug representation deserialization mismatch"); } Suggestion importance[1-10]: 6 Why: This suggestion enhances the test function by adding a type annotation and a more descriptive assertion message, improving test clarity and debugging.	6
Maintainability	Group related gen_test_serde! macro calls and add comments for better organization Consider grouping related gen_test_serde! macro calls together and adding comments to explain the groupings. This can improve code organization and readability. src/eth/primitives/mod.rs [128-141] +// Transaction related types gen_test_serde!(LocalTransactionExecution); + +// Log and filter related types gen_test_serde!(LogFilter); gen_test_serde!(LogFilterInput); gen_test_serde!(LogFilterInputTopic); -// TODO: gen_test_serde!(StratusError); -// TODO: gen_test_serde!(TransactionStage); + +// TODO: Implement these tests +// gen_test_serde!(StratusError); +// gen_test_serde!(TransactionStage); + +// Basic types gen_test_serde!(Account); gen_test_serde!(Address); +gen_test_serde!(Bytes); + +// Block related types gen_test_serde!(Block); gen_test_serde!(BlockFilter); gen_test_serde!(BlockHeader); gen_test_serde!(BlockNumber); -gen_test_serde!(Bytes); + +// Input types gen_test_serde!(CallInput); Suggestion importance[1-10]: 5 Why: This suggestion improves code readability and organization by grouping related macro calls and adding explanatory comments, but it's a minor enhancement.	5

[github-actions]

PR Description updated to latest commit (b5254f6)