test(peer-and-connection-management): Implement tests #2566
Conversation
|
You can find the image built from this PR at Built from 70cc31e |
AlejandroCabeza
force-pushed
the
test-peer-connection-management
branch
3 times, most recently
from
bbebbac
to
3cea89b
Compare
There was a problem hiding this comment.
Thanks for that! 🥳
I just added some non-blocking questions. Doubts that I have mostly because I may miss something :)
Aside note: I think we should invest efforts on making xasyncTests available. I think you are doing an outstanding work on those and is a pitty that they could get there forgotten. I mean, is great to add TODO for future revision but I think we should invest some time to fix those
| await server2.stop() | ||
|
|
||
| suite "Tracked Peer Metadata": | ||
| xasyncTest "Metadata Recording": |
There was a problem hiding this comment.
Just to confirm, we have a related issue for all those tests labeled as xasyncTest right?
| serverPeerStore.get(clientPeerId).connectedness == Connectedness.Connected | ||
|
|
||
| suite "Handling Connections on Different Networks": | ||
| # TODO: Implement after discv5 and peer manager's interaction is understood |
There was a problem hiding this comment.
Maybe we can have a 1:1 session next week to try to understand them together :D
There was a problem hiding this comment.
Fine by me :)
| let | ||
| privKey = generateSecp256k1Key() | ||
| extIp = "127.0.0.1" | ||
| tcpPort = 61500u16 |
There was a problem hiding this comment.
in this case, it cannot connect because the port isn't valid right? If that's the case, maybe interesting to add a comment
There was a problem hiding this comment.
AFAIK, the port is valid. The reason it can't connect is because that server hasn't been started.
| check: | ||
| clientPeerStore.peers().len == 1 |
There was a problem hiding this comment.
For some reason, I was expecting this to be 0 , considering the capacity == 1
There was a problem hiding this comment.
Not following you. Can you repeat?
There was a problem hiding this comment.
Not following you. Can you repeat?
hahah yes, given that we are setting capacity == 1 I wonder why clientPeerStore.peers().len == 1
There was a problem hiding this comment.
Doesn't it make sense? What would make sense to you?
There was a problem hiding this comment.
Doesn't it make sense? What would make sense to you?
Sorry, I expressed myself wrongly and twice.
I mean that, given that we are setting clientPeerStore.capacity = 0 at the beginning of the test, I was expecting that no peers could get connected and therefore, I was expecting this: clientPeerStore.peers().len == 0
There was a problem hiding this comment.
@AlejandroCabeza - could it happen that clientPeerStore.capacity = 0 means unlimited?
There was a problem hiding this comment.
That's actually very likely the case, same as with the colocationLimit.
|
|
||
| # Then the server should have all clients in its peer store | ||
| check: | ||
| serverPeerStore.peers().len == 3 |
There was a problem hiding this comment.
Shouldn't this be 0?
There was a problem hiding this comment.
No, because colocationLimit == 0 means "unlimited".
AlejandroCabeza
force-pushed
the
test-peer-connection-management
branch
from
2a973bd
to
6cc5401
Compare
AlejandroCabeza
force-pushed
the
test-peer-connection-management
branch
from
6cc5401
to
66ae6a6
Compare
|
You can find the image built from this PR at Built from 169b604 |
|
You can find the image built from this PR at Built from 169b604 |
* Implement peer and connection management tests. * Fix multiple peers added on initialisation. * Remove clusterId parameter from newTestWakuNode.
Description
Implement tests for peer and connection management
Changes
peersByShardinitialisation.clusterIdparameter fromnewTestWakuNode(unused).NotConnectedonaddPeer.