Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Ignore flaky test #13631

Merged
merged 5 commits into from
Mar 17, 2023
Merged

Ignore flaky test #13631

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
810f2da
Ignore flaky test
ggwpez Mar 17, 2023
71efc2f
Re-enable running_the_node_works_and_can_be_interrupted
ggwpez Mar 17, 2023
f722f17
Re-enable notifications_back_pressure
ggwpez Mar 17, 2023
81ef3cb
Unused import
ggwpez Mar 17, 2023
6655b0d
Merge master for faster tests
ggwpez Mar 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions client/rpc-spec-v2/src/chain_head/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1024,6 +1024,9 @@ async fn follow_prune_best_block() {
}

#[tokio::test]
Copy link
Member

Choose a reason for hiding this comment

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

CC @lexnv

Copy link
Contributor

Choose a reason for hiding this comment

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

Extracting the error from the CI job(https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2529507/raw):

--- STDERR:              sc-rpc-spec-v2 chain_head::tests::follow_forks_pruned_block ---
thread 'chain_head::tests::follow_forks_pruned_block' panicked at 'assertion failed: `(left == right)`
  left: `Finalized(Finalized { finalized_block_hashes: ["0x0af997b079e96b7f333bf2fe71cb914ad3eb49355923fd3db96c0cc91243bd7a"], pruned_block_hashes: [] })`,
 right: `BestBlockChanged(BestBlockChanged { best_block_hash: "0x0af997b079e96b7f333bf2fe71cb914ad3eb49355923fd3db96c0cc91243bd7a" })

These seem to be flaky for at least follow_forks_pruned_block, I couldn't find in the logs the error for follow_report_multiple_pruned_block but I expect that to also be affected.

Offhand looking at the code it seems that the Finalized event is received before NewBlock event. Which is not generating the BestBlock before the Finalized.

I expect the error is coming from:

substrate/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs

Lines 394 to 398 in 0506da0

self.best_block_cache = Some(*hash);
},
// This is the first best block event that we generate.
None => {
self.best_block_cache = Some(*hash);

Those lines should already be handled by the generate_import_events.

Thanks for pointing this out! Will create a patch for it!

Copy link
Contributor

Choose a reason for hiding this comment

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

Fix published at: #13632, will make a note to reenable the tests if this PR gets merged first! Thanks!

#[cfg(disable_flaky)]
#[allow(dead_code)]
// FIXME: https://github.com/paritytech/substrate/issues/11321
async fn follow_forks_pruned_block() {
let builder = TestClientBuilder::new();
let backend = builder.backend();
Expand Down Expand Up @@ -1137,6 +1140,9 @@ async fn follow_forks_pruned_block() {
}

#[tokio::test]
#[cfg(disable_flaky)]
#[allow(dead_code)]
// FIXME: https://github.com/paritytech/substrate/issues/11321
async fn follow_report_multiple_pruned_block() {
let builder = TestClientBuilder::new();
let backend = builder.backend();
Expand Down
6 changes: 5 additions & 1 deletion client/service/test/src/client/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use sc_consensus::{
};
use sc_service::client::{new_in_mem, Client, LocalCallExecutor};
use sp_api::ProvideRuntimeApi;
use sp_consensus::{BlockOrigin, BlockStatus, Error as ConsensusError, SelectChain};
use sp_consensus::{BlockOrigin, Error as ConsensusError, SelectChain};
use sp_core::{testing::TaskExecutor, traits::CallContext, H256};
use sp_runtime::{
generic::BlockId,
Expand Down Expand Up @@ -1567,7 +1567,11 @@ fn respects_block_rules() {
}

#[test]
#[cfg(disable_flaky)]
#[allow(dead_code)]
// FIXME: https://github.com/paritytech/substrate/issues/11321
fn returns_status_for_pruned_blocks() {
use sc_consensus::BlockStatus;
sp_tracing::try_init_simple();
let tmp = tempfile::tempdir().unwrap();

Expand Down