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

Refactor LC handlers Tests #14834

Merged
merged 5 commits into from
Feb 7, 2025

Conversation

Inspector-Butters
Copy link
Contributor

What type of PR is this?
Other

What does this PR do? Why is it needed?
This PR refactors the light client handlers tests, and bundles them together, to make it easier and cleaner to add more tests.

Which issues(s) does this PR fix?
Part of #12991

Other notes for review
I don't think this PR needs a changelog entry since it's not user-facing and it does not add/remove/change functionality.


db := dbtesting.SetupDB(t)
db := dbtesting.SetupDB(t)
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be done once before all tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I were to call it only once before all tests, then I need a way to clean the db at the end of each test, so the next test gets a fresh db, because otherwise they will have conflicting data. but I can't find any methods that does this.
Also if we can't save all the updates at once, since then the tests would be dependent on each other's ordering. and also some of the tests need "broken" updates to be stored in the db, so it would mess up the other tests' point.

}

func TestLightClientHandler_GetLightClientOptimisticUpdateAltair(t *testing.T) {
func TestLightClientHandler_GetLightClientFinalityUpdate(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

you only have Altair covered

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this PR is supposed to be just a refactor of the tests and not to add any new functionality/test.

@rkapka rkapka enabled auto-merge February 7, 2025 15:34
@rkapka rkapka added this pull request to the merge queue Feb 7, 2025
Merged via the queue into prysmaticlabs:develop with commit 7c17af2 Feb 7, 2025
15 of 16 checks passed
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.

2 participants