test: adjustments following pr reviews

Author: Alenar

mithril-signer/src/database/query/signed_beacon/delete_signed_beacon.rs

@@ -48,52 +48,70 @@ mod tests {
 
     use super::*;
 
+    // Epoch of the signed entities is irrelevant for those tests, only SignedBeacon.epoch matter
+    const WHATEVER_EPOCH: Epoch = Epoch(378);
+
     #[test]
-    fn test_prune_below_epoch_threshold() {
+    fn test_delete_nothing_if_nothing_strictly_below_epoch_threshold() {
+        let connection = main_db_connection().unwrap();
+        insert_signed_beacons(
+            &connection,
+            SignedBeaconRecord::fakes(&[(
+                Epoch(7),
+                vec![SignedEntityType::MithrilStakeDistribution(WHATEVER_EPOCH)],
+            )]),
+        );
+
+        let delete_cursor = connection
+            .fetch(DeleteSignedBeaconRecordQuery::below_epoch_threshold(Epoch(
+                7,
+            )))
+            .unwrap();
+        assert_eq!(0, delete_cursor.count());
+
+        let get_all_cursor = connection.fetch(GetSignedBeaconQuery::all()).unwrap();
+        assert_eq!(1, get_all_cursor.count());
+    }
+
+    #[test]
+    fn test_delete_below_epoch_threshold() {
         let connection = main_db_connection().unwrap();
         insert_signed_beacons(
             &connection,
             SignedBeaconRecord::fakes(&[
                 (
                     Epoch(7),
                     vec![
-                        SignedEntityType::MithrilStakeDistribution(Epoch(7)),
-                        SignedEntityType::CardanoTransactions(Epoch(7), BlockNumber(12)),
+                        SignedEntityType::MithrilStakeDistribution(WHATEVER_EPOCH),
+                        SignedEntityType::CardanoTransactions(WHATEVER_EPOCH, BlockNumber(12)),
                     ],
                 ),
                 (
                     Epoch(8),
                     vec![
-                        SignedEntityType::MithrilStakeDistribution(Epoch(8)),
-                        SignedEntityType::CardanoStakeDistribution(Epoch(8)),
+                        SignedEntityType::MithrilStakeDistribution(WHATEVER_EPOCH),
+                        SignedEntityType::CardanoStakeDistribution(WHATEVER_EPOCH),
                     ],
                 ),
                 (
                     Epoch(9),
                     vec![
-                        SignedEntityType::MithrilStakeDistribution(Epoch(9)),
-                        SignedEntityType::CardanoStakeDistribution(Epoch(9)),
-                        SignedEntityType::CardanoTransactions(Epoch(9), BlockNumber(23)),
+                        SignedEntityType::MithrilStakeDistribution(WHATEVER_EPOCH),
+                        SignedEntityType::CardanoStakeDistribution(WHATEVER_EPOCH),
+                        SignedEntityType::CardanoTransactions(WHATEVER_EPOCH, BlockNumber(23)),
                     ],
                 ),
             ]),
         );
 
-        let cursor = connection
-            .fetch(DeleteSignedBeaconRecordQuery::below_epoch_threshold(Epoch(
-                7,
-            )))
-            .unwrap();
-        assert_eq!(0, cursor.count());
-
-        let cursor = connection
+        let delete_cursor = connection
             .fetch(DeleteSignedBeaconRecordQuery::below_epoch_threshold(Epoch(
                 9,
             )))
             .unwrap();
-        assert_eq!(4, cursor.count());
+        assert_eq!(4, delete_cursor.count());
 
-        let cursor = connection.fetch(GetSignedBeaconQuery::all()).unwrap();
-        assert_eq!(3, cursor.count());
+        let get_all_cursor = connection.fetch(GetSignedBeaconQuery::all()).unwrap();
+        assert_eq!(3, get_all_cursor.count());
     }
 }

mithril-signer/src/database/query/signed_beacon/get_signed_beacon.rs

@@ -146,11 +146,11 @@ mod tests {
             let connection = main_db_connection().unwrap();
             let records = SignedBeaconRecord::fakes(&[
                 (
-                    Epoch(3),
+                    Epoch(974),
                     vec![SignedEntityType::MithrilStakeDistribution(Epoch(3))],
                 ),
                 (
-                    Epoch(4),
+                    Epoch(975),
                     vec![SignedEntityType::CardanoStakeDistribution(Epoch(4))],
                 ),
             ]);
@@ -167,34 +167,34 @@ mod tests {
 
             assert_eq!(
                 vec![SignedBeaconRecord::fake(
-                    Epoch(3),
+                    Epoch(974),
                     SignedEntityType::MithrilStakeDistribution(Epoch(3))
                 )],
                 stored_records
             );
         }
 
         #[test]
-        fn with_multiple_matchings_over_several_epochs() {
+        fn with_multiple_matching_over_several_epochs() {
             let connection = main_db_connection().unwrap();
             let records = SignedBeaconRecord::fakes(&[
                 (
-                    Epoch(3),
+                    Epoch(329),
                     vec![
                         SignedEntityType::MithrilStakeDistribution(Epoch(3)),
                         SignedEntityType::CardanoStakeDistribution(Epoch(3)),
                     ],
                 ),
                 (
-                    Epoch(4),
+                    Epoch(330),
                     vec![
                         SignedEntityType::CardanoStakeDistribution(Epoch(4)),
                         SignedEntityType::MithrilStakeDistribution(Epoch(4)),
                         SignedEntityType::CardanoTransactions(Epoch(4), BlockNumber(109)),
                     ],
                 ),
                 (
-                    Epoch(5),
+                    Epoch(331),
                     vec![
                         SignedEntityType::CardanoTransactions(Epoch(5), BlockNumber(124)),
                         SignedEntityType::CardanoTransactions(Epoch(5), BlockNumber(133)),
@@ -218,26 +218,54 @@ mod tests {
             assert_eq!(
                 SignedBeaconRecord::fakes(&[
                     (
-                        Epoch(5),
+                        Epoch(331),
                         vec![SignedEntityType::CardanoTransactions(
                             Epoch(5),
                             BlockNumber(133)
                         ),],
                     ),
                     (
-                        Epoch(4),
+                        Epoch(330),
                         vec![
                             SignedEntityType::CardanoTransactions(Epoch(4), BlockNumber(109)),
                             SignedEntityType::MithrilStakeDistribution(Epoch(4)),
                         ],
                     ),
                     (
-                        Epoch(3),
+                        Epoch(329),
                         vec![SignedEntityType::MithrilStakeDistribution(Epoch(3)),],
                     ),
                 ]),
                 stored_records
             );
         }
+
+        #[test]
+        fn duplicate_entities_in_parameter_should_not_yield_duplicate_matching() {
+            let connection = main_db_connection().unwrap();
+            let records = SignedBeaconRecord::fakes(&[(
+                Epoch(242),
+                vec![SignedEntityType::MithrilStakeDistribution(Epoch(3))],
+            )]);
+            insert_signed_beacons(&connection, records.clone());
+
+            let stored_records: Vec<SignedBeaconRecord> = connection
+                .fetch_collect(
+                    GetSignedBeaconQuery::by_signed_entities(&[
+                        SignedEntityType::MithrilStakeDistribution(Epoch(3)),
+                        SignedEntityType::MithrilStakeDistribution(Epoch(3)),
+                    ])
+                    .unwrap(),
+                )
+                .unwrap();
+
+            assert_eq!(
+                vec![SignedBeaconRecord::fake(
+                    Epoch(242),
+                    SignedEntityType::MithrilStakeDistribution(Epoch(3))
+                )],
+                stored_records
+            );
+        }
     }
 }

mithril-signer/src/database/record/signed_beacon_record.rs

@@ -148,7 +148,24 @@ mod tests {
             signed_at: initiated_at + chrono::TimeDelta::minutes(2),
         };
 
+        // Check `impl PartialEq<BeaconToSign> for SignedBeaconRecord`
         assert_eq!(signed_beacon, beacon_to_sign);
+        assert_ne!(
+            signed_beacon,
+            BeaconToSign {
+                epoch: beacon_to_sign.epoch + 13,
+                ..beacon_to_sign.clone()
+            }
+        );
+
+        // Check `impl PartialEq<SignedBeaconRecord> for BeaconToSign`
         assert_eq!(beacon_to_sign, signed_beacon);
+        assert_ne!(
+            beacon_to_sign,
+            SignedBeaconRecord {
+                epoch: signed_beacon.epoch + 11,
+                ..signed_beacon.clone()
+            }
+        );
     }
 }

mithril-signer/src/database/repository/signed_beacon_repository.rs

@@ -233,18 +233,26 @@ mod tests {
                 .collect(),
         );
 
-        let to_filter = all_signed_entity_type_for(&time_point);
         let available_entities = repository
-            .filter_out_already_signed_entities(to_filter.clone())
+            .filter_out_already_signed_entities(vec![
+                SignedEntityType::MithrilStakeDistribution(time_point.epoch),
+                SignedEntityType::CardanoStakeDistribution(time_point.epoch),
+                SignedEntityType::CardanoTransactions(
+                    time_point.epoch,
+                    time_point.chain_point.block_number,
+                ),
+                SignedEntityType::CardanoStakeDistribution(time_point.epoch + 10),
+            ])
             .await
             .unwrap();
 
-        let expected: Vec<SignedEntityType> = to_filter
-            .iter()
-            .filter(|entity| !signed_beacons.contains(entity))
-            .cloned()
-            .collect();
-        assert_eq!(expected, available_entities);
+        assert_eq!(
+            vec![
+                SignedEntityType::CardanoStakeDistribution(time_point.epoch),
+                SignedEntityType::CardanoStakeDistribution(time_point.epoch + 10),
+            ],
+            available_entities
+        );
     }
 
     #[tokio::test]

mithril-signer/src/services/certifier.rs

@@ -163,8 +163,13 @@ mod tests {
         );
 
         let beacon_to_sign = certifier_service.get_beacon_to_sign().await.unwrap();
+        let signed_discriminant: Option<SignedEntityTypeDiscriminants> =
+            beacon_to_sign.map(|b| b.signed_entity_type.into());
 
-        assert!(beacon_to_sign.is_some());
+        assert_eq!(
+            SignedEntityTypeDiscriminants::all().first().cloned(),
+            signed_discriminant
+        );
     }
 
     #[tokio::test]
@@ -231,8 +236,7 @@ mod tests {
     }
 
     #[tokio::test]
-    async fn draining_out_all_beacon_to_signs_dont_repeat_value_and_use_signed_entity_discriminant_order(
-    ) {
+    async fn draining_out_all_beacons_to_sign_use_signed_entity_discriminant_order() {
         let ticker_service = DumbTickerService::new(TimePoint::new(1, 14, ChainPoint::dummy()));
         let certifier_service = SignerCertifierService::new(
             Arc::new(ticker_service),
@@ -249,7 +253,6 @@ mod tests {
             .await
             .unwrap()
             .expect("There should be a beacon to sign since nothing is locked or signed");
-        let mut all_signed_beacons = vec![previous_beacon_to_sign.clone()];
 
         loop {
             certifier_service
@@ -259,25 +262,54 @@ mod tests {
             let next_beacon_to_sign = certifier_service.get_beacon_to_sign().await.unwrap();
 
             if let Some(beacon) = next_beacon_to_sign {
-                assert!(
-                    !all_signed_beacons.contains(&beacon),
-                    "Beacon should not repeat"
-                );
                 assert!(
                     SignedEntityTypeDiscriminants::from(
                         &previous_beacon_to_sign.signed_entity_type
                     ) < SignedEntityTypeDiscriminants::from(&beacon.signed_entity_type),
                     "Beacon should follow SignedEntityTypeDiscriminants order"
                 );
 
-                all_signed_beacons.push(beacon.clone());
                 previous_beacon_to_sign = beacon;
             } else {
                 break;
             }
         }
     }
 
+    #[tokio::test]
+    async fn draining_out_all_beacons_to_sign_doesnt_repeat_value() {
+        let ticker_service = DumbTickerService::new(TimePoint::new(1, 14, ChainPoint::dummy()));
+        let certifier_service = SignerCertifierService::new(
+            Arc::new(ticker_service),
+            Arc::new(DumbSignedBeaconStore::default()),
+            Arc::new(DumbSignedEntityConfigProvider::new(
+                CardanoTransactionsSigningConfig::dummy(),
+                SignedEntityTypeDiscriminants::all(),
+            )),
+            Arc::new(SignedEntityTypeLock::new()),
+        );
+
+        let mut all_signed_beacons = vec![];
+        while let Some(beacon_to_sign) = certifier_service.get_beacon_to_sign().await.unwrap() {
+            certifier_service
+                .mark_beacon_as_signed(&beacon_to_sign)
+                .await
+                .unwrap();
+            all_signed_beacons.push(beacon_to_sign);
+        }
+
+        let mut dedup_signed_beacons = all_signed_beacons.clone();
+        dedup_signed_beacons.dedup();
+        assert!(
+            !all_signed_beacons.is_empty(),
+            "There should be at least one beacon to sign"
+        );
+        assert_eq!(
+            all_signed_beacons, dedup_signed_beacons,
+            "Beacon should not repeat"
+        );
+    }
+
     pub mod tests_tooling {
         use std::collections::BTreeSet;
         use tokio::sync::RwLock;