Skip to content

Conversation

@rlve
Copy link
Contributor

@rlve rlve commented May 12, 2025

This PR tries to revive another stale PR with tests - test(gossipsub): Topic Membership Tests - which I just closed.

Actions taken:

  • copied the tests from the original PR
  • updated them to the latest conventions
  • removed unnecessary tests:
    • "subscription limit test" - test had a wrong assumption around the limit of the topic a local node can subscribe to and test logic itself was incorrect, another test around this already exist:
      asyncTest "subscription limits":
    • "handle JOIN topic and mesh is updated" - doesn't add anything new, repeats "mesh and gossipsub populated when subscribed"
    • "multiple peers join and leave topic simultaneously" - doesn't add anything new, repeats "handle SUBSCRIBE and UNSUBSCRIBE multiple topics"

@rlve rlve requested a review from a team as a code owner May 12, 2025 15:25
@rlve rlve requested review from gmelodie and richard-ramos May 12, 2025 15:25
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.65%. Comparing base (ccb24b5) to head (2d8aa2a).
Report is 1 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1363   +/-   ##
=======================================
  Coverage   84.64%   84.65%           
=======================================
  Files          97       97           
  Lines       17522    17525    +3     
=======================================
+ Hits        14832    14836    +4     
+ Misses       2690     2689    -1     

see 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@gmelodie gmelodie left a comment

Choose a reason for hiding this comment

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

LGTM besides the nitpicks

@github-project-automation github-project-automation bot moved this from new to In Progress in nim-libp2p May 13, 2025
@rlve rlve merged commit 1c1547b into master May 13, 2025
16 of 18 checks passed
@rlve rlve deleted the block6Test-update branch May 13, 2025 15:22
@github-project-automation github-project-automation bot moved this from In Progress to done in nim-libp2p May 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants