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

chore: new lightpush tests #1571

Merged
merged 9 commits into from
Sep 19, 2023
Merged

chore: new lightpush tests #1571

merged 9 commits into from
Sep 19, 2023

Conversation

fbarbu15
Copy link
Collaborator

@fbarbu15 fbarbu15 commented Sep 15, 2023

Problem

Similar to #1552

Solution

Added PR with following fixes:

  • Reached 42 automated tests
  • Added retries on failing code from before each
  • Added some reusable functions regarding message collection and validation

@fbarbu15 fbarbu15 changed the title new lightpush tests chore: new lightpush tests Sep 15, 2023
@github-actions
Copy link

github-actions bot commented Sep 15, 2023

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
Waku core 28.79 KB (0%) 576 ms (0%) 397 ms (-13.48% 🔽) 973 ms
Waku Simple Light Node 310.09 KB (0%) 6.3 s (0%) 1.2 s (-4.84% 🔽) 7.4 s
ECIES encryption 28.68 KB (0%) 574 ms (0%) 447 ms (+17.72% 🔺) 1.1 s
Symmetric encryption 28.68 KB (0%) 574 ms (0%) 396 ms (-13.49% 🔽) 970 ms
DNS discovery 118.59 KB (0%) 2.4 s (0%) 701 ms (+6.98% 🔺) 3.1 s
Privacy preserving protocols 122.6 KB (0%) 2.5 s (0%) 555 ms (-11.97% 🔽) 3.1 s
Light protocols 29.25 KB (0%) 585 ms (0%) 370 ms (+35.02% 🔺) 955 ms
History retrieval protocols 28.34 KB (0%) 567 ms (0%) 434 ms (+49% 🔺) 1.1 s
Deterministic Message Hashing 5.64 KB (0%) 113 ms (0%) 158 ms (+84.48% 🔺) 270 ms

@fbarbu15 fbarbu15 added the E:End-to-end testing See https://github.com/waku-org/pm/issues/34 for details label Sep 15, 2023
@fbarbu15 fbarbu15 marked this pull request as ready for review September 15, 2023 14:50
@fbarbu15 fbarbu15 requested a review from a team as a code owner September 15, 2023 14:50
@@ -0,0 +1,193 @@
import {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: we can rename the file simply utils.ts since it's in tests/light-push already

}
}

export async function runNodes(
Copy link
Collaborator

Choose a reason for hiding this comment

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

we can probably generalise this as a util itself for a scope of all tests

}
}

export function tearDownNodes(nwaku: NimGoNode, waku: LightNode): void {
Copy link
Collaborator

Choose a reason for hiding this comment

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

generalise this for the entire tests scope as well

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added to src

@@ -0,0 +1,227 @@
import { createEncoder, DefaultPubSubTopic } from "@waku/core";
Copy link
Collaborator

Choose a reason for hiding this comment

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

can rename file to index.spec.ts

return this.list[index];
}

async waitForMessages(
Copy link
Collaborator

Choose a reason for hiding this comment

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

this can probably be generalised for the scope of all tests as well

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added all message collection logic to src/message_collector.ts and made it work for both MessageRpcResponse and DecodedMessage
Thanks for the hint

}
}

export async function runNodes(
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we make this configurable to allow arbitrary number of waku and nwaku to be started with this?

something like:

runNodes(content, {nwaku: 3, jswaku: 1})

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we could but we would need to add lots of parameters to runNodes to make it trully generic
And looking at other tests/protocols, most of the have different setup needs

export const messageText = "Light Push works!";
export const messagePayload = { payload: utf8ToBytes(messageText) };

export async function runNodes(
Copy link
Collaborator

Choose a reason for hiding this comment

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

oh, all protocol tests would have their own version of runNodes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

from what I saw, it's just filter and lightpush. The other protocols would not use similar setup-node code. That's why I didn't put it in a global util functions like the others.
And event between filter and lightpush the code is pretty different: different nwaku args, different createLightNode args, different waitForRemotePeer protocols.
I can merge them into one but not sure how much value that will add

Copy link
Collaborator

@danisharora099 danisharora099 left a comment

Choose a reason for hiding this comment

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

thanks!

@fbarbu15
Copy link
Collaborator Author

thanks!

Thank you! Can you please merge it? I don't have access

@danisharora099 danisharora099 merged commit e284c78 into master Sep 19, 2023
10 checks passed
@danisharora099 danisharora099 deleted the chore/new-lightpush-tests branch September 19, 2023 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E:End-to-end testing See https://github.com/waku-org/pm/issues/34 for details
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants