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

feat: spec test for connection manager #1417

Merged
merged 13 commits into from
Jul 24, 2023
Merged

Conversation

danisharora099
Copy link
Collaborator

@danisharora099 danisharora099 commented Jul 3, 2023

This PR addresses #1413:

  • adds a spec test for the ConnectionManager module
  • introduces Sinon into our testing suite, along with Chai + Mocha

Cases covered:

  • tests that the peer:discovery event emitter when fired triggers a dial attempt for the peer
  • for bootstrap peers:
    • dials to bootstrap peers are made until the maxBootstrapPeersAllowed flag is reached
  • for non-bootstrap peers (peer-exchange for now):
    • dials to non-bootstrap peers always trigger a dial attempt to them without any upper limits

Notes

Please mention any test cases you think should be added/have been missed for ConnectionManager

@danisharora099 danisharora099 requested a review from a team July 3, 2023 16:28
@danisharora099 danisharora099 changed the title Feat/conn manager spec feat: spec test for connection manager Jul 3, 2023
@github-actions
Copy link

github-actions bot commented Jul 3, 2023

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
Waku core 29.75 KB (-0.03% 🔽) 596 ms (-0.03% 🔽) 1.2 s (+12.39% 🔺) 1.8 s
Waku Simple Light Node 297.56 KB (-0.01% 🔽) 6 s (-0.01% 🔽) 4.7 s (-26.95% 🔽) 10.6 s
ECIES encryption 28.54 KB (0%) 571 ms (0%) 2 s (+14.21% 🔺) 2.6 s
Symmetric encryption 28.55 KB (0%) 572 ms (0%) 1.7 s (-14.46% 🔽) 2.3 s
DNS discovery 108.47 KB (0%) 2.2 s (0%) 3.8 s (+25.44% 🔺) 6 s
Privacy preserving protocols 135.51 KB (-0.01% 🔽) 2.8 s (-0.01% 🔽) 4.9 s (+2.43% 🔺) 7.7 s
Light protocols 31.78 KB (-0.06% 🔽) 636 ms (-0.06% 🔽) 2.3 s (+38.51% 🔺) 3 s
History retrieval protocols 30.42 KB (-0.03% 🔽) 609 ms (-0.03% 🔽) 1.8 s (+6.36% 🔺) 2.4 s
Deterministic Message Hashing 5.78 KB (0%) 116 ms (0%) 729 ms (+0.72% 🔺) 844 ms

this.libp2pComponents.peerStore.addEventListener(
"peer",
this.libp2pComponents.addEventListener(
"peer:discovery",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

haha, good thing we're adding tests :)

);
});

it("should be called for every peer with PEER_EXCHANGE tags", async function () {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we checkng that peer exhcange peers are not limited by the max bootstrap peers?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, that is covered by the "should be called for every peer with PEER_EXCHANGE tags" scenario:

it("should be called for every peer with PEER_EXCHANGE tags", async function () {

@danisharora099 danisharora099 merged commit d2f675d into master Jul 24, 2023
8 of 9 checks passed
@danisharora099 danisharora099 deleted the feat/conn-manager-spec branch July 24, 2023 06:38
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 this pull request may close these issues.

3 participants