-
Notifications
You must be signed in to change notification settings - Fork 9
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: dht #7
test: dht #7
Conversation
c14b887
to
dc0898a
Compare
3b5c9f2
to
280fdfb
Compare
As discussed with @jacobheun and @bigs , we will move forward with this PR skipping the |
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.
Just some minor feedback around removing unneeded timeouts.
test/dht/content-routing/go2go.js
Outdated
}) | ||
|
||
it('go peer to go peer', async function () { | ||
this.timeout(30 * 1000) |
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.
We should drop all these timeouts in the content routing. CI is showing sub 1 second test runs, so the standard timeout should be more than fine. This will just make testing take longer when things aren't behaving properly.
test/dht/peer-routing/go2go.js
Outdated
}) | ||
|
||
it('go peer to go peer', async function () { | ||
this.timeout(10 * 1000) |
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.
peer routing tests also show well under 5seconds, we should be able to drop the timeouts in all the peer routing tests.
Added interop tests for dht regarding:
content-routing
content-fetching
peer-routing
Needs:
libp2p/js-libp2p-daemon-client#fix/dht-find-providers
(do not know why this only works with that change though)