-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[Test] Add unit test for validator Router connections #3198
Changes from 2 commits
f654dae
9c03d9f
2974612
f038b45
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 |
---|---|---|
|
@@ -22,7 +22,7 @@ use core::time::Duration; | |
#[tokio::test] | ||
async fn test_connect_without_handshake() { | ||
// Create 2 routers. | ||
let node0 = validator(0, 2).await; | ||
let node0 = validator(0, 2, true).await; | ||
let node1 = client(0, 2).await; | ||
assert_eq!(node0.number_of_connected_peers(), 0); | ||
assert_eq!(node1.number_of_connected_peers(), 0); | ||
|
@@ -78,7 +78,7 @@ async fn test_connect_without_handshake() { | |
#[tokio::test] | ||
async fn test_connect_with_handshake() { | ||
// Create 2 routers. | ||
let node0 = validator(0, 2).await; | ||
let node0 = validator(0, 2, true).await; | ||
let node1 = client(0, 2).await; | ||
assert_eq!(node0.number_of_connected_peers(), 0); | ||
assert_eq!(node1.number_of_connected_peers(), 0); | ||
|
@@ -150,11 +150,48 @@ async fn test_connect_with_handshake() { | |
} | ||
} | ||
|
||
#[tokio::test] | ||
async fn test_validator_connection() { | ||
// Create 2 routers. | ||
let node0 = validator(0, 2, false).await; | ||
let node1 = validator(0, 2, false).await; | ||
assert_eq!(node0.number_of_connected_peers(), 0); | ||
assert_eq!(node1.number_of_connected_peers(), 0); | ||
|
||
// Enable handshake protocol. | ||
node0.enable_handshake().await; | ||
node1.enable_handshake().await; | ||
|
||
// Start listening. | ||
node0.tcp().enable_listener().await.unwrap(); | ||
node1.tcp().enable_listener().await.unwrap(); | ||
|
||
{ | ||
// Connect node0 to node1. | ||
node0.connect(node1.local_ip()); | ||
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. aren't we racing against the 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. heartbeat is not turned on, only if we call |
||
// Sleep briefly. | ||
tokio::time::sleep(Duration::from_millis(200)).await; | ||
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.
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. Will keep it in mind for future tests, for now I'll keep the logic similar to the rest of the tests in the file. 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. IIRC 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. Just tried it. Just awaiting node0's connect is not enough, because node1 might not be connected yet. I suggest we leave refactoring this file for later. |
||
|
||
print_tcp!(node0); | ||
print_tcp!(node1); | ||
|
||
// Check the TCP level. | ||
assert_eq!(node0.tcp().num_connected(), 1); | ||
assert_eq!(node0.tcp().num_connecting(), 0); | ||
assert_eq!(node1.tcp().num_connected(), 1); | ||
assert_eq!(node1.tcp().num_connecting(), 0); | ||
|
||
// Check the router level. | ||
assert_eq!(node0.number_of_connected_peers(), 1); | ||
assert_eq!(node1.number_of_connected_peers(), 1); | ||
} | ||
} | ||
|
||
#[ignore] | ||
#[tokio::test] | ||
async fn test_connect_simultaneously_with_handshake() { | ||
// Create 2 routers. | ||
let node0 = validator(0, 2).await; | ||
let node0 = validator(0, 2, true).await; | ||
let node1 = client(0, 2).await; | ||
assert_eq!(node0.number_of_connected_peers(), 0); | ||
assert_eq!(node1.number_of_connected_peers(), 0); | ||
|
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.
Note that we have to trust the node type as it's given to us by the peer, I don't think we previously used it for anything so we're introducing a new assumption.
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.
You are right. The easiest immediate resolution is to pass in validator routers explicitly via
--peers
. I added a unit test for it and updated the PR description. 2974612