From 82ab861132c5b05b3d57dcd7ec8d5f8d20ed907f Mon Sep 17 00:00:00 2001
From: ppom <reaction@ppom.me>
Date: Sun, 16 Feb 2025 12:00:00 +0100
Subject: [PATCH] Fix and enhance tests

The tokio task that removes the trigger after the last after-action was
not woken up & cancelled by the cancellation token, but left to die when
tokio quits.

Unfortunately this broke the tests, where tokio is kept from one
reaction launch to another. Those tasks kept a reference to the DB, then
preventing it to close, then preventing a new DB instance from being
opened.

This was difficult to debug due to the tests only checking that reaction
quits with an error, but not testing what kind of error it is.
So they now check that reaction quits with the "normal" error, which is
that all streams finished.

The tokio task is now woken up, like the other sleeping tasks, at
shutdown, and quits doing nothing.

It's not very satisfying because I assume it's a bit costly to wake up a
task, so I'm gonna see how I can improve this.

I may start a new tokio runtime for each subtest.
I may delete this task and move its instructions to the last
after-action.
I may do both.
---
 src/daemon/filter.rs | 21 +++++++++++++--------
 tests/simple.rs      | 14 ++++++++++----
 2 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/src/daemon/filter.rs b/src/daemon/filter.rs
index 689e410..269c8d2 100644
--- a/src/daemon/filter.rs
+++ b/src/daemon/filter.rs
@@ -218,15 +218,20 @@ impl FilterManager {
             let triggers = self.triggers.clone();
             let m = m.clone();
             let t = *t;
+            let shutdown = self.shutdown.clone();
             tokio::spawn(async move {
-                tokio::time::sleep(sleep_duration).await;
-                triggers
-                    .fetch_and_update_ext::<Match, BTreeSet<Time>, _>(&m, |set| {
-                        let mut set = set.unwrap_or_default();
-                        set.remove(&t);
-                        Some(set)
-                    })
-                    .unwrap();
+                if tokio::select! {
+                    _ = shutdown.wait() => false,
+                    _ = tokio::time::sleep(sleep_duration) => true
+                } {
+                    triggers
+                        .fetch_and_update_ext::<Match, BTreeSet<Time>, _>(&m, |set| {
+                            let mut set = set.unwrap_or_default();
+                            set.remove(&t);
+                            Some(set)
+                        })
+                        .unwrap();
+                }
             });
         }
     }
diff --git a/tests/simple.rs b/tests/simple.rs
index 77b870c..30ae52e 100644
--- a/tests/simple.rs
+++ b/tests/simple.rs
@@ -148,9 +148,12 @@ async fn simple() {
 
     file_with_contents(out_path, "");
 
-    assert!(daemon(config_path.into(), socket_path.into())
-        .await
-        .is_err());
+    let daemon_exit = daemon(config_path.into(), socket_path.into()).await;
+    assert!(daemon_exit.is_err());
+    assert_eq!(
+        daemon_exit.unwrap_err().to_string(),
+        "quitting because all streams finished"
+    );
 
     // 36 from DB
     // 12 from DB
@@ -183,8 +186,11 @@ async fn simple() {
     file_with_contents(out_path, "");
 
     let daemon_exit = daemon(config_path.into(), socket_path.into()).await;
-    eprintln!("daemon exit with {:?}", daemon_exit);
     assert!(daemon_exit.is_err());
+    assert_eq!(
+        daemon_exit.unwrap_err().to_string(),
+        "quitting because all streams finished"
+    );
 
     // make sure all numbers appear in the output
     assert_eq!(