Skip to content

Conversation

@StephenButtolph
Copy link
Contributor

Why this should be merged

  1. There are test helpers that aren't used externally, we can move them to files that are only accessible for testing.
  2. Prefixing everything with test isn't really needed if it is defined in a test file.
  3. Some of these helpers weren't used anywhere. Like NoOpGossiper and FullSet.Gossip.
  4. We can simplify the usage of the helpers by using more specific types rather than using structs with a single field.

How this works

Just cleanup, no functional changes.

How this was tested

CI

Need to be documented in RELEASES.md?

No.

@StephenButtolph StephenButtolph self-assigned this Nov 25, 2025
Copilot AI review requested due to automatic review settings November 25, 2025 17:52
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the gossip test infrastructure by moving test helpers from production code into test files and simplifying their implementation. The changes remove unused test utilities and improve code organization by using more specific types instead of wrapper structs.

Key changes:

  • Moved test helper types (testTx, testSet, testMarshaller, NoOpGossiper, TestGossiper, FullSet) from production files to test files
  • Simplified test helper implementation by using type aliases and function types instead of struct wrappers
  • Removed unused test helpers that had no external usage

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
network/p2p/gossip/test_gossip.go Deleted file containing test helpers that are now defined in test files
network/p2p/gossip/handler.go Updated interface assertion to use the Gossipable interface instead of concrete test type
network/p2p/gossip/gossip_test.go Added test helper types (tx, marshaller, setDouble, gossiperFunc, fullSet) previously defined in production code
network/p2p/gossip/gossip.go Removed test helper types (NoOpGossiper, TestGossiper, FullSet) and updated interface assertions
network/p2p/gossip/bloom_test.go Updated test data to use simplified tx type alias

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@StephenButtolph StephenButtolph added this pull request to the merge queue Nov 25, 2025
@StephenButtolph StephenButtolph removed this pull request from the merge queue due to a manual request Nov 25, 2025
@StephenButtolph
Copy link
Contributor Author

hold off until repo merger is done

@StephenButtolph StephenButtolph added this pull request to the merge queue Nov 25, 2025
Merged via the queue into master with commit 5950ba3 Nov 25, 2025
35 checks passed
@StephenButtolph StephenButtolph deleted the simplify-gossip-tests branch November 25, 2025 20:08
@github-project-automation github-project-automation bot moved this to Done 🎉 in avalanchego Nov 25, 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