Unverified Commit cc2d7afd authored by Peter Goodspeed-Niklaus's avatar Peter Goodspeed-Niklaus Committed by GitHub
Browse files

Add test suite and minor refinements to the utility subsystem (#1403)

* get conclude signal working properly; don't allocate a vector

* wip: add test suite / example / explanation for using utility subsystem

Unfortunately, the test fails right now for reasons which seem
very odd. Just have to keep poking at it.

* explicitly import everything

* fix subsystem-util test

The root problem here was two-fold:

- there was a circular dependency from subsystem -> test-helpers/subsystem ->
  subsystem
- cfg(test) doesn't propagate between crates

The solution: move the subsystem test helpers into a sub-module
within subsystem. Publicly export them from the previous location
so no other code breaks.

Doing this has an additional benefit: it ensures that no production
code can ever accidentally use the subsystem helpers, as they are compile-
gated on cfg(test).

* fully commit to moving test helpers into a subsystem module

* add some more tests

* get rid of log tests in favor of real error forwarding

It's not obvious whether we'll ever really want to chase down
these errors outside a testing context, but having the capability
won't hurt.

* fix issue which caused test to hang on osx

* only require that job errors are PartialEq when testing

also fix polkadot-node-core-backing tests

* get rid of any notion of partialeq

* rethink testing

Combine tests of starting and stopping job: leaving a test executor
with a job running was pretty clearly the cause of the sometimes-hang.

Also, add a timeout so tests _can't_ hang anymore; they just fail
after a while.

* rename fwd_errors -> forward_errors

* warn on error propagation failure

* fix unused import leftover from merge

* derive eq for subsystemerror
parent 57b57ab3
Pipeline #101099 passed with stages
in 19 minutes and 59 seconds
......@@ -4457,7 +4457,6 @@ dependencies = [
"polkadot-node-primitives",
"polkadot-node-subsystem",
"polkadot-primitives",
"polkadot-subsystem-test-helpers",
"sc-network",
"sp-core",
"sp-runtime",
......@@ -4498,7 +4497,6 @@ dependencies = [
"polkadot-node-subsystem",
"polkadot-primitives",
"polkadot-statement-table",
"polkadot-subsystem-test-helpers",
"sc-client-api",
"sc-keystore",
"sp-api",
......@@ -4549,12 +4547,14 @@ dependencies = [
name = "polkadot-node-subsystem"
version = "0.1.0"
dependencies = [
"assert_matches",
"async-trait",
"derive_more 0.99.9",
"futures 0.3.5",
"futures-timer 3.0.2",
"log 0.4.8",
"parity-scale-codec",
"parking_lot 0.10.2",
"pin-project",
"polkadot-node-primitives",
"polkadot-primitives",
......@@ -4615,7 +4615,6 @@ dependencies = [
"polkadot-node-primitives",
"polkadot-node-subsystem",
"polkadot-primitives",
"polkadot-subsystem-test-helpers",
"sc-network",
"sp-core",
"sp-runtime",
......@@ -4970,7 +4969,6 @@ dependencies = [
"polkadot-node-primitives",
"polkadot-node-subsystem",
"polkadot-primitives",
"polkadot-subsystem-test-helpers",
"sp-core",
"sp-keyring",
"sp-runtime",
......@@ -4987,17 +4985,6 @@ dependencies = [
"sp-core",
]
[[package]]
name = "polkadot-subsystem-test-helpers"
version = "0.1.0"
dependencies = [
"async-trait",
"futures 0.3.5",
"parking_lot 0.10.2",
"polkadot-node-subsystem",
"sp-core",
]
[[package]]
name = "polkadot-test-runtime"
version = "0.8.17"
......
......@@ -52,7 +52,6 @@ members = [
"node/service",
"node/core/backing",
"node/subsystem",
"node/test-helpers/subsystem",
"node/test-service",
"parachain/test-parachains",
......
......@@ -22,5 +22,5 @@ bitvec = { version = "0.17.4", default-features = false, features = ["alloc"] }
sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" }
sp-keyring = { git = "https://github.com/paritytech/substrate", branch = "master" }
futures = { version = "0.3.5", features = ["thread-pool"] }
subsystem-test = { package = "polkadot-subsystem-test-helpers", path = "../../test-helpers/subsystem" }
assert_matches = "1.3.0"
polkadot-subsystem = { package = "polkadot-node-subsystem", path = "../../subsystem", features = [ "test-helpers" ] }
......@@ -745,7 +745,7 @@ where
/// Run this subsystem
pub async fn run(ctx: Context, keystore: KeyStorePtr, spawner: Spawner) {
<Manager<Spawner, Context>>::run(ctx, keystore, spawner).await
<Manager<Spawner, Context>>::run(ctx, keystore, spawner, None).await
}
}
......@@ -895,13 +895,13 @@ mod tests {
}
struct TestHarness {
virtual_overseer: subsystem_test::TestSubsystemContextHandle<CandidateBackingMessage>,
virtual_overseer: polkadot_subsystem::test_helpers::TestSubsystemContextHandle<CandidateBackingMessage>,
}
fn test_harness<T: Future<Output=()>>(keystore: KeyStorePtr, test: impl FnOnce(TestHarness) -> T) {
let pool = sp_core::testing::SpawnBlockingExecutor::new();
let (context, virtual_overseer) = subsystem_test::make_subsystem_context(pool.clone());
let (context, virtual_overseer) = polkadot_subsystem::test_helpers::make_subsystem_context(pool.clone());
let subsystem = CandidateBackingSubsystem::run(context, keystore, pool.clone());
......@@ -959,7 +959,7 @@ mod tests {
// Tests that the subsystem performs actions that are requied on startup.
async fn test_startup(
virtual_overseer: &mut subsystem_test::TestSubsystemContextHandle<CandidateBackingMessage>,
virtual_overseer: &mut polkadot_subsystem::test_helpers::TestSubsystemContextHandle<CandidateBackingMessage>,
test_state: &TestState,
) {
// Start work on some new parent.
......
......@@ -18,6 +18,5 @@ polkadot-subsystem = { package = "polkadot-node-subsystem", path = "../../subsys
[dev-dependencies]
parking_lot = "0.10.0"
subsystem-test = { package = "polkadot-subsystem-test-helpers", path = "../../test-helpers/subsystem" }
assert_matches = "1.3.0"
sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" }
......@@ -531,7 +531,7 @@ mod tests {
use assert_matches::assert_matches;
use polkadot_subsystem::messages::{StatementDistributionMessage, BitfieldDistributionMessage};
use subsystem_test::{SingleItemSink, SingleItemStream};
use polkadot_subsystem::test_helpers::{SingleItemSink, SingleItemStream};
// The subsystem's view of the network - only supports a single call to `event_stream`.
struct TestNetwork {
......@@ -550,7 +550,7 @@ mod tests {
TestNetwork,
TestNetworkHandle,
) {
let (net_tx, net_rx) = subsystem_test::single_item_sink();
let (net_tx, net_rx) = polkadot_subsystem::test_helpers::single_item_sink();
let (action_tx, action_rx) = mpsc::unbounded();
(
......@@ -631,13 +631,13 @@ mod tests {
struct TestHarness {
network_handle: TestNetworkHandle,
virtual_overseer: subsystem_test::TestSubsystemContextHandle<NetworkBridgeMessage>,
virtual_overseer: polkadot_subsystem::test_helpers::TestSubsystemContextHandle<NetworkBridgeMessage>,
}
fn test_harness<T: Future<Output=()>>(test: impl FnOnce(TestHarness) -> T) {
let pool = sp_core::testing::SpawnBlockingExecutor::new();
let (network, network_handle) = new_test_network();
let (context, virtual_overseer) = subsystem_test::make_subsystem_context(pool);
let (context, virtual_overseer) = polkadot_subsystem::test_helpers::make_subsystem_context(pool);
let network_bridge = run_network(
network,
......
......@@ -18,6 +18,6 @@ polkadot-subsystem = { package = "polkadot-node-subsystem", path = "../../subsys
[dev-dependencies]
parking_lot = "0.10.0"
subsystem-test = { package = "polkadot-subsystem-test-helpers", path = "../../test-helpers/subsystem" }
assert_matches = "1.3.0"
sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" }
polkadot-subsystem = { package = "polkadot-node-subsystem", path = "../../subsystem", features = [ "test-helpers" ] }
......@@ -620,7 +620,7 @@ mod tests {
};
let pool = sp_core::testing::SpawnBlockingExecutor::new();
let (mut ctx, mut handle) = subsystem_test::make_subsystem_context(pool);
let (mut ctx, mut handle) = polkadot_subsystem::test_helpers::make_subsystem_context(pool);
let mut descriptor = CandidateDescriptor::default();
descriptor.pov_hash = pov_hash;
......@@ -700,7 +700,7 @@ mod tests {
};
let pool = sp_core::testing::SpawnBlockingExecutor::new();
let (mut ctx, mut handle) = subsystem_test::make_subsystem_context(pool);
let (mut ctx, mut handle) = polkadot_subsystem::test_helpers::make_subsystem_context(pool);
let mut descriptor = CandidateDescriptor::default();
descriptor.pov_hash = pov_hash;
......@@ -778,7 +778,7 @@ mod tests {
};
let pool = sp_core::testing::SpawnBlockingExecutor::new();
let (mut ctx, mut handle) = subsystem_test::make_subsystem_context(pool);
let (mut ctx, mut handle) = polkadot_subsystem::test_helpers::make_subsystem_context(pool);
executor::block_on(async move {
handle_network_update(
......@@ -850,7 +850,7 @@ mod tests {
};
let pool = sp_core::testing::SpawnBlockingExecutor::new();
let (mut ctx, mut handle) = subsystem_test::make_subsystem_context(pool);
let (mut ctx, mut handle) = polkadot_subsystem::test_helpers::make_subsystem_context(pool);
executor::block_on(async move {
// Peer A answers our request before peer B.
......@@ -938,7 +938,7 @@ mod tests {
};
let pool = sp_core::testing::SpawnBlockingExecutor::new();
let (mut ctx, mut handle) = subsystem_test::make_subsystem_context(pool);
let (mut ctx, mut handle) = polkadot_subsystem::test_helpers::make_subsystem_context(pool);
executor::block_on(async move {
// Peer A answers our request: right relay parent, awaited hash, wrong PoV.
......@@ -1001,7 +1001,7 @@ mod tests {
};
let pool = sp_core::testing::SpawnBlockingExecutor::new();
let (mut ctx, mut handle) = subsystem_test::make_subsystem_context(pool);
let (mut ctx, mut handle) = polkadot_subsystem::test_helpers::make_subsystem_context(pool);
executor::block_on(async move {
// Peer A answers our request: right relay parent, awaited hash, wrong PoV.
......@@ -1062,7 +1062,7 @@ mod tests {
};
let pool = sp_core::testing::SpawnBlockingExecutor::new();
let (mut ctx, mut handle) = subsystem_test::make_subsystem_context(pool);
let (mut ctx, mut handle) = polkadot_subsystem::test_helpers::make_subsystem_context(pool);
executor::block_on(async move {
// Peer A answers our request: right relay parent, awaited hash, wrong PoV.
......@@ -1120,7 +1120,7 @@ mod tests {
};
let pool = sp_core::testing::SpawnBlockingExecutor::new();
let (mut ctx, mut handle) = subsystem_test::make_subsystem_context(pool);
let (mut ctx, mut handle) = polkadot_subsystem::test_helpers::make_subsystem_context(pool);
executor::block_on(async move {
let max_plausibly_awaited = n_validators * 2;
......@@ -1205,7 +1205,7 @@ mod tests {
};
let pool = sp_core::testing::SpawnBlockingExecutor::new();
let (mut ctx, mut handle) = subsystem_test::make_subsystem_context(pool);
let (mut ctx, mut handle) = polkadot_subsystem::test_helpers::make_subsystem_context(pool);
executor::block_on(async move {
let pov_hash = make_pov(vec![1, 2, 3]).hash();
......@@ -1267,7 +1267,7 @@ mod tests {
};
let pool = sp_core::testing::SpawnBlockingExecutor::new();
let (mut ctx, mut handle) = subsystem_test::make_subsystem_context(pool);
let (mut ctx, mut handle) = polkadot_subsystem::test_helpers::make_subsystem_context(pool);
executor::block_on(async move {
let pov_hash = make_pov(vec![1, 2, 3]).hash();
......@@ -1344,7 +1344,7 @@ mod tests {
};
let pool = sp_core::testing::SpawnBlockingExecutor::new();
let (mut ctx, mut handle) = subsystem_test::make_subsystem_context(pool);
let (mut ctx, mut handle) = polkadot_subsystem::test_helpers::make_subsystem_context(pool);
executor::block_on(async move {
handle_network_update(
......@@ -1427,7 +1427,7 @@ mod tests {
};
let pool = sp_core::testing::SpawnBlockingExecutor::new();
let (mut ctx, mut handle) = subsystem_test::make_subsystem_context(pool);
let (mut ctx, mut handle) = polkadot_subsystem::test_helpers::make_subsystem_context(pool);
executor::block_on(async move {
handle_network_update(
......
......@@ -21,7 +21,7 @@ indexmap = "1.4.0"
[dev-dependencies]
parking_lot = "0.10.0"
subsystem-test = { package = "polkadot-subsystem-test-helpers", path = "../../test-helpers/subsystem" }
polkadot-subsystem = { package = "polkadot-node-subsystem", path = "../../subsystem", features = ["test-helpers"] }
assert_matches = "1.3.0"
sp-keyring = { git = "https://github.com/paritytech/substrate", branch = "master" }
sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" }
......@@ -1213,7 +1213,7 @@ mod tests {
};
let pool = sp_core::testing::SpawnBlockingExecutor::new();
let (mut ctx, mut handle) = subsystem_test::make_subsystem_context(pool);
let (mut ctx, mut handle) = polkadot_subsystem::test_helpers::make_subsystem_context(pool);
let peer = PeerId::random();
executor::block_on(async move {
......@@ -1305,7 +1305,7 @@ mod tests {
].into_iter().collect();
let pool = sp_core::testing::SpawnBlockingExecutor::new();
let (mut ctx, mut handle) = subsystem_test::make_subsystem_context(pool);
let (mut ctx, mut handle) = polkadot_subsystem::test_helpers::make_subsystem_context(pool);
executor::block_on(async move {
let statement = {
......
......@@ -13,6 +13,7 @@ futures-timer = "3.0.2"
keystore = { package = "sc-keystore", git = "https://github.com/paritytech/substrate", branch = "master" }
log = "0.4.8"
parity-scale-codec = "1.3.0"
parking_lot = { version = "0.10.0", optional = true }
pin-project = "0.4.22"
polkadot-node-primitives = { path = "../primitives" }
polkadot-primitives = { path = "../../primitives" }
......@@ -20,3 +21,12 @@ polkadot-statement-table = { path = "../../statement-table" }
sc-network = { git = "https://github.com/paritytech/substrate", branch = "master" }
sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" }
streamunordered = "0.5.1"
[dev-dependencies]
assert_matches = "1.3.0"
async-trait = "0.1"
futures = { version = "0.3.5", features = ["thread-pool"] }
parking_lot = "0.10.0"
[features]
test-helpers = [ "parking_lot" ]
......@@ -35,6 +35,8 @@ use crate::messages::AllMessages;
pub mod messages;
pub mod util;
#[cfg(any(test, feature = "test-helpers"))]
pub mod test_helpers;
/// Signals sent by an overseer to a subsystem.
#[derive(PartialEq, Clone, Debug)]
......@@ -71,7 +73,7 @@ pub enum FromOverseer<M> {
/// * Subsystems dying when they are not expected to
/// * Subsystems not dying when they are told to die
/// * etc.
#[derive(Debug)]
#[derive(Debug, PartialEq, Eq)]
pub struct SubsystemError;
impl From<mpsc::SendError> for SubsystemError {
......
......@@ -408,4 +408,10 @@ pub enum AllMessages {
AvailabilityStore(AvailabilityStoreMessage),
/// Message for the network bridge subsystem.
NetworkBridge(NetworkBridgeMessage),
/// Test message
///
/// This variant is only valid while testing, but makes the process of testing the
/// subsystem job manager much simpler.
#[cfg(test)]
Test(String),
}
......@@ -16,8 +16,8 @@
//! Utilities for testing subsystems.
use polkadot_subsystem::{SubsystemContext, FromOverseer, SubsystemResult, SubsystemError};
use polkadot_subsystem::messages::AllMessages;
use crate::{SubsystemContext, FromOverseer, SubsystemResult, SubsystemError};
use crate::messages::AllMessages;
use futures::prelude::*;
use futures::channel::mpsc;
......
This diff is collapsed.
[package]
name = "polkadot-subsystem-test-helpers"
version = "0.1.0"
authors = ["Parity Technologies <admin@parity.io>"]
edition = "2018"
description = "Helpers for testing subsystems"
[dependencies]
futures = "0.3.5"
async-trait = "0.1"
polkadot-subsystem = { package = "polkadot-node-subsystem", path = "../../subsystem" }
parking_lot = "0.10.0"
sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" }
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment