-
Notifications
You must be signed in to change notification settings - Fork 42
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
chore: upgrade nwaku to 0.34.0 and update tests suite for compatibility #2170
base: master
Are you sure you want to change the base?
Conversation
size-limit report 📦
|
19b817a
to
36952bd
Compare
e27033a
to
c48d1b8
Compare
Is it ready for review? Both test steps has failed :( |
All of the tests seem to pass, but the CI runner doesn't seem to exit out of the process |
b78448b
to
4edbc06
Compare
20c8e4f
to
5d209e9
Compare
await sendMessagesAutosharding(nwaku, totalMsgs, customContentTopic1); | ||
await sendMessagesAutosharding(nwaku, totalMsgs, customContentTopic2); | ||
for (let i = 0; i < totalMsgs; i++) { | ||
await serviceNodes.sendRelayMessage( |
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.
previous method seems more convenient that that
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.
true, something buggy with the autosharding API, confirming -- similar reason for #2170 (comment)
@@ -236,54 +255,6 @@ describe("Waku Store (Autosharding), custom pubsub topic", function () { | |||
}); | |||
expect(result2).to.not.eq(-1); | |||
}); | |||
|
|||
it("Generator, 2 nwaku nodes each with different pubsubtopics", async function () { |
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.
why removed? it seems like a viable test case
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.
good point
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.
overall looks good, asked some questions and let's remove logs
looks good, node tests fail tho |
…f just one) - nwaku now expects >=1 nodes at least connected
01f61c5
to
98179ce
Compare
Description
This pull request upgrades the nwaku dependency to version
0.34.0
and includes various improvements and fixes in the test suite to accommodate this upgrade. Key changes include:Notes
nwaku
to0.34.0
and check interop tests #2176Contribution checklist:
!
in title if breaks public API;