Skip to content
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

connectSome test method #240

Closed
achingbrain opened this issue May 1, 2022 · 2 comments · Fixed by #241
Closed

connectSome test method #240

achingbrain opened this issue May 1, 2022 · 2 comments · Fixed by #241
Assignees

Comments

@achingbrain
Copy link
Collaborator

achingbrain commented May 1, 2022

The connectSome looks like it makes num connections between the nodes in gss. It will not connect nodes to themselves but it doesn't seem to prevent connecting A to B and then B to A and calling that two connections.

Sometimes the tests demand a certain number of connections for each node after using connectSome but there does not appear to be any guarantee this will actually happen due to the above, which may be one reason why the more complicated tests in this module can be a bit flaky.

@achingbrain
Copy link
Collaborator Author

Actually it's kind of worse because i doesn't get incremented while j < num is true, so the node for i = 0 gets loads of connections but high i values might not get any, so it can cause disjoint networks to be formed.

A better algorithm might be:

  1. Decrement num after any connection is created, return when num === 0
  2. Reject any num value that is less than gss.length
  3. Maintain a collection of connected nodes
  4. Connect two nodes from gss and add them to the connected nodes collection
  5. Until all nodes have one connection, connect them to a random node in the connected nodes collection and then store them in the connected nodes collection
  6. Until num === 0 connect two random nodes from the connected nodes collection that are not the same node and are not already connected

@achingbrain
Copy link
Collaborator Author

achingbrain commented May 1, 2022

Ah, no - forget it. It's supposed to ensure every member of gss has num peers, not num connections between gss members.

There is a bug in there though, if it generates the same value of n twice it treats it as two separate connections, so there's no guarantee each member of gss will have num connections which can cause promises like these to never resolve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants