node/approval-voting: Continue to migrate tests to subsystem tests (#3471)

* node/approval-voting: Continue to migrate tests to subsystem tests

* node/approval-voting: Continue to implement subsystem tests.

* Add more tests

* node/approval-voting: Add more tests

* node/approval-voting: Difficuly determining the proper test scenario for threshold tests

* node/approval-voting: Introduce should_trigger_assignment tests

* node/approval-voting: Finalize threshold tests

* node/approval-voting: Address Feedback and add comments

* node/approval-voting: Tidy up approval tests

* Tidy up tests

* Fix rustfmt

node/core/approval-voting/src/lib.rs

@@ -92,9 +92,6 @@ use crate::{
 #[cfg(test)]
 mod tests;
 
-#[cfg(test)]
-mod old_tests;
-
 const APPROVAL_SESSIONS: SessionIndex = 6;
 const APPROVAL_CHECKING_TIMEOUT: Duration = Duration::from_secs(120);
 const APPROVAL_CACHE_SIZE: usize = 1024;
@@ -455,8 +452,9 @@ impl Wakeups {
 			Some(tick) => {
 				clock.wait(tick).await;
 				match self.wakeups.entry(tick) {
-					Entry::Vacant(_) =>
-						panic!("entry is known to exist since `first` was `Some`; qed"),
+					Entry::Vacant(_) => {
+						panic!("entry is known to exist since `first` was `Some`; qed")
+					},
 					Entry::Occupied(mut entry) => {
 						let (hash, candidate_hash) = entry.get_mut().pop()
 							.expect("empty entries are removed here and in `schedule`; no other mutation of this map; qed");
@@ -919,8 +917,9 @@ async fn handle_actions(
 				.await;
 
 				match confirmation_rx.await {
-					Err(oneshot::Canceled) =>
-						tracing::warn!(target: LOG_TARGET, "Dispute coordinator confirmation lost",),
+					Err(oneshot::Canceled) => {
+						tracing::warn!(target: LOG_TARGET, "Dispute coordinator confirmation lost",)
+					},
 					Ok(ImportStatementsResult::ValidImport) => {},
 					Ok(ImportStatementsResult::InvalidImport) => tracing::warn!(
 						target: LOG_TARGET,

node/core/approval-voting/src/persisted_entries.rs

@@ -217,12 +217,6 @@ impl ApprovalEntry {
 			(None, None)
 		}
 	}
-
-	/// For tests: set our assignment.
-	#[cfg(test)]
-	pub fn set_our_assignment(&mut self, our_assignment: OurAssignment) {
-		self.our_assignment = Some(our_assignment);
-	}
 }
 
 impl From<crate::approval_db::v1::ApprovalEntry> for ApprovalEntry {
@@ -294,11 +288,6 @@ impl CandidateEntry {
 	pub fn approval_entry(&self, block_hash: &Hash) -> Option<&ApprovalEntry> {
 		self.block_assignments.get(block_hash)
 	}
-
-	#[cfg(test)]
-	pub fn add_approval_entry(&mut self, block_hash: Hash, approval_entry: ApprovalEntry) {
-		self.block_assignments.insert(block_hash, approval_entry);
-	}
 }
 
 impl From<crate::approval_db::v1::CandidateEntry> for CandidateEntry {
@@ -384,26 +373,6 @@ impl BlockEntry {
 		})
 	}
 
-	/// For tests: Add a candidate to the block entry. Returns the
-	/// index where the candidate was added.
-	///
-	/// Panics if the core is already used.
-	#[cfg(test)]
-	pub fn add_candidate(&mut self, core: CoreIndex, candidate_hash: CandidateHash) -> usize {
-		let pos = self.candidates.binary_search_by_key(&core, |(c, _)| *c).unwrap_err();
-
-		self.candidates.insert(pos, (core, candidate_hash));
-
-		// bug in bitvec?
-		if pos < self.approved_bitfield.len() {
-			self.approved_bitfield.insert(pos, false);
-		} else {
-			self.approved_bitfield.push(false);
-		}
-
-		pos
-	}
-
 	/// Get the slot of the block.
 	pub fn slot(&self) -> Slot {
 		self.slot