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.

Sign up for GitHub

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

Jump to bottom

test(rln): Implement rln tests #2639

Merged
merged 21 commits into from
Aug 2, 2024
Merged

test(rln): Implement rln tests #2639

merged 21 commits into from
Aug 2, 2024

Conversation

AlejandroCabeza
Copy link
Contributor

Description

Extend RLN tests coverage

Changes

  • Implement test plan

@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 26, 2024 •
edited
Loading

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:2639-rln-v1

Built from 5e8baa5

@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 26, 2024 •
edited
Loading

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:2639-rln-v2

Built from 5e8baa5

rymnc
rymnc reviewed May 14, 2024
View reviewed changes
tests/waku_rln_relay/test_rln_group_manager_onchain.nim Outdated
Comment on lines 73 to 107
asyncTest "should error if contract does not exist":
let manager = await setup()
manager.ethContractAddress = "0x0000000000000000000000000000000000000000"

expect(CatchableError):
await manager.init()

Copy link
Contributor

Choose a reason for hiding this comment

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

this test already exists, i believe

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will double check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't find it.
Perhaps you're referring to "should error on initialization when loaded metadata does not match". I did a quick check on that one and the error message is not the same as the one in this test.

This was referenced May 29, 2024
Open
Open
Closed
Open
@AlejandroCabeza AlejandroCabeza mentioned this pull request Jul 29, 2024
Open
@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 29, 2024 •
edited
Loading

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:2639

Built from a166ca6

@AlejandroCabeza AlejandroCabeza marked this pull request as ready for review July 29, 2024 15:51
rymnc
rymnc reviewed Jul 30, 2024
View reviewed changes
Copy link
Contributor

@rymnc rymnc left a comment

Choose a reason for hiding this comment

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

Left some comments in-line!
This PR has some awesome QoL changes, apologies for the delay in getting it merged!!

tests/node/test_wakunode_sharding.nim Outdated Show resolved Hide resolved
tests/waku_keystore/utils.nim Outdated Show resolved Hide resolved
tests/waku_rln_relay/test_rln_group_manager_onchain.nim Outdated Show resolved Hide resolved
tests/waku_rln_relay/test_wakunode_rln_relay.nim Outdated Show resolved Hide resolved
tests/waku_rln_relay/test_wakunode_rln_relay.nim Outdated Show resolved Hide resolved
tests/waku_rln_relay/utils_onchain.nim Outdated Show resolved Hide resolved
rymnc
rymnc approved these changes Aug 2, 2024
View reviewed changes
Copy link
Contributor

@rymnc rymnc left a comment

Choose a reason for hiding this comment

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

LGTM, checks are failing though, is it due to an unrelated change?

@AlejandroCabeza
Copy link
Contributor Author

LGTM, checks are failing though, is it due to an unrelated change?

I think they are failing in master as well.

@Ivansete-status Can you confirm?

@Ivansete-status
Copy link
Collaborator

LGTM, checks are failing though, is it due to an unrelated change?

I think they are failing in master as well.

@Ivansete-status Can you confirm?

Yes you are absolutely right, this issue also happens in master.

@AlejandroCabeza AlejandroCabeza merged commit a3fa175 into master Aug 2, 2024
10 of 11 checks passed
@AlejandroCabeza AlejandroCabeza deleted the tests/rln branch August 2, 2024 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rymnc rymnc rymnc approved these changes

@Ivansete-status Ivansete-status Ivansete-status approved these changes

Assignees

@AlejandroCabeza AlejandroCabeza

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AlejandroCabeza @Ivansete-status @rymnc