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

test(swarm): Wait for swarm1 to disconnect #3465

Merged
merged 2 commits into from
Feb 14, 2023
Merged

Conversation

mxinden
Copy link
Member

@mxinden mxinden commented Feb 14, 2023

Description

This commit does two things:

  • Check that swarm1, given that it has no ban, establishes 21 connections and swarm2, given that it has a ban, establishes 20 connections.

  • Wait for swarm1 to notice disconnect, given that swarm2 closed the connection to the banned swarm1.

Fixes flake introduced in caed1fe.

Notes

Links to any relevant issues

Open Questions

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • A changelog entry has been made in the appropriate crates

This commit does two things:

- Check that swarm1, given that it has no ban, establishes 21 connections and swarm2, given that it
has a ban, establishes 20 connections.

- Wait for swarm1 to notice disconnect, given that swarm2 closed the connection to the banned
swarm1.

Fixes flake introduced in libp2p@caed1fe.
Copy link
Contributor

@thomaseizinger thomaseizinger left a comment

Choose a reason for hiding this comment

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

Thanks!

I don't fully understand why it was flaky before. Should go away soon anyway :)

@mxinden
Copy link
Member Author

mxinden commented Feb 14, 2023

Sorry, I didn't do a good job writing a pull request description.

                    if swarm1.behaviour.on_connection_established.len() == s1_expected_conns
                        && swarm2.behaviour.assert_connected(s2_expected_conns, 2)

s1_expected_conns did not account for swarm1 establishing the banned connection, i.e. it was 20, but should be 21. In case swarm2 going to 20 before swarm1 going to 21, the test would succeed. Otherwise it would not.

Thanks for the review. Merging here to unblock other pull requests.

@mergify mergify bot merged commit 239a62c into libp2p:master Feb 14, 2023
thomaseizinger added a commit that referenced this pull request Feb 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants