Skip to content

Commit

Permalink
Avoid error log "ZeroCollators" when running cargo test (#394)
Browse files Browse the repository at this point in the history
  • Loading branch information
@tmpolaczyk
tmpolaczyk authored Jan 26, 2024
1 parent 496d044 commit 1465770
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 48 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ tempfile = "3.1.0"
thiserror = { version = "1.0.40" }
tokio = { version = "1.32.0", default-features = false }
tracing = { version = "0.1.37", default-features = false }
tracing-subscriber = { version = "0.2.25", default-features = false }
url = "2.2.2"

[patch.crates-io]
Expand Down
3 changes: 3 additions & 0 deletions pallets/collator-assignment/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ tp-traits = { workspace = true }

[dev-dependencies]
sp-io = { workspace = true }
tracing = { workspace = true }
tracing-subscriber = { workspace = true }

[features]
default = [ "std" ]
Expand All @@ -46,6 +48,7 @@ std = [
"sp-runtime/std",
"sp-std/std",
"tp-traits/std",
"tracing/std",
]
runtime-benchmarks = [
"frame-benchmarking/runtime-benchmarks",
Expand Down
54 changes: 50 additions & 4 deletions pallets/collator-assignment/src/mock.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@

use {
crate::{
self as pallet_collator_assignment, GetRandomnessForNextBlock,
RotateCollatorsEveryNSessions,
self as pallet_collator_assignment, pallet::CollatorContainerChain,
GetRandomnessForNextBlock, RotateCollatorsEveryNSessions,
},
frame_support::{
parameter_types,
Expand All @@ -30,7 +30,9 @@ use {
traits::{BlakeTwo256, IdentityLookup},
BuildStorage,
},
sp_std::collections::btree_map::BTreeMap,
tp_traits::{ParaId, RemoveInvulnerables, RemoveParaIdsWithNoCredits},
tracing_subscriber::{layer::SubscriberExt, FmtSubscriber},
};

type Block = frame_system::mocking::MockBlock<Test>;
Expand Down Expand Up @@ -225,10 +227,20 @@ impl pallet_collator_assignment::Config for Test {

// Build genesis storage according to the mock runtime.
pub fn new_test_ext() -> sp_io::TestExternalities {
system::GenesisConfig::<Test>::default()
let mut ext: sp_io::TestExternalities = system::GenesisConfig::<Test>::default()
.build_storage()
.unwrap()
.into()
.into();

ext.execute_with(|| {
MockData::mutate(|mocks| {
// Initialize collators with 1 collator to avoid error `ZeroCollators` in session 0
mocks.collators = vec![100];
mocks.min_orchestrator_chain_collators = 1;
})
});

ext
}

pub trait GetCollators<AccountId, SessionIndex> {
Expand Down Expand Up @@ -290,3 +302,37 @@ impl RemoveParaIdsWithNoCredits for RemoveParaIdsAbove5000 {
}
}
}

/// Returns a map of collator to assigned para id
pub fn assigned_collators() -> BTreeMap<u64, u32> {
let assigned_collators = CollatorContainerChain::<Test>::get();

let mut h = BTreeMap::new();

for (para_id, collators) in assigned_collators.container_chains.iter() {
for collator in collators.iter() {
h.insert(*collator, u32::from(*para_id));
}
}

for collator in assigned_collators.orchestrator_chain {
h.insert(collator, 1000);
}

h
}

/// Returns the default assignment for session 0 used in tests. Collator 100 is assigned to the orchestrator chain.
pub fn initial_collators() -> BTreeMap<u64, u32> {
BTreeMap::from_iter(vec![(100, 1000)])
}

/// Executes code without printing any logs. Can be used in tests where we expect logs to be printed, to avoid clogging
/// up stderr. Only affects the current thread, if `f` spawns any threads or if logs come from another thread, they will
/// not be silenced.
pub fn silence_logs<F: FnOnce() -> R, R>(f: F) -> R {
let no_logging_layer = tracing_subscriber::filter::LevelFilter::OFF;
let no_logging_subscriber = FmtSubscriber::builder().finish().with(no_logging_layer);

tracing::subscriber::with_default(no_logging_subscriber, f)
}
78 changes: 34 additions & 44 deletions pallets/collator-assignment/src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,24 +24,6 @@ mod assign_full;
mod prioritize_invulnerables;
mod select_chains;

fn assigned_collators() -> BTreeMap<u64, u32> {
let assigned_collators = CollatorContainerChain::<Test>::get();

let mut h = BTreeMap::new();

for (para_id, collators) in assigned_collators.container_chains.iter() {
for collator in collators.iter() {
h.insert(*collator, u32::from(*para_id));
}
}

for collator in assigned_collators.orchestrator_chain {
h.insert(collator, 1000);
}

h
}

#[test]
fn assign_initial_collators() {
new_test_ext().execute_with(|| {
Expand All @@ -57,10 +39,10 @@ fn assign_initial_collators() {
m.container_chains = vec![1001, 1002]
});

assert_eq!(assigned_collators(), BTreeMap::new(),);
assert_eq!(assigned_collators(), initial_collators(),);
run_to_block(6);

assert_eq!(assigned_collators(), BTreeMap::new(),);
assert_eq!(assigned_collators(), initial_collators(),);
run_to_block(11);

assert_eq!(
Expand Down Expand Up @@ -95,10 +77,10 @@ fn assign_collators_after_one_leaves_container() {
m.container_chains = vec![1001, 1002]
});

assert_eq!(assigned_collators(), BTreeMap::new(),);
assert_eq!(assigned_collators(), initial_collators(),);
run_to_block(6);

assert_eq!(assigned_collators(), BTreeMap::new(),);
assert_eq!(assigned_collators(), initial_collators(),);
run_to_block(11);

assert_eq!(
Expand Down Expand Up @@ -158,7 +140,7 @@ fn assign_collators_after_one_leaves_orchestrator_chain() {
m.container_chains = vec![1001, 1002]
});

assert_eq!(assigned_collators(), BTreeMap::new(),);
assert_eq!(assigned_collators(), initial_collators(),);
run_to_block(11);

assert_eq!(
Expand Down Expand Up @@ -215,7 +197,7 @@ fn assign_collators_if_config_orchestrator_chain_collators_increases() {
m.collators = vec![1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16];
m.container_chains = vec![1001, 1002]
});
assert_eq!(assigned_collators(), BTreeMap::new(),);
assert_eq!(assigned_collators(), initial_collators(),);
run_to_block(11);

assert_eq!(
Expand Down Expand Up @@ -275,7 +257,7 @@ fn assign_collators_if_config_orchestrator_chain_collators_decreases() {
m.collators = vec![1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16];
m.container_chains = vec![1001, 1002]
});
assert_eq!(assigned_collators(), BTreeMap::new(),);
assert_eq!(assigned_collators(), initial_collators(),);
run_to_block(11);

assert_eq!(
Expand Down Expand Up @@ -321,7 +303,7 @@ fn assign_collators_if_config_collators_per_container_increases() {
m.container_chains = vec![1001, 1002]
});

assert_eq!(assigned_collators(), BTreeMap::new(),);
assert_eq!(assigned_collators(), initial_collators(),);
run_to_block(11);

assert_eq!(
Expand Down Expand Up @@ -381,7 +363,7 @@ fn assign_collators_if_container_chain_is_removed() {
m.collators = vec![1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16];
m.container_chains = vec![1001, 1002]
});
assert_eq!(assigned_collators(), BTreeMap::new(),);
assert_eq!(assigned_collators(), initial_collators(),);
run_to_block(11);

assert_eq!(
Expand Down Expand Up @@ -435,7 +417,7 @@ fn assign_collators_if_container_chain_is_added() {
m.collators = vec![1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16];
m.container_chains = vec![1001, 1002]
});
assert_eq!(assigned_collators(), BTreeMap::new(),);
assert_eq!(assigned_collators(), initial_collators(),);
run_to_block(11);

assert_eq!(
Expand Down Expand Up @@ -493,7 +475,7 @@ fn assign_collators_after_decrease_num_collators() {
m.collators = vec![1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16];
m.container_chains = vec![1001, 1002]
});
assert_eq!(assigned_collators(), BTreeMap::new(),);
assert_eq!(assigned_collators(), initial_collators(),);
run_to_block(11);

let initial_assignment = BTreeMap::from_iter(vec![
Expand All @@ -513,7 +495,15 @@ fn assign_collators_after_decrease_num_collators() {
m.collators = vec![];
});

run_to_block(21);
// Disable logs in this test because it will print:
// Error in collator assignment, will keep previous assignment. ZeroCollators
// But only if this test runs after:
// test mock::__construct_runtime_integrity_test::runtime_integrity_tests ... ok
// Because that test enables logging
silence_logs(|| {
run_to_block(21);
});

// There are no collators but that would brick the chain, so we keep the old assignment
assert_eq!(assigned_collators(), initial_assignment);
});
Expand All @@ -533,7 +523,7 @@ fn assign_collators_stay_constant_if_new_collators_can_take_new_chains() {
m.collators = vec![1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16];
m.container_chains = vec![];
});
assert_eq!(assigned_collators(), BTreeMap::new(),);
assert_eq!(assigned_collators(), initial_collators(),);
run_to_block(11);

assert_eq!(
Expand Down Expand Up @@ -577,7 +567,7 @@ fn assign_collators_move_extra_container_chain_to_orchestrator_chain_if_not_enou
m.collators = vec![1, 2, 3, 4];
m.container_chains = vec![];
});
assert_eq!(assigned_collators(), BTreeMap::new(),);
assert_eq!(assigned_collators(), initial_collators(),);
run_to_block(11);

assert_eq!(
Expand Down Expand Up @@ -612,7 +602,7 @@ fn assign_collators_reorganize_container_chains_if_not_enough_collators() {
m.collators = vec![1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12];
m.container_chains = vec![1001, 1002, 1003, 1004, 1005];
});
assert_eq!(assigned_collators(), BTreeMap::new(),);
assert_eq!(assigned_collators(), initial_collators(),);
run_to_block(11);

assert_eq!(
Expand Down Expand Up @@ -669,7 +659,7 @@ fn assign_collators_set_zero_per_container() {
m.collators = vec![1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12];
m.container_chains = vec![1001, 1002, 1003, 1004];
});
assert_eq!(assigned_collators(), BTreeMap::new(),);
assert_eq!(assigned_collators(), initial_collators(),);
run_to_block(11);

assert_eq!(
Expand Down Expand Up @@ -718,7 +708,7 @@ fn assign_collators_rotation() {
m.collators = vec![1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12];
m.container_chains = vec![1001, 1002, 1003, 1004];
});
assert_eq!(assigned_collators(), BTreeMap::new(),);
assert_eq!(assigned_collators(), initial_collators(),);
run_to_block(11);

let initial_assignment = BTreeMap::from_iter(vec![
Expand Down Expand Up @@ -794,7 +784,7 @@ fn assign_collators_rotation_container_chains_are_shuffled() {
m.collators = vec![1, 2, 3, 4];
m.container_chains = vec![1001, 1002];
});
assert_eq!(assigned_collators(), BTreeMap::new(),);
assert_eq!(assigned_collators(), initial_collators(),);
run_to_block(11);

let initial_assignment =
Expand Down Expand Up @@ -833,7 +823,7 @@ fn assign_collators_rotation_parathreads_are_shuffled() {
m.collators = vec![1, 2, 3, 4];
m.parathreads = vec![5001, 5002];
});
assert_eq!(assigned_collators(), BTreeMap::new(),);
assert_eq!(assigned_collators(), initial_collators(),);
run_to_block(11);

let initial_assignment =
Expand Down Expand Up @@ -872,7 +862,7 @@ fn assign_collators_rotation_collators_are_shuffled() {
m.collators = vec![1, 2, 3, 4, 5, 6, 7, 8, 9, 10];
m.container_chains = vec![1001, 1002];
});
assert_eq!(assigned_collators(), BTreeMap::new(),);
assert_eq!(assigned_collators(), initial_collators(),);
run_to_block(11);

let initial_assignment = BTreeMap::from_iter(vec![
Expand Down Expand Up @@ -930,7 +920,7 @@ fn assign_collators_invulnerables_priority_orchestrator() {
m.collators = vec![1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 100];
m.container_chains = vec![1001, 1002];
});
assert_eq!(assigned_collators(), BTreeMap::new(),);
assert_eq!(assigned_collators(), initial_collators(),);
run_to_block(11);

let initial_assignment = BTreeMap::from_iter(vec![
Expand Down Expand Up @@ -967,7 +957,7 @@ fn assign_collators_invulnerables_priority_orchestrator_reassigned() {
m.collators = vec![1, 2, 3, 4, 5, 100, 101, 102, 103, 104];
m.container_chains = vec![1001, 1002];
});
assert_eq!(assigned_collators(), BTreeMap::new(),);
assert_eq!(assigned_collators(), initial_collators(),);
run_to_block(11);

let initial_assignment = BTreeMap::from_iter(vec![
Expand Down Expand Up @@ -1043,7 +1033,7 @@ fn assign_collators_all_invulnerables() {
m.collators = vec![101, 102, 103, 104, 105, 106, 107, 108, 109, 110];
m.container_chains = vec![1001, 1002];
});
assert_eq!(assigned_collators(), BTreeMap::new(),);
assert_eq!(assigned_collators(), initial_collators(),);
run_to_block(11);

let initial_assignment = BTreeMap::from_iter(vec![
Expand Down Expand Up @@ -1077,7 +1067,7 @@ fn rotation_events() {
m.collators = vec![1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12];
m.container_chains = vec![1001, 1002, 1003, 1004];
});
assert_eq!(assigned_collators(), BTreeMap::new(),);
assert_eq!(assigned_collators(), initial_collators(),);

// Block 1 should emit event, random seed was not set
System::assert_last_event(
Expand Down Expand Up @@ -1170,7 +1160,7 @@ fn assign_collators_remove_from_orchestator_when_all_assigned() {
m.collators = vec![1, 2];
m.container_chains = vec![1001];
});
assert_eq!(assigned_collators(), BTreeMap::new(),);
assert_eq!(assigned_collators(), initial_collators(),);
run_to_block(11);

let initial_assignment = BTreeMap::from_iter(vec![(1, 1000), (2, 1000)]);
Expand Down Expand Up @@ -1223,7 +1213,7 @@ fn collator_assignment_includes_empty_chains() {
m.container_chains = vec![2000, 2001, 2002];
m.parathreads = vec![3000, 3001, 3002]
});
assert_eq!(assigned_collators(), BTreeMap::new(),);
assert_eq!(assigned_collators(), initial_collators(),);
run_to_block(11);

let assigned_collators = CollatorContainerChain::<Test>::get();
Expand Down

0 comments on commit 1465770

Please sign in to comment.