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

Directly confirm blocks within unit tests rather than starting/forcing elections #4605

Merged

Conversation

clemahieu
Copy link
Contributor

@clemahieu clemahieu commented May 6, 2024

Replaces nano::test::force_confirm with nano::test::confirm and overrides. The action was renamed to test::confirm because the action is to confirm. election::force_confirm is an action specific to an election where the normal path causes confirmation after the vote tally has quorum and force_confirm simulates this.

This removes ledger::force_confirm as it was inappropriately calling the private ledger::confirm(block) rather than the public ledger::confirm(hash).

While this doesn't fix any test specifically it should help with general unit test reliability by not running election functionality when setting up tests.

@clemahieu clemahieu requested a review from dsiganos May 6, 2024 22:14
@clemahieu clemahieu added this to the V27 milestone May 6, 2024
@clemahieu clemahieu 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 May 6, 2024
@clemahieu clemahieu force-pushed the testing_direct_confirmation branch 2 times, most recently from 1205538 to ab9f963 Compare May 8, 2024 13:27
…orcing an election.

Tests wanting to test election behaviour directly can start their own elections.
Though this doesn't fix any specific test problem, this change should be friendly to reducing intermittent test failure due to object interaction effects of running actual network elections.
Tests modified:
active_transactions.confirm_frontier
active_transactions.keep_local
bootstrap_processor.lazy_hash_pruning
bootstrap_processor.lazy_hash_pruning
frontier_req.confirmed_frontier
election.continuous_voting
node.vote_by_hash_bundle
node.epoch_conflict_confirm
node.rollback_vote_self
optimistic_scheduler.activate_one
optimistic_scheduler.under_gap_threshold
rpc.receivable_unconfirmed
rpc.history_pruning
rpc.accounts_receivable_confirmed
rpc.block_confirmed
node.aggressive_flooding
@clemahieu clemahieu force-pushed the testing_direct_confirmation branch from ab9f963 to 4c813b2 Compare May 9, 2024 00:48
@clemahieu clemahieu merged commit 21e48cd into nanocurrency:develop May 9, 2024
25 of 26 checks passed
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.

1 participant