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

fix(tests): flaky timeout to wait for peer-exchange #2010

Closed
danisharora099 opened this issue May 14, 2024 · 0 comments · Fixed by #2071
Closed

fix(tests): flaky timeout to wait for peer-exchange #2010

danisharora099 opened this issue May 14, 2024 · 0 comments · Fixed by #2071
Assignees
Labels
test Issue related to the test suite with no expected consequence to production code

Comments

@danisharora099
Copy link
Collaborator

This is a change request

Problem

For our compliance test:

// we do this because we want peer-exchange discovery to get initialised before we dial the peer which contains info about the other peer
setTimeout(() => {
void waku.libp2p.dialProtocol(nwaku2Ma, PeerExchangeCodec);
}, 1000);
, we seem to wait 1000ms/1s, to ensure peer-exchange is mounted on the js-waku node.

This is quite hacky, and seems to introduce some race conditions as brought up by @gabrielmer.

Proposed Solutions

Use a more predictable logic that waits for peer-exchange to be mounted.
Eg: peerExchange.isMounted/peerExchange.on("mounted")

@fryorcraken fryorcraken added this to Waku May 14, 2024
@weboko weboko moved this to Triage in Waku May 14, 2024
@weboko weboko added the test Issue related to the test suite with no expected consequence to production code label May 30, 2024
@weboko weboko moved this from Triage to Priority in Waku May 30, 2024
@danisharora099 danisharora099 moved this from Priority to In Progress in Waku Jul 18, 2024
@danisharora099 danisharora099 moved this from In Progress to Code Review / QA in Waku Jul 19, 2024
@github-project-automation github-project-automation bot moved this from Code Review / QA to Done in Waku Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issue related to the test suite with no expected consequence to production code
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants