-
Notifications
You must be signed in to change notification settings - Fork 4
Added Simple Unit Tests #7
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
Conversation
LeoPatOZ
commented
Sep 10, 2025
- Unit tests for builder and scanner
fmt git import
src/block_scanner.rs
Outdated
| } | ||
|
|
||
| pub fn with_blocks_read_per_epoch(&mut self, blocks_read_per_epoch: u64) -> &mut Self { | ||
| // TODO: Clamp this value to a reasonable range |
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.
There is a DEFAULT_BLOCKS_READ_PER_EPOCH value set by default, isn't it safe to assume that the dev overriding this value knows what they're doing?
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.
yeah i guess should be fine, I think the get_Logs will fail otherwise
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.
yeah i guess should be fine, I think the get_Logs will fail otherwise.
This is a config param set by the node operators https://github.com/paradigmxyz/reth/blob/edc1ae8f4dd7d222e3de986794b2d0082fed65af/crates/rpc/rpc-server-types/src/constants.rs#L13 so ultimately its up to them
| const WS_URL: &str = "ws://localhost:8545"; | ||
|
|
||
| #[test] | ||
| fn test_builder_new_defaults() { |
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.
although we're changing the design, these tests might come in handy for the event scanner, so we might as well keep them temporarily
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.
yeah wasnt sure what to do as we transition
|
Overall, solid base test cases ✅ |