Skip to content

[Merged by Bors] - Remove redundant gossipsub tests#4294

Closed
jmcph4 wants to merge 1 commit intosigp:unstablefrom
jmcph4:2335-kill-gossipsub-tests
Closed

[Merged by Bors] - Remove redundant gossipsub tests#4294
jmcph4 wants to merge 1 commit intosigp:unstablefrom
jmcph4:2335-kill-gossipsub-tests

Conversation

@jmcph4
Copy link
Contributor

@jmcph4 jmcph4 commented May 15, 2023

Issue Addressed

#2335

Proposed Changes

  • Remove the lighthouse-network::tests::gossipsub_tests module
  • Remove dead code from the lighthouse-network::tests::common helper module (build_full_mesh)

Additional Info

After discussion with both @divagant-martian and @AgeManning, these tests seem to have two main issues in that they are:

  • Redundant, in that they don't test anything meaningful (due to our handling of duplicate messages)
  • Out-of-place, in that it doesn't really test Lighthouse-specific functionality (rather libp2p functionality)

As such, this PR supersedes #4286.

This commit removes long-redundant gossipsub unit tests.
Copy link
Contributor

@divagant-martian divagant-martian left a comment

Choose a reason for hiding this comment

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

LGTM, ty @jmcph4

@jmcph4 jmcph4 requested a review from pawanjay176 May 16, 2023 01:03
@AgeManning
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request May 16, 2023
## Issue Addressed

#2335 

## Proposed Changes

 - Remove the `lighthouse-network::tests::gossipsub_tests` module
 - Remove dead code from the `lighthouse-network::tests::common` helper module (`build_full_mesh`)

## Additional Info

After discussion with both @divagant-martian and @AgeManning, these tests seem to have two main issues in that they are:

 - Redundant, in that they don't test anything meaningful (due to our handling of duplicate messages)
 - Out-of-place, in that it doesn't really test Lighthouse-specific functionality (rather libp2p functionality)

As such, this PR supersedes #4286.
@bors
Copy link

bors bot commented May 16, 2023

@bors bors bot changed the title Remove redundant gossipsub tests [Merged by Bors] - Remove redundant gossipsub tests May 16, 2023
@bors bors bot closed this May 16, 2023
ghost pushed a commit to oone-world/lighthouse that referenced this pull request Jul 13, 2023
## Issue Addressed

sigp#2335 

## Proposed Changes

 - Remove the `lighthouse-network::tests::gossipsub_tests` module
 - Remove dead code from the `lighthouse-network::tests::common` helper module (`build_full_mesh`)

## Additional Info

After discussion with both @divagant-martian and @AgeManning, these tests seem to have two main issues in that they are:

 - Redundant, in that they don't test anything meaningful (due to our handling of duplicate messages)
 - Out-of-place, in that it doesn't really test Lighthouse-specific functionality (rather libp2p functionality)

As such, this PR supersedes sigp#4286.
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
## Issue Addressed

sigp#2335 

## Proposed Changes

 - Remove the `lighthouse-network::tests::gossipsub_tests` module
 - Remove dead code from the `lighthouse-network::tests::common` helper module (`build_full_mesh`)

## Additional Info

After discussion with both @divagant-martian and @AgeManning, these tests seem to have two main issues in that they are:

 - Redundant, in that they don't test anything meaningful (due to our handling of duplicate messages)
 - Out-of-place, in that it doesn't really test Lighthouse-specific functionality (rather libp2p functionality)

As such, this PR supersedes sigp#4286.
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
## Issue Addressed

sigp#2335 

## Proposed Changes

 - Remove the `lighthouse-network::tests::gossipsub_tests` module
 - Remove dead code from the `lighthouse-network::tests::common` helper module (`build_full_mesh`)

## Additional Info

After discussion with both @divagant-martian and @AgeManning, these tests seem to have two main issues in that they are:

 - Redundant, in that they don't test anything meaningful (due to our handling of duplicate messages)
 - Out-of-place, in that it doesn't really test Lighthouse-specific functionality (rather libp2p functionality)

As such, this PR supersedes sigp#4286.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants