Auto merge of #236 - lennart/ALL-8-scuffle-signal, r=TroyKomodo

scuffle-signal: test coverage and docs - Improve test coverage - Improve docs by adding examples Requested-by: lennartkloock <[email protected]> Reviewed-by: TroyKomodo <[email protected]>
ScuffleCloud · Jan 7, 2025 · 6d886bf · 6d886bf
2 parents a958b8b + c44f941
commit 6d886bf
Show file tree

Hide file tree

Showing 6 changed files with 400 additions and 39 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/crates/context/src/lib.rs b/crates/context/src/lib.rs
@@ -309,11 +309,11 @@ mod tests {
     #[tokio::test]
     async fn new() {
         let (ctx, handler) = Context::new();
-        assert_eq!(handler.is_done(), false);
-        assert_eq!(ctx.is_done(), false);
+        assert!(!handler.is_done());
+        assert!(!ctx.is_done());
 
         let handler = Handler::default();
-        assert_eq!(handler.is_done(), false);
+        assert!(!handler.is_done());
     }
 
     #[tokio::test]
@@ -322,54 +322,54 @@ mod tests {
         let (child_ctx, child_handler) = ctx.new_child();
         let child_ctx2 = ctx.clone();
 
-        assert_eq!(handler.is_done(), false);
-        assert_eq!(ctx.is_done(), false);
-        assert_eq!(child_handler.is_done(), false);
-        assert_eq!(child_ctx.is_done(), false);
-        assert_eq!(child_ctx2.is_done(), false);
+        assert!(!handler.is_done());
+        assert!(!ctx.is_done());
+        assert!(!child_handler.is_done());
+        assert!(!child_ctx.is_done());
+        assert!(!child_ctx2.is_done());
 
         handler.cancel();
 
-        assert_eq!(handler.is_done(), true);
-        assert_eq!(ctx.is_done(), true);
-        assert_eq!(child_handler.is_done(), true);
-        assert_eq!(child_ctx.is_done(), true);
-        assert_eq!(child_ctx2.is_done(), true);
+        assert!(handler.is_done());
+        assert!(ctx.is_done());
+        assert!(child_handler.is_done());
+        assert!(child_ctx.is_done());
+        assert!(child_ctx2.is_done());
     }
 
     #[tokio::test]
     async fn cancel_child() {
         let (ctx, handler) = Context::new();
         let (child_ctx, child_handler) = ctx.new_child();
 
-        assert_eq!(handler.is_done(), false);
-        assert_eq!(ctx.is_done(), false);
-        assert_eq!(child_handler.is_done(), false);
-        assert_eq!(child_ctx.is_done(), false);
+        assert!(!handler.is_done());
+        assert!(!ctx.is_done());
+        assert!(!child_handler.is_done());
+        assert!(!child_ctx.is_done());
 
         child_handler.cancel();
 
-        assert_eq!(handler.is_done(), false);
-        assert_eq!(ctx.is_done(), false);
-        assert_eq!(child_handler.is_done(), true);
-        assert_eq!(child_ctx.is_done(), true);
+        assert!(!handler.is_done());
+        assert!(!ctx.is_done());
+        assert!(child_handler.is_done());
+        assert!(child_ctx.is_done());
     }
 
     #[tokio::test]
     async fn shutdown() {
         let (ctx, handler) = Context::new();
 
-        assert_eq!(handler.is_done(), false);
-        assert_eq!(ctx.is_done(), false);
+        assert!(!handler.is_done());
+        assert!(!ctx.is_done());
 
         // This is expected to timeout
         assert!(handler
             .shutdown()
             .with_timeout(std::time::Duration::from_millis(200))
             .await
             .is_err());
-        assert_eq!(handler.is_done(), true);
-        assert_eq!(ctx.is_done(), true);
+        assert!(handler.is_done());
+        assert!(ctx.is_done());
         assert!(ctx
             .into_done()
             .with_timeout(std::time::Duration::from_millis(200))
@@ -391,23 +391,23 @@ mod tests {
             .with_timeout(std::time::Duration::from_millis(200))
             .await
             .is_ok());
-        assert_eq!(handler.is_done(), true);
+        assert!(handler.is_done());
     }
 
     #[tokio::test]
     async fn global_handler() {
         let handler = Handler::global();
 
-        assert_eq!(handler.is_done(), false);
+        assert!(!handler.is_done());
 
         handler.cancel();
 
-        assert_eq!(handler.is_done(), true);
-        assert_eq!(Handler::global().is_done(), true);
-        assert_eq!(Context::global().is_done(), true);
+        assert!(handler.is_done());
+        assert!(Handler::global().is_done());
+        assert!(Context::global().is_done());
 
         let (child_ctx, child_handler) = Handler::global().new_child();
-        assert_eq!(child_handler.is_done(), true);
-        assert_eq!(child_ctx.is_done(), true);
+        assert!(child_handler.is_done());
+        assert!(child_ctx.is_done());
     }
 }
diff --git a/crates/signal/Cargo.toml b/crates/signal/Cargo.toml
@@ -10,6 +10,9 @@ license = "MIT OR Apache-2.0"
 description = "Ergonomic async signal handling."
 keywords = ["signal", "async"]
 
+[lints.rust]
+unexpected_cfgs = { level = "warn", check-cfg = ['cfg(coverage_nightly)'] }
+
 [dependencies]
 tokio = { version = "1", default-features = false, features = ["signal"] }
 scuffle-bootstrap = { workspace = true, optional = true }
@@ -19,6 +22,7 @@ scuffle-workspace-hack.workspace = true
 
 [dev-dependencies]
 tokio = { version = "1.41.1", features = ["macros", "rt", "time"] }
+tokio-test = "0.4.4"
 libc = "0.2"
 futures = "0.3"
 scuffle-future-ext = { path = "../future-ext" }

diff --git a/crates/signal/README.md b/crates/signal/README.md
@@ -14,6 +14,34 @@ A crate designed to provide a more user friendly interface to `tokio::signal`.
 The `tokio::signal` module provides a way for us to wait for a signal to be received in a non-blocking way.
 This crate extends that with a more helpful interface allowing the ability to listen to multiple signals concurrently.
 
+## Example
+
+```rust
+use scuffle_signal::SignalHandler;
+use tokio::signal::unix::SignalKind;
+
+let mut handler = SignalHandler::new()
+    .with_signal(SignalKind::interrupt())
+    .with_signal(SignalKind::terminate());
+
+// Wait for a signal to be received
+let signal = handler.await;
+
+// Handle the signal
+let user_defined1 = SignalKind::interrupt();
+let terminate = SignalKind::terminate();
+match signal {
+    interrupt => {
+        // Handle SIGINT
+        println!("received SIGINT");
+    },
+    terminate => {
+        // Handle SIGTERM
+        println!("received SIGTERM");
+    },
+}
+```
+
 ## Status
 
 This crate is currently under development and is not yet stable.

diff --git a/crates/signal/src/bootstrap.rs b/crates/signal/src/bootstrap.rs
@@ -73,3 +73,201 @@ impl<Global: SignalConfig> Service<Global> for SignalSvc {
         Ok(())
     }
 }
+
+#[cfg_attr(all(coverage_nightly, test), coverage(off))]
+#[cfg(test)]
+mod tests {
+    use std::sync::Arc;
+
+    use scuffle_bootstrap::global::GlobalWithoutConfig;
+    use scuffle_bootstrap::Service;
+    use scuffle_future_ext::FutureExt;
+    use tokio::signal::unix::SignalKind;
+
+    use super::{SignalConfig, SignalSvc};
+    use crate::tests::raise_signal;
+    use crate::SignalHandler;
+
+    async fn force_shutdown_two_signals<Global: GlobalWithoutConfig + SignalConfig>() {
+        let (ctx, handler) = scuffle_context::Context::new();
+
+        // Block the global context
+        let _global_ctx = scuffle_context::Context::global();
+
+        let svc = SignalSvc;
+        let global = <Global as GlobalWithoutConfig>::init().await.unwrap();
+
+        assert!(svc.enabled(&global).await.unwrap());
+        let result = tokio::spawn(svc.run(global, ctx));
+
+        // Wait for the service to start
+        tokio::time::sleep(tokio::time::Duration::from_millis(10)).await;
+
+        raise_signal(tokio::signal::unix::SignalKind::interrupt());
+        tokio::time::sleep(tokio::time::Duration::from_millis(10)).await;
+        raise_signal(tokio::signal::unix::SignalKind::interrupt());
+
+        match result.with_timeout(tokio::time::Duration::from_millis(100)).await {
+            Ok(Ok(Err(e))) => {
+                assert_eq!(e.to_string(), "received signal, shutting down immediately: SignalKind(2)");
+            }
+            _ => panic!("unexpected result"),
+        }
+
+        assert!(handler
+            .shutdown()
+            .with_timeout(tokio::time::Duration::from_millis(100))
+            .await
+            .is_ok());
+    }
+
+    struct TestGlobal;
+
+    impl GlobalWithoutConfig for TestGlobal {
+        fn init() -> impl std::future::Future<Output = anyhow::Result<Arc<Self>>> + Send {
+            std::future::ready(Ok(Arc::new(Self)))
+        }
+    }
+
+    impl SignalConfig for TestGlobal {}
+
+    #[tokio::test]
+    async fn default_bootstrap_service() {
+        force_shutdown_two_signals::<TestGlobal>().await;
+    }
+    struct NoTimeoutTestGlobal;
+
+    impl GlobalWithoutConfig for NoTimeoutTestGlobal {
+        fn init() -> impl std::future::Future<Output = anyhow::Result<Arc<Self>>> + Send {
+            std::future::ready(Ok(Arc::new(Self)))
+        }
+    }
+
+    impl SignalConfig for NoTimeoutTestGlobal {
+        fn timeout(&self) -> Option<std::time::Duration> {
+            None
+        }
+    }
+
+    #[tokio::test]
+    async fn bootstrap_service_no_timeout() {
+        let (ctx, handler) = scuffle_context::Context::new();
+        let svc = SignalSvc;
+        let global = NoTimeoutTestGlobal::init().await.unwrap();
+
+        assert!(svc.enabled(&global).await.unwrap());
+        let result = tokio::spawn(svc.run(global, ctx));
+
+        // Wait for the service to start
+        tokio::time::sleep(tokio::time::Duration::from_millis(5)).await;
+
+        raise_signal(tokio::signal::unix::SignalKind::interrupt());
+        assert!(result.await.is_ok());
+
+        assert!(handler
+            .shutdown()
+            .with_timeout(tokio::time::Duration::from_millis(100))
+            .await
+            .is_ok());
+    }
+
+    #[tokio::test]
+    async fn bootstrap_service_force_shutdown() {
+        force_shutdown_two_signals::<NoTimeoutTestGlobal>().await;
+    }
+
+    struct NoSignalsTestGlobal;
+
+    impl GlobalWithoutConfig for NoSignalsTestGlobal {
+        fn init() -> impl std::future::Future<Output = anyhow::Result<Arc<Self>>> + Send {
+            std::future::ready(Ok(Arc::new(Self)))
+        }
+    }
+
+    impl SignalConfig for NoSignalsTestGlobal {
+        fn signals(&self) -> Vec<tokio::signal::unix::SignalKind> {
+            vec![]
+        }
+
+        fn timeout(&self) -> Option<std::time::Duration> {
+            None
+        }
+    }
+
+    #[tokio::test]
+    async fn bootstrap_service_no_signals() {
+        let (ctx, handler) = scuffle_context::Context::new();
+        let svc = SignalSvc;
+        let global = NoSignalsTestGlobal::init().await.unwrap();
+
+        assert!(!svc.enabled(&global).await.unwrap());
+        let result = tokio::spawn(svc.run(global, ctx));
+
+        // Wait for the service to start
+        tokio::time::sleep(tokio::time::Duration::from_millis(10)).await;
+
+        // Make a new handler to catch the raised signal as it is expected to not be
+        // caught by the service
+        let mut signal_handler = SignalHandler::new().with_signal(SignalKind::terminate());
+
+        raise_signal(tokio::signal::unix::SignalKind::terminate());
+
+        // Wait for a signal to be received
+        assert_eq!(signal_handler.recv().await, SignalKind::terminate());
+
+        // Expected to timeout
+        assert!(result.with_timeout(tokio::time::Duration::from_millis(100)).await.is_err());
+
+        assert!(handler
+            .shutdown()
+            .with_timeout(tokio::time::Duration::from_millis(100))
+            .await
+            .is_ok());
+    }
+
+    struct SmallTimeoutTestGlobal;
+
+    impl GlobalWithoutConfig for SmallTimeoutTestGlobal {
+        fn init() -> impl std::future::Future<Output = anyhow::Result<Arc<Self>>> + Send {
+            std::future::ready(Ok(Arc::new(Self)))
+        }
+    }
+
+    impl SignalConfig for SmallTimeoutTestGlobal {
+        fn timeout(&self) -> Option<std::time::Duration> {
+            Some(std::time::Duration::from_millis(5))
+        }
+    }
+
+    #[tokio::test]
+    async fn bootstrap_service_timeout_force_shutdown() {
+        let (ctx, handler) = scuffle_context::Context::new();
+
+        // Block the global context
+        let _global_ctx = scuffle_context::Context::global();
+
+        let svc = SignalSvc;
+        let global = SmallTimeoutTestGlobal::init().await.unwrap();
+
+        assert!(svc.enabled(&global).await.unwrap());
+        let result = tokio::spawn(svc.run(global, ctx));
+
+        // Wait for the service to start
+        tokio::time::sleep(tokio::time::Duration::from_millis(10)).await;
+
+        raise_signal(tokio::signal::unix::SignalKind::terminate());
+
+        match result.with_timeout(tokio::time::Duration::from_millis(100)).await {
+            Ok(Ok(Err(e))) => {
+                assert_eq!(e.to_string(), "timeout reached, shutting down immediately");
+            }
+            _ => panic!("unexpected result"),
+        }
+
+        assert!(handler
+            .shutdown()
+            .with_timeout(tokio::time::Duration::from_millis(100))
+            .await
+            .is_ok());
+    }
+}