-
Notifications
You must be signed in to change notification settings - Fork 999
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
Tests for is_pending_outbound #1938
Tests for is_pending_outbound #1938
Conversation
Continuation of PR: libp2p#1928 Signed-off-by: Tejas Sanap <[email protected]>
Hi @mxinden, diff --git a/protocols/request-response/tests/ping.rs b/protocols/request-response/tests/ping.rs
index b03931cd..5622fb06 100644
--- a/protocols/request-response/tests/ping.rs
+++ b/protocols/request-response/tests/ping.rs
@@ -274,6 +274,28 @@ fn ping_protocol_throttled() {
pool.run_until(peer2);
}
+#[test]
+fn is_pending_outbound() {
+ let ping = Ping("ping".to_string().into_bytes());
+ let offline_peer = PeerId::random();
+
+ let protocols = iter::once((PingProtocol(), ProtocolSupport::Full));
+ let cfg = RequestResponseConfig::default();
+
+ let (peer1_id, trans) = mk_transport();
+ let ping_proto1 = RequestResponse::throttled(PingCodec(), protocols.clone(), cfg.clone());
+ let mut swarm1 = Swarm::new(trans, ping_proto1, peer1_id.clone());
+
+ let request_id_1 = swarm1.send_request(offline_peer, ping.clone());
+
+ // Poll swarm until request 1 is reported as failed.
+
+ let request_id_2 = swarm1.send_request(offline_peer, ping.clone());
+
+ assert!(!swarm1.is_pending_outbound(offline_peers, request_id_1));
+ assert!(swarm1.is_pending_outbound(offline_peers, request_id_2));
+}
+ Here by poll do you mean the poll function? Here is the doc for the poll function, just to point you to what I mean. |
let request_id_1 = swarm1.send_request(&offline_peer, ping.clone()).unwrap(); | ||
|
||
// Poll swarm until request 1 is reported as failed. | ||
swarm1.network.poll(); |
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.
Polling the swarm a single time likely won't do it here.
In case you want to read up on the concept of asynchronous programming in Rust, or the concept of a Future
and its poll
function, I recommend the async-book.
Below is a concrete example of polling a swarm until some event X happens:
rust-libp2p/protocols/request-response/tests/ping.rs
Lines 98 to 116 in 6d0773b
loop { | |
match swarm2.next().await { | |
RequestResponseEvent::Message { | |
peer, | |
message: RequestResponseMessage::Response { request_id, response } | |
} => { | |
count += 1; | |
assert_eq!(&response, &expected_pong); | |
assert_eq!(&peer, &peer1_id); | |
assert_eq!(req_id, request_id); | |
if count >= num_pings { | |
return | |
} else { | |
req_id = swarm2.send_request(&peer1_id, ping.clone()); | |
} | |
}, | |
e => panic!("Peer2: Unexpected event: {:?}", e) | |
} | |
} |
Hi @mxinden, |
Signed-off-by: Tejas Sanap <[email protected]>
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.
Two comments relating to coding style, other than that, this looks good to me.
let peer1 = async move { | ||
match swarm1.next().await { | ||
RequestResponseEvent::OutboundFailure{peer, request_id: req_id, error: _error} => { | ||
assert_eq!(&offline_peer, &peer); | ||
assert_eq!(req_id, request_id1); | ||
}, | ||
e => panic!("Peer: Unexpected event: {:?}", e), | ||
} | ||
|
||
let request_id2 = swarm1.send_request(&offline_peer, ping.clone()); | ||
|
||
assert!(!swarm1.is_pending_outbound(&offline_peer, &request_id1)); | ||
assert!(swarm1.is_pending_outbound(&offline_peer, &request_id2)); | ||
}; | ||
|
||
let () = async_std::task::block_on(peer1); |
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.
let peer1 = async move { | |
match swarm1.next().await { | |
RequestResponseEvent::OutboundFailure{peer, request_id: req_id, error: _error} => { | |
assert_eq!(&offline_peer, &peer); | |
assert_eq!(req_id, request_id1); | |
}, | |
e => panic!("Peer: Unexpected event: {:?}", e), | |
} | |
let request_id2 = swarm1.send_request(&offline_peer, ping.clone()); | |
assert!(!swarm1.is_pending_outbound(&offline_peer, &request_id1)); | |
assert!(swarm1.is_pending_outbound(&offline_peer, &request_id2)); | |
}; | |
let () = async_std::task::block_on(peer1); | |
match futures::executor::block_on(swarm1.next()) { | |
RequestResponseEvent::OutboundFailure{peer, request_id: req_id, error: _error} => { | |
assert_eq!(&offline_peer, &peer); | |
assert_eq!(req_id, request_id1); | |
}, | |
e => panic!("Peer: Unexpected event: {:?}", e), | |
} | |
let request_id2 = swarm1.send_request(&offline_peer, ping.clone()); | |
assert!(!swarm1.is_pending_outbound(&offline_peer, &request_id1)); | |
assert!(swarm1.is_pending_outbound(&offline_peer, &request_id2)); |
This should make it a bit more readable. What do you think?
/// Check if is_response_outbound works properly | ||
#[test] | ||
fn offline_peers() { |
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.
/// Check if is_response_outbound works properly | |
#[test] | |
fn offline_peers() { | |
#[test] | |
fn is_response_outbound() { |
Signed-off-by: Tejas Sanap <[email protected]>
@mxinden, done! |
Thanks @whereistejas. |
Continuation of PR: #1928
Signed-off-by: Tejas Sanap [email protected]