From d7047578e92ed3adb25326987e3f50e092e0ac1d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bastian=20K=C3=B6cher?= <bkchr@users.noreply.github.com>
Date: Mon, 7 Dec 2020 15:47:39 +0100
Subject: [PATCH] Fix tests on master (#2080)
Because of a bug in the test script, we didn't stopped CI when the main
tests are failed.
---
polkadot/node/core/backing/src/lib.rs | 48 ++++++++------
polkadot/node/core/provisioner/src/lib.rs | 6 +-
polkadot/node/core/provisioner/src/tests.rs | 65 +++++++++++--------
polkadot/node/overseer/src/lib.rs | 2 +-
.../node/test/service/tests/build-blocks.rs | 2 +-
polkadot/primitives/src/v1.rs | 5 ++
polkadot/scripts/gitlab/test_linux_stable.sh | 1 +
7 files changed, 78 insertions(+), 51 deletions(-)
diff --git a/polkadot/node/core/backing/src/lib.rs b/polkadot/node/core/backing/src/lib.rs
index 1d17827d0a3..54dbe295f7b 100644
--- a/polkadot/node/core/backing/src/lib.rs
+++ b/polkadot/node/core/backing/src/lib.rs
@@ -754,7 +754,7 @@ impl CandidateBackingJob {
self.background_validate_and_make_available(BackgroundValidationParams {
tx_from: self.tx_from.clone(),
tx_command: self.background_validation_tx.clone(),
- candidate: candidate,
+ candidate,
relay_parent: self.parent,
pov: None,
validator_index: self.table_context.validator.as_ref().map(|v| v.index()),
@@ -1644,28 +1644,38 @@ mod tests {
AllMessages::Provisioner(
ProvisionerMessage::ProvisionableData(
_,
- ProvisionableData::BackedCandidate(BackedCandidate {
- candidate,
- validity_votes,
- validator_indices,
+ ProvisionableData::BackedCandidate(CandidateReceipt {
+ descriptor,
+ ..
})
)
- ) if candidate == candidate_a => {
- assert_eq!(validity_votes.len(), 3);
-
- assert!(validity_votes.contains(
- &ValidityAttestation::Implicit(signed_a.signature().clone())
- ));
- assert!(validity_votes.contains(
- &ValidityAttestation::Explicit(signed_b.signature().clone())
- ));
- assert!(validity_votes.contains(
- &ValidityAttestation::Explicit(signed_c.signature().clone())
- ));
- assert_eq!(validator_indices, bitvec::bitvec![Lsb0, u8; 1, 0, 1, 1]);
- }
+ ) if descriptor == candidate_a.descriptor
+ );
+
+ let (tx, rx) = oneshot::channel();
+ let msg = CandidateBackingMessage::GetBackedCandidates(
+ test_state.relay_parent,
+ vec![candidate_a.hash()],
+ tx,
);
+ virtual_overseer.send(FromOverseer::Communication{ msg }).await;
+
+ let candidates = rx.await.unwrap();
+ assert_eq!(1, candidates.len());
+ assert_eq!(candidates[0].validity_votes.len(), 3);
+
+ assert!(candidates[0].validity_votes.contains(
+ &ValidityAttestation::Implicit(signed_a.signature().clone())
+ ));
+ assert!(candidates[0].validity_votes.contains(
+ &ValidityAttestation::Explicit(signed_b.signature().clone())
+ ));
+ assert!(candidates[0].validity_votes.contains(
+ &ValidityAttestation::Explicit(signed_c.signature().clone())
+ ));
+ assert_eq!(candidates[0].validator_indices, bitvec::bitvec![Lsb0, u8; 1, 0, 1, 1]);
+
virtual_overseer.send(FromOverseer::Signal(
OverseerSignal::ActiveLeaves(ActiveLeavesUpdate::stop_work(test_state.relay_parent)))
).await;
diff --git a/polkadot/node/core/provisioner/src/lib.rs b/polkadot/node/core/provisioner/src/lib.rs
index be3f38ba1ea..4e985a26220 100644
--- a/polkadot/node/core/provisioner/src/lib.rs
+++ b/polkadot/node/core/provisioner/src/lib.rs
@@ -17,7 +17,7 @@
//! The provisioner is responsible for assembling a relay chain block
//! from a set of available parachain candidates of its choice.
-#![deny(missing_docs, unused_crate_dependencies, unused_results)]
+#![deny(missing_docs, unused_crate_dependencies)]
use bitvec::vec::BitVec;
use futures::{
@@ -455,8 +455,8 @@ async fn select_candidates(
// maps to either 0 or 1 backed candidate, and the hashes correspond. Therefore, by checking them
// in order, we can ensure that the backed candidates are also in order.
let mut backed_idx = 0;
- for selected in selected_candidates.iter() {
- if *selected == candidates.get(backed_idx).ok_or(Error::BackedCandidateOrderingProblem)?.hash() {
+ for selected in selected_candidates {
+ if selected == candidates.get(backed_idx).ok_or(Error::BackedCandidateOrderingProblem)?.hash() {
backed_idx += 1;
}
}
diff --git a/polkadot/node/core/provisioner/src/tests.rs b/polkadot/node/core/provisioner/src/tests.rs
index 1cfe0cf29fc..cc3750591ee 100644
--- a/polkadot/node/core/provisioner/src/tests.rs
+++ b/polkadot/node/core/provisioner/src/tests.rs
@@ -192,13 +192,13 @@ mod select_availability_bitfields {
mod select_candidates {
use futures_timer::Delay;
use super::super::*;
- use super::{build_occupied_core, default_bitvec, occupied_core, scheduled_core};
+ use super::{build_occupied_core, occupied_core, scheduled_core, default_bitvec};
use polkadot_node_subsystem::messages::{
AllMessages, RuntimeApiMessage,
RuntimeApiRequest::{AvailabilityCores, PersistedValidationData as PersistedValidationDataReq},
};
use polkadot_primitives::v1::{
- BlockNumber, CandidateDescriptor, CommittedCandidateReceipt, PersistedValidationData,
+ BlockNumber, CandidateDescriptor, PersistedValidationData, CommittedCandidateReceipt, CandidateCommitments,
};
const BLOCK_UNDER_PRODUCTION: BlockNumber = 128;
@@ -297,7 +297,7 @@ mod select_candidates {
]
}
- async fn mock_overseer(mut receiver: mpsc::Receiver<FromJobCommand>) {
+ async fn mock_overseer(mut receiver: mpsc::Receiver<FromJobCommand>, expected: Vec<BackedCandidate>) {
use ChainApiMessage::BlockNumber;
use RuntimeApiMessage::Request;
@@ -313,8 +313,12 @@ mod select_candidates {
FromJobCommand::SendMessage(AllMessages::RuntimeApi(Request(_parent_hash, AvailabilityCores(tx)))) => {
tx.send(Ok(mock_availability_cores())).unwrap()
}
- // non-exhaustive matches are fine for testing
- _ => unimplemented!(),
+ FromJobCommand::SendMessage(
+ AllMessages::CandidateBacking(CandidateBackingMessage::GetBackedCandidates(_, _, sender))
+ ) => {
+ let _ = sender.send(expected.clone());
+ }
+ _ => panic!("Unexpected message: {:?}", from_job),
}
}
}
@@ -341,10 +345,8 @@ mod select_candidates {
#[test]
fn can_succeed() {
- test_harness(mock_overseer, |mut tx: mpsc::Sender<FromJobCommand>| async move {
- let result = select_candidates(&[], &[], &[], Default::default(), &mut tx).await;
- println!("{:?}", result);
- assert!(result.is_ok());
+ test_harness(|r| mock_overseer(r, Vec::new()), |mut tx: mpsc::Sender<FromJobCommand>| async move {
+ select_candidates(&[], &[], &[], Default::default(), &mut tx).await.unwrap();
})
}
@@ -358,23 +360,19 @@ mod select_candidates {
let empty_hash = PersistedValidationData::<BlockNumber>::default().hash();
- let candidate_template = BackedCandidate {
- candidate: CommittedCandidateReceipt {
- descriptor: CandidateDescriptor {
- persisted_validation_data_hash: empty_hash,
- ..Default::default()
- },
+ let candidate_template = CandidateReceipt {
+ descriptor: CandidateDescriptor {
+ persisted_validation_data_hash: empty_hash,
..Default::default()
},
- validity_votes: Vec::new(),
- validator_indices: default_bitvec(n_cores),
+ commitments_hash: CandidateCommitments::default().hash(),
};
let candidates: Vec<_> = std::iter::repeat(candidate_template)
.take(mock_cores.len())
.enumerate()
.map(|(idx, mut candidate)| {
- candidate.candidate.descriptor.para_id = idx.into();
+ candidate.descriptor.para_id = idx.into();
candidate
})
.cycle()
@@ -386,12 +384,12 @@ mod select_candidates {
candidate
} else if idx < mock_cores.len() * 2 {
// for the second repetition of the candidates, give them the wrong hash
- candidate.candidate.descriptor.persisted_validation_data_hash
+ candidate.descriptor.persisted_validation_data_hash
= Default::default();
candidate
} else {
// third go-around: right hash, wrong para_id
- candidate.candidate.descriptor.para_id = idx.into();
+ candidate.descriptor.para_id = idx.into();
candidate
}
})
@@ -403,15 +401,28 @@ mod select_candidates {
.map(|&idx| candidates[idx].clone())
.collect();
- test_harness(mock_overseer, |mut tx: mpsc::Sender<FromJobCommand>| async move {
+ let expected_backed = expected_candidates
+ .iter()
+ .map(|c| BackedCandidate {
+ candidate: CommittedCandidateReceipt { descriptor: c.descriptor.clone(), ..Default::default() },
+ validity_votes: Vec::new(),
+ validator_indices: default_bitvec(n_cores),
+ })
+ .collect();
+
+ test_harness(|r| mock_overseer(r, expected_backed), |mut tx: mpsc::Sender<FromJobCommand>| async move {
let result =
select_candidates(&mock_cores, &[], &candidates, Default::default(), &mut tx)
- .await;
-
- if result.is_err() {
- println!("{:?}", result);
- }
- assert_eq!(result.unwrap(), expected_candidates);
+ .await.unwrap();
+
+ result.into_iter()
+ .for_each(|c|
+ assert!(
+ expected_candidates.iter().any(|c2| c.candidate.corresponds_to(c2)),
+ "Failed to find candidate: {:?}",
+ c,
+ )
+ );
})
}
}
diff --git a/polkadot/node/overseer/src/lib.rs b/polkadot/node/overseer/src/lib.rs
index 661ef79b063..bbd9f626fe3 100644
--- a/polkadot/node/overseer/src/lib.rs
+++ b/polkadot/node/overseer/src/lib.rs
@@ -2165,7 +2165,7 @@ mod tests {
fn test_candidate_backing_msg() -> CandidateBackingMessage {
let (sender, _) = oneshot::channel();
- CandidateBackingMessage::GetBackedCandidates(Default::default(), sender)
+ CandidateBackingMessage::GetBackedCandidates(Default::default(), Vec::new(), sender)
}
fn test_candidate_selection_msg() -> CandidateSelectionMessage {
diff --git a/polkadot/node/test/service/tests/build-blocks.rs b/polkadot/node/test/service/tests/build-blocks.rs
index 777f266a03a..bad22e7f0ff 100644
--- a/polkadot/node/test/service/tests/build-blocks.rs
+++ b/polkadot/node/test/service/tests/build-blocks.rs
@@ -21,7 +21,7 @@ use sp_keyring::Sr25519Keyring;
#[substrate_test_utils::test]
async fn ensure_test_service_build_blocks(task_executor: TaskExecutor) {
- sc_cli::init_logger("", Default::default(), None).expect("Sets up logger");
+ sc_cli::init_logger("", Default::default(), None, false).expect("Sets up logger");
let mut alice = run_validator_node(
task_executor.clone(),
diff --git a/polkadot/primitives/src/v1.rs b/polkadot/primitives/src/v1.rs
index 1b469806840..3985254028c 100644
--- a/polkadot/primitives/src/v1.rs
+++ b/polkadot/primitives/src/v1.rs
@@ -231,6 +231,11 @@ impl<H: Clone> CommittedCandidateReceipt<H> {
pub fn hash(&self) -> CandidateHash where H: Encode {
self.to_plain().hash()
}
+
+ /// Does this committed candidate receipt corrensponds to the given [`CandidateReceipt`]?
+ pub fn corresponds_to(&self, receipt: &CandidateReceipt<H>) -> bool where H: PartialEq {
+ receipt.descriptor == self.descriptor && receipt.commitments_hash == self.commitments.hash()
+ }
}
impl PartialOrd for CommittedCandidateReceipt {
diff --git a/polkadot/scripts/gitlab/test_linux_stable.sh b/polkadot/scripts/gitlab/test_linux_stable.sh
index 17f5c80a72a..b841d8abecf 100755
--- a/polkadot/scripts/gitlab/test_linux_stable.sh
+++ b/polkadot/scripts/gitlab/test_linux_stable.sh
@@ -1,4 +1,5 @@
#!/usr/bin/env bash
+set -e
#shellcheck source=lib.sh
source "$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )/lib.sh"
--
GitLab