-
Notifications
You must be signed in to change notification settings - Fork 27
Comprehensive unit tests for eth2network connections and messaging #202
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?
Comprehensive unit tests for eth2network connections and messaging #202
Conversation
We have blockers for this cannot test full protocol functionality without complete ReqResp implementation, we need to fix loop scheduling, resolve memory management these are todos we need to address before we can test real network |
yes we need to test ethlibp2p network connections, leave req/resp out of it, let there be other issues why are they blocker to complete this job |
pkgs/network/src/mock.zig
Outdated
| // TODO: prevent from publishing to self handler | ||
| const self: *Self = @ptrCast(@alignCast(ptr)); | ||
| return self.gossipHandler.onGossip(data, true); | ||
| return self.gossipHandler.onGossip(data, false); // Use sync delivery for tests |
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.
do not change the intended behavior of mock network
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.
I made this so to ensure deterministic delivery without relying on event-loop scheduling, which isn’t running in our unit tests.
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.
no this can't be done, fix tests
though you can have this for your dev, ethlibp2p network anyway doesnt use event loop scheduling which is what this PR has to target
| pub fn onGossip(ptr: *anyopaque, data: *const interface.GossipMessage) anyerror!void { | ||
| const self: *Self = @ptrCast(@alignCast(ptr)); | ||
| return self.gossipHandler.onGossip(data, true); | ||
| return self.gossipHandler.onGossip(data, false); // Use sync delivery for tests |
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.
undo
|
is this PR needed (post fixing the issues) or do we already have tests to cover eth2network? |
Implements comprehensive unit test suite for eth2network connections and primary messaging functionality, covering all network layer components without node dependencies.
closes #159