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

Fix unit test bootstrap_processor.lazy_pruning_missing_block #4575

Merged

Conversation

dsiganos
Copy link
Contributor

No description provided.

@dsiganos dsiganos added unit test Related to a new, changed or fixed unit test exclude from changelog Indicates the change is not relevant for appearing in the release changelog labels Apr 18, 2024
@@ -116,6 +116,8 @@ class ledger final
// Returns the next receivable entry equal or greater than 'key'
std::optional<std::pair<nano::pending_key, nano::pending_info>> receivable_lower_bound (secure::transaction const & tx, nano::account const & account, nano::block_hash const & hash) const;
void initialize (nano::generate_cache_flags const &);

public: // for unit tests only
Copy link
Contributor

Choose a reason for hiding this comment

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

This is mainly used by confirming set, so the comment is inaccurate.

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 mean making the member function public for use in unit tests only.
I will make it more explicit.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is still less than ideal. A safe pattern similar to the way it's done in other classes should be used that provides a public force_confirm method with a proper release_assert that guards it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's follow good patterns that are already established and work. A single assert is more descriptive than any comment.

@dsiganos dsiganos merged commit 5ef471f into nanocurrency:develop Apr 26, 2024
22 of 26 checks passed
@dsiganos dsiganos deleted the fix_lazy_pruning_missing_block branch April 26, 2024 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exclude from changelog Indicates the change is not relevant for appearing in the release changelog unit test Related to a new, changed or fixed unit test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants