From c73398ab038b39efa7cd377a77acec4ad11c5c74 Mon Sep 17 00:00:00 2001 From: Nikolay Volf Date: Wed, 8 Apr 2020 02:49:32 -0700 Subject: [PATCH] remove flakiness (#5572) --- substrate/Cargo.lock | 4 ++-- substrate/client/transaction-pool/Cargo.toml | 2 +- .../client/transaction-pool/src/revalidation.rs | 13 +++++++++++-- .../client/transaction-pool/src/testing/pool.rs | 15 +++++++-------- 4 files changed, 21 insertions(+), 13 deletions(-) diff --git a/substrate/Cargo.lock b/substrate/Cargo.lock index de2b174826..3eaf51f752 100644 --- a/substrate/Cargo.lock +++ b/substrate/Cargo.lock @@ -2300,9 +2300,9 @@ checksum = "141340095b15ed7491bd3d4ced9d20cebfb826174b6bb03386381f62b01e3d77" [[package]] name = "intervalier" -version = "0.3.1" +version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "14200459dc2319eb13708aed1c1efb8307e0e0e801e7282476939492e1492631" +checksum = "64fa110ec7b8f493f416eed552740d10e7030ad5f63b2308f82c9608ec2df275" dependencies = [ "futures 0.3.4", "futures-timer 2.0.2", diff --git a/substrate/client/transaction-pool/Cargo.toml b/substrate/client/transaction-pool/Cargo.toml index 29b8069842..aeb20e9863 100644 --- a/substrate/client/transaction-pool/Cargo.toml +++ b/substrate/client/transaction-pool/Cargo.toml @@ -24,7 +24,7 @@ sc-transaction-graph = { version = "2.0.0-alpha.5", path = "./graph" } sp-transaction-pool = { version = "2.0.0-alpha.5", path = "../../primitives/transaction-pool" } sc-client-api = { version = "2.0.0-alpha.5", path = "../api" } sp-blockchain = { version = "2.0.0-alpha.5", path = "../../primitives/blockchain" } -intervalier = "0.3.1" +intervalier = "0.4.0" parity-util-mem = { version = "0.6.0", default-features = false, features = ["primitive-types"] } [dev-dependencies] diff --git a/substrate/client/transaction-pool/src/revalidation.rs b/substrate/client/transaction-pool/src/revalidation.rs index 9bcb2dac39..f203bf08a0 100644 --- a/substrate/client/transaction-pool/src/revalidation.rs +++ b/substrate/client/transaction-pool/src/revalidation.rs @@ -30,7 +30,7 @@ use std::time::Duration; #[cfg(not(test))] const BACKGROUND_REVALIDATION_INTERVAL: Duration = Duration::from_millis(200); #[cfg(test)] -pub const BACKGROUND_REVALIDATION_INTERVAL: Duration = Duration::from_millis(5); +pub const BACKGROUND_REVALIDATION_INTERVAL: Duration = Duration::from_millis(1); const BACKGROUND_REVALIDATION_BATCH_SIZE: usize = 20; @@ -214,12 +214,21 @@ impl RevalidationWorker { loop { futures::select! { - _ = interval.next() => { + _guard = interval.next() => { let next_batch = this.prepare_batch(); let batch_len = next_batch.len(); batch_revalidate(this.pool.clone(), this.api.clone(), this.best_block, next_batch).await; + #[cfg(test)] + { + use intervalier::Guard; + // only trigger test events if something was processed + if batch_len == 0 { + _guard.expect("Always some() in tests").skip(); + } + } + if batch_len > 0 || this.len() > 0 { log::debug!( target: "txpool", diff --git a/substrate/client/transaction-pool/src/testing/pool.rs b/substrate/client/transaction-pool/src/testing/pool.rs index c6c54d9720..e7021e8ea0 100644 --- a/substrate/client/transaction-pool/src/testing/pool.rs +++ b/substrate/client/transaction-pool/src/testing/pool.rs @@ -221,7 +221,7 @@ fn should_revalidate_during_maintenance() { block_on(pool.maintain(block_event(1))); assert_eq!(pool.status().ready, 1); - block_on(notifier.next_blocking()); + block_on(notifier.next()); // test that pool revalidated transaction that left ready and not included in the block assert_eq!(pool.api.validation_requests().len(), 3); @@ -264,8 +264,8 @@ fn should_not_retain_invalid_hashes_from_retracted() { block_on(pool.maintain(event)); // maintenance is in background - block_on(notifier.next_blocking()); - + block_on(notifier.next()); + assert_eq!(pool.status().ready, 0); } @@ -281,7 +281,6 @@ fn should_revalidate_transaction_multiple_times() { pool.api.push_block(1, vec![xt.clone()]); block_on(pool.maintain(block_event(1))); - block_on(notifier.next_blocking()); block_on(pool.submit_one(&BlockId::number(0), SOURCE, xt.clone())).expect("1. Imported"); assert_eq!(pool.status().ready, 1); @@ -290,7 +289,7 @@ fn should_revalidate_transaction_multiple_times() { pool.api.add_invalid(&xt); block_on(pool.maintain(block_event(2))); - block_on(notifier.next_blocking()); + block_on(notifier.next()); assert_eq!(pool.status().ready, 0); } @@ -309,14 +308,14 @@ fn should_revalidate_across_many_blocks() { pool.api.push_block(1, vec![]); block_on(pool.maintain(block_event(1))); - block_on(notifier.next_blocking()); + block_on(notifier.next()); block_on(pool.submit_one(&BlockId::number(2), SOURCE, xt3.clone())).expect("1. Imported"); assert_eq!(pool.status().ready, 3); pool.api.push_block(2, vec![xt1.clone()]); block_on(pool.maintain(block_event(2))); - block_on(notifier.next_blocking()); + block_on(notifier.next()); assert_eq!(pool.status().ready, 2); // xt1 and xt2 validated twice, then xt3 once, then xt2 and xt3 again @@ -361,7 +360,7 @@ fn should_push_watchers_during_maintaince() { // clear timer events if any block_on(pool.maintain(block_event(0))); - block_on(notifier.next_blocking()); + block_on(notifier.next()); // then // hash3 is now invalid