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

Move snow.DefaultConsensusContextTest to snowtest.ConsensusContext #2507

Merged
merged 7 commits into from
Dec 19, 2023

Conversation

dhrubabasu
Copy link
Contributor

@dhrubabasu dhrubabasu commented Dec 19, 2023

Why this should be merged

Creates a snowtest pkg to contain all strictly testing-related structs. This segregates it from our production code.

This PR is the first step moving snow.DefaultConsensusContextTest() to snowtest.EmptyConsensusContext()

How this works

Creates the pkg and moves some testing-related things to it.

How this was tested

CI

@dhrubabasu dhrubabasu added testing This primarily focuses on testing cleanup Code quality improvement labels Dec 19, 2023
@dhrubabasu dhrubabasu added this to the v1.10.18 milestone Dec 19, 2023
@dhrubabasu dhrubabasu self-assigned this Dec 19, 2023
@dhrubabasu dhrubabasu marked this pull request as draft December 19, 2023 16:17
@dhrubabasu dhrubabasu changed the base branch from dev to snow-deadcode December 19, 2023 16:18
Base automatically changed from snow-deadcode to dev December 19, 2023 16:59
@dhrubabasu dhrubabasu changed the title Create snowtest pkg Move snow.DefaultConsensusContextTest() to snowtest.EmptyConsensusContext() Dec 19, 2023
@dhrubabasu dhrubabasu marked this pull request as ready for review December 19, 2023 19:29
@dhrubabasu dhrubabasu changed the title Move snow.DefaultConsensusContextTest() to snowtest.EmptyConsensusContext() Move snow.DefaultConsensusContextTest to snowtest.EmptyConsensusContext Dec 19, 2023
return nil
}

func EmptyConsensusContext() *snow.ConsensusContext {
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is its own snowtest package we could just call this snowtest.ConsensusContext since it's now clear that this is a testing implementation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, re-named.


var _ snow.Acceptor = noOpAcceptor{}

type noOpAcceptor struct{}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think there's a reason to not export this

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 prefer to keep things unexported unless we plan on using it elsewhere but open to exporting if people feel differently

@dhrubabasu dhrubabasu changed the title Move snow.DefaultConsensusContextTest to snowtest.EmptyConsensusContext Move snow.DefaultConsensusContextTest to snowtest.ConsensusContext Dec 19, 2023
@StephenButtolph StephenButtolph added this pull request to the merge queue Dec 19, 2023
Merged via the queue into dev with commit 7140a84 Dec 19, 2023
17 checks passed
@StephenButtolph StephenButtolph deleted the snowtest-pkg branch December 19, 2023 22:18
tedim52 pushed a commit to tedim52/avalanchego that referenced this pull request Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Code quality improvement testing This primarily focuses on testing
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants