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

Add test to beacon node fallback feature #6568

Merged
merged 118 commits into from
Feb 4, 2025

Conversation

chong-he
Copy link
Member

@chong-he chong-he commented Nov 6, 2024

Add test to beacon node fallback using mockito crate: https://crates.io/crates/mockito to create mock beacon nodes. The tests include:

  • testing the update_all_candidates function in update_all_candidates_should_update_sync_status
  • testing the broadcast function in broadcast_should_send_to_all_bns
  • testing the first_success function in first_success_should_try_nodes_in_order

A majority part of the code was written by @jimmygchen, thank you Jimmy for the help and guidance, I learn a lot along the way.

Copy link
Member

@jimmygchen jimmygchen left a comment

Choose a reason for hiding this comment

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

@chong-he Nice, I've added some comments but looking good!

Copy link
Member

@jimmygchen jimmygchen left a comment

Choose a reason for hiding this comment

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

Thanks @chong-he, looks good to me now.

@jimmygchen jimmygchen added ready-for-merge This PR is ready to merge. ready-for-review The code is ready for review and removed ready-for-review The code is ready for review ready-for-merge This PR is ready to merge. labels Jan 9, 2025
Copy link
Member

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

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

Cleaner and simpler than I thought it would be! Nice work!

Copy link

mergify bot commented Feb 4, 2025

This pull request has merge conflicts. Could you please resolve them @chong-he? 🙏

@jimmygchen jimmygchen added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Feb 4, 2025
@jimmygchen
Copy link
Member

Great work, thanks @chong-he ☺️

@mergify mergify bot merged commit 3d06bc2 into sigp:unstable Feb 4, 2025
31 checks passed
@chong-he chong-he deleted the vc-fallback-test branch February 13, 2025 05:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge This PR is ready to merge. test improvement Improve tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants