Fix tests on master (#2080)

Because of a bug in the test script, we didn't stopped CI when the main
tests are failed.

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;

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;
 		}
 	}

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,
+					)
+				);
 		})
 	}
 }

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 {

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(),

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 {

scripts/gitlab/test_linux_stable.sh

 #!/usr/bin/env bash
+set -e
 
 #shellcheck source=lib.sh
 source "$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )/lib.sh"