fix bug and added more tests.

src/signing/collector/signing_finish_early_strategy.rs

@@ -34,4 +34,30 @@ impl SigningFinishEarlyStrategy {
             when_some_transaction_is_invalid,
         }
     }
+
+    pub fn r#continue() -> Self {
+        Self::new(
+            WhenAllTransactionsAreValid(SignaturesCollectingContinuation::Continue),
+            WhenSomeTransactionIsInvalid(SignaturesCollectingContinuation::Continue),
+        )
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    type Sut = SigningFinishEarlyStrategy;
+
+    #[test]
+    fn test_continue() {
+        let sut = Sut::r#continue();
+        assert_eq!(
+            sut.when_all_transactions_are_valid.0,
+            SignaturesCollectingContinuation::Continue
+        );
+        assert_eq!(
+            sut.when_some_transaction_is_invalid.0,
+            SignaturesCollectingContinuation::Continue
+        );
+    }
 }

src/signing/signatures_outcome_types/signatures_outcome.rs

@@ -56,6 +56,8 @@ impl SignaturesOutcome {
             "Discrepancy, found intent hash in both successful and failed transactions, this is a programmer error."
         );
 
+        assert!(failed_transactions.is_empty() || !neglected_factor_sources.is_empty(), "Discrepancy, found failed transactions but no neglected factor sources, this is a programmer error.");
+
         Self {
             successful_transactions,
             failed_transactions,
@@ -83,13 +85,33 @@ impl SignaturesOutcome {
         self.neglected_factor_sources.clone()
     }
 
-    pub fn ids_of_neglected_factor_sources(&self) -> IndexSet<FactorSourceIDFromHash> {
+    fn ids_of_neglected_factor_sources_filter(
+        &self,
+        filter: fn(&NeglectedFactor) -> bool,
+    ) -> IndexSet<FactorSourceIDFromHash> {
         self.neglected_factor_sources()
             .into_iter()
+            .filter(filter)
             .map(|n| n.factor_source_id())
             .collect()
     }
 
+    pub fn ids_of_neglected_factor_sources(&self) -> IndexSet<FactorSourceIDFromHash> {
+        self.ids_of_neglected_factor_sources_filter(|_| true)
+    }
+
+    pub fn ids_of_neglected_factor_sources_skipped_by_user(
+        &self,
+    ) -> IndexSet<FactorSourceIDFromHash> {
+        self.ids_of_neglected_factor_sources_filter(|nf| {
+            nf.reason == NeglectFactorReason::UserExplicitlySkipped
+        })
+    }
+
+    pub fn ids_of_neglected_factor_sources_failed(&self) -> IndexSet<FactorSourceIDFromHash> {
+        self.ids_of_neglected_factor_sources_filter(|nf| nf.reason == NeglectFactorReason::Failure)
+    }
+
     pub fn signatures_of_failed_transactions(&self) -> IndexSet<HDSignature> {
         self.failed_transactions.all_signatures()
     }
@@ -120,4 +142,16 @@ mod tests {
             [],
         );
     }
+
+    #[test]
+    #[should_panic(
+        expected = "Discrepancy, found failed transactions but no neglected factor sources, this is a programmer error."
+    )]
+    fn new_panics_if_failed_tx_is_not_empty_but_neglected_is() {
+        Sut::new(
+            MaybeSignedTransactions::empty(),
+            MaybeSignedTransactions::sample(),
+            [],
+        );
+    }
 }

src/testing/signing/test_interactors/test_parallel_interactor.rs

@@ -27,7 +27,7 @@ impl SignWithFactorParallelInteractor for TestSigningParallelInteractor {
             .collect::<IndexSet<_>>();
 
         if self.should_simulate_failure(ids.clone()) {
-            return SignWithFactorsOutcome::user_skipped_factors(ids);
+            return SignWithFactorsOutcome::failure_with_factors(ids);
         }
 
         match self

tests/main.rs

@@ -738,6 +738,7 @@ mod signing_tests {
 
             #[actix_rt::test]
             async fn many_failing_tx() {
+                sensible_env_logger::safe_init!();
                 let factor_sources = &HDFactorSource::all();
                 let a0 = &Account::a0();
                 let p3 = &Persona::p3();
@@ -778,6 +779,18 @@ mod signing_tests {
                         .collect_vec()
                 );
 
+                assert_eq!(
+                    outcome
+                        .ids_of_neglected_factor_sources_failed()
+                        .into_iter()
+                        .collect_vec(),
+                    vec![FactorSourceIDFromHash::fs0()]
+                );
+
+                assert!(outcome
+                    .ids_of_neglected_factor_sources_skipped_by_user()
+                    .is_empty());
+
                 assert_eq!(
                     outcome
                         .successful_transactions()
@@ -1226,6 +1239,40 @@ mod signing_tests {
                 );
                 let outcome = collector.collect_signatures().await;
                 assert!(!outcome.successful());
+                assert_eq!(
+                    outcome
+                        .ids_of_neglected_factor_sources_failed()
+                        .into_iter()
+                        .collect_vec(),
+                    vec![FactorSourceIDFromHash::fs0()]
+                );
+                assert!(outcome
+                    .ids_of_neglected_factor_sources_skipped_by_user()
+                    .is_empty())
+            }
+
+            async fn failure_e5<E: IsEntity>() {
+                let collector = SignaturesCollector::new_test(
+                    SigningFinishEarlyStrategy::r#continue(),
+                    HDFactorSource::all(),
+                    [TXToSign::new([E::e5()])],
+                    SimulatedUser::prudent_with_failures(
+                        SimulatedFailures::with_simulated_failures([FactorSourceIDFromHash::fs4()]),
+                    ),
+                );
+
+                let outcome = collector.collect_signatures().await;
+                assert!(outcome.successful());
+                assert_eq!(
+                    outcome
+                        .ids_of_neglected_factor_sources_failed()
+                        .into_iter()
+                        .collect_vec(),
+                    vec![FactorSourceIDFromHash::fs4()]
+                );
+                assert!(outcome
+                    .ids_of_neglected_factor_sources_skipped_by_user()
+                    .is_empty());
             }
 
             async fn building_can_succeed_even_if_one_factor_source_fails_assert_ids_of_successful_tx_e4<
@@ -1422,10 +1469,15 @@ mod signing_tests {
                 }
 
                 #[actix_rt::test]
-                async fn failure() {
+                async fn failure_a0() {
                     failure_e0::<E>().await
                 }
 
+                #[actix_rt::test]
+                async fn failure_a5() {
+                    failure_e5::<E>().await
+                }
+
                 #[actix_rt::test]
                 async fn building_can_succeed_even_if_one_factor_source_fails_assert_ids_of_successful_tx(
                 ) {
@@ -1597,10 +1649,15 @@ mod signing_tests {
                 }
 
                 #[actix_rt::test]
-                async fn failure() {
+                async fn failure_p0() {
                     failure_e0::<E>().await
                 }
 
+                #[actix_rt::test]
+                async fn failure_p5() {
+                    failure_e5::<E>().await
+                }
+
                 #[actix_rt::test]
                 async fn building_can_succeed_even_if_one_factor_source_fails_assert_ids_of_successful_tx(
                 ) {