Skip to content

Fix and re-enable test_finalize_validated.#4464

Merged
afck merged 2 commits intolinera-io:mainfrom
afck:test-finalize-validated
Sep 2, 2025
Merged

Fix and re-enable test_finalize_validated.#4464
afck merged 2 commits intolinera-io:mainfrom
afck:test-finalize-validated

Conversation

@afck
Copy link
Contributor

@afck afck commented Sep 2, 2025

Motivation

This test is #[ignore]d because it's flaky: If the faulty validators respond before the other ones start processing the proposal, neither of them will have the proposal in their chain manager.

Also, test_sparse_sender_chain is flaky: If the randomly chosen validator (the first in the committee, i.e. by public key) is faulty, processing the notification will fail.

Proposal

Explicitly handle the proposal in one of the validators to make sure the test is set up for the intended scenario.

In test_sparse_sender_chain, use two validators with no faulty one, so the selected validator is guaranteed to have the block.

Test Plan

I ran it 20 times locally and it always passed.

Release Plan

  • Nothing to do / These changes follow the usual release cycle.
  • (Backporting would be no problem.)

Links

@afck afck requested review from bart-linera and deuszx September 2, 2025 10:13
Copy link
Contributor

@bart-linera bart-linera left a comment

Choose a reason for hiding this comment

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

🎉

@afck afck enabled auto-merge September 2, 2025 10:23
@afck afck added this pull request to the merge queue Sep 2, 2025
@bart-linera
Copy link
Contributor

Just as a note for the future: test_sparse_sender_chains needed fixing because it used one faulty validator, and after #4462 such validators have no chains. Before that PR, they used to have "fake" chains with zero initial balances, and because the test uses a chain that has a zero initial balance anyway, the faulty validator would still have the correct chain and the test would work even if it was chosen as the validator to use when processing the notification. Now, not having the chain, a faulty validator can't process the notification correctly, which is why the test became flaky and setting up the network without faulty validators fixes it.

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 2, 2025
@afck afck added this pull request to the merge queue Sep 2, 2025
Merged via the queue into linera-io:main with commit 699d574 Sep 2, 2025
28 checks passed
@afck afck deleted the test-finalize-validated branch September 2, 2025 11:58
afck added a commit to afck/linera-protocol that referenced this pull request Sep 4, 2025
## Motivation

This test is `#[ignore]`d because it's flaky: If the faulty validators
respond before the other ones start processing the proposal, neither of
them will have the proposal in their chain manager.

Also, `test_sparse_sender_chain` is flaky: If the randomly chosen
validator (the first in the committee, i.e. by public key) is faulty,
processing the notification will fail.

## Proposal

Explicitly handle the proposal in one of the validators to make sure the
test is set up for the intended scenario.

In `test_sparse_sender_chain`, use two validators with no faulty one, so
the selected validator is guaranteed to have the block.

## Test Plan

I ran it 20 times locally and it always passed.

## Release Plan

- Nothing to do / These changes follow the usual release cycle.
- (Backporting would be no problem.)

## Links

- Closes linera-io#3860.
- [reviewer
checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist)
@afck afck mentioned this pull request Sep 4, 2025
ma2bd pushed a commit that referenced this pull request Sep 5, 2025
## Motivation

This test is `#[ignore]`d because it's flaky: If the faulty validators
respond before the other ones start processing the proposal, neither of
them will have the proposal in their chain manager.

Also, `test_sparse_sender_chain` is flaky: If the randomly chosen
validator (the first in the committee, i.e. by public key) is faulty,
processing the notification will fail.

## Proposal

Explicitly handle the proposal in one of the validators to make sure the
test is set up for the intended scenario.

In `test_sparse_sender_chain`, use two validators with no faulty one, so
the selected validator is guaranteed to have the block.

## Test Plan

I ran it 20 times locally and it always passed.

## Release Plan

- Nothing to do / These changes follow the usual release cycle.
- (Backporting would be no problem.)

## Links

- Closes #3860.
- [reviewer
checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist)
ma2bd pushed a commit that referenced this pull request Sep 5, 2025
## Motivation

This test is `#[ignore]`d because it's flaky: If the faulty validators
respond before the other ones start processing the proposal, neither of
them will have the proposal in their chain manager.

Also, `test_sparse_sender_chain` is flaky: If the randomly chosen
validator (the first in the committee, i.e. by public key) is faulty,
processing the notification will fail.

## Proposal

Explicitly handle the proposal in one of the validators to make sure the
test is set up for the intended scenario.

In `test_sparse_sender_chain`, use two validators with no faulty one, so
the selected validator is guaranteed to have the block.

## Test Plan

I ran it 20 times locally and it always passed.

## Release Plan

- Nothing to do / These changes follow the usual release cycle.
- (Backporting would be no problem.)

## Links

- Closes #3860.
- [reviewer
checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist)
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.

Fix test_finalize_validated

2 participants