testing: Use cargo-mutants to surface gaps in pavex_session's test suite

Mutation testing highlighted a few gaps in the test suite for `pavex_session`
that had not been caught by looking at coverage on its own.
New tests have been added to catch all discovered "MISSED" mutants,
as well as guidance on how to rerun these tools when touching the crate
in the future.

Author: LukeMathWalker

.gitignore

@@ -4,6 +4,7 @@
 /libs/ui_tests/Cargo.lock
 /libs/target
 /libs/vendor
+/libs/**/mutants.out*
 /ci_utils/target
 /doc_examples/tutorial_envs/
 /doc_examples/**/target
@@ -12,10 +13,10 @@
 /examples/**/.cargo
 !/examples/.cargo
 /.cargo
+/.cache/
 /vendor
 .DS_Store
 *.snap
 trace-*.json
-/.cache/
 /docs/api_reference/
 .direnv/

CHANGELOG.md

@@ -9,15 +9,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [0.1.76](https://github.com/LukeMathWalker/pavex/compare/0.1.75...0.1.76) - 2025-02-24
 
-
 ### ⛰️ Features
+
 - Bump nightly version used by pavexc to generate JSON docs from 'nightly-2025-01-04' to 'nightly-2025-02-22' (by @LukeMathWalker) - #454
 - Use Rust 2024 edition in the generated server SDK as well as in the starter project generated by 'pavex new' (by @LukeMathWalker) - #454
 
-
 ### Contributors
 
-* @LukeMathWalker
+- @LukeMathWalker
 
 ## [0.1.75](https://github.com/LukeMathWalker/pavex/compare/0.1.74...0.1.75) - 2025-02-23
 

libs/pavex_session/CONTRIBUTING.md

@@ -0,0 +1,29 @@
+# Testing
+
+HTTP sessions are security-critical components of a backend application.\
+It is therefore important to test the session management system thoroughly.
+
+Whenever you submit a pull request that changes the session management system,\
+you must include tests that verify the correctness of the changes.
+
+There are two key tools you can rely on to spot if more tests are needed:
+
+- **Code coverage**:\
+  The code coverage tool will show you which parts of the code are not tested.\
+  If you see that some parts of the session management system are not tested,\
+  you should write tests for them.\
+  Use [`cargo-llvm-cov`](https://github.com/taiki-e/cargo-llvm-cov) to generate a code coverage report:
+
+  ```bash
+  cargo llvm-cov --no-cfg-coverage -p pavex_session --html --open
+  ```
+- **Mutation testing**:\
+  Mutation testing is a technique to evaluate the quality of your tests.\
+  It works by introducing small changes (mutations) to the code and checking if the tests catch them.\
+  If the tests do not catch the mutations, it means that the tests are not thorough enough.\
+  Use [`cargo-mutants`](https://mutants.rs/welcome.html) to run mutation tests:
+
+  ```bash
+  # Exclude Debug and Display implementations from mutation testing
+  cargo mutants -E "impl Debug" -E "impl Display" -p pavex_session
+  ```

libs/pavex_session/src/config/state.rs

@@ -73,6 +73,7 @@ impl Default for SessionStateConfig {
 }
 
 fn default_ttl() -> std::time::Duration {
+    // 1 day
     std::time::Duration::from_secs(60 * 60 * 24)
 }
 

libs/pavex_session/src/session_.rs

@@ -221,7 +221,7 @@ impl<'store> Session<'store> {
 
         static MAX_N_ATTEMPTS: usize = 16;
 
-        let i = 0;
+        let mut i = 0;
         let new = loop {
             if i >= MAX_N_ATTEMPTS {
                 panic!(
@@ -234,6 +234,8 @@ impl<'store> Session<'store> {
             let new = SessionId::random();
             if Some(new) != old {
                 break new;
+            } else {
+                i += 1;
             }
         };
 

libs/pavex_session/tests/session/config/state.rs

@@ -1,4 +1,6 @@
-use pavex_session::config::TtlExtensionThreshold;
+use pavex_session::{Session, SessionConfig, config::TtlExtensionThreshold};
+
+use crate::fixtures::spy_store;
 
 #[test]
 fn test_ttl_extension_threshold_valid() {
@@ -69,3 +71,39 @@ fn test_ttl_extension_threshold_deserialization() {
         "Expected JSON value to be invalid"
     );
 }
+
+#[tokio::test]
+async fn default_ttl_can_be_changed() {
+    let ((store, call_tracker), mut config) = (spy_store(), SessionConfig::default());
+    let ttl_seconds = 10;
+    config.state.ttl = std::time::Duration::from_secs(ttl_seconds);
+
+    let mut session = Session::new(&store, &config, None);
+    session
+        .server_mut()
+        .set("key".into(), "value")
+        .await
+        .unwrap();
+    session.finalize().await.unwrap().unwrap();
+
+    let oplog = call_tracker.operation_log().await;
+    let last = oplog.last().unwrap();
+    assert_eq!(last, &format!("create <id> {ttl_seconds}s"));
+}
+
+#[tokio::test]
+async fn default_ttl() {
+    let ((store, call_tracker), config) = (spy_store(), SessionConfig::default());
+
+    let mut session = Session::new(&store, &config, None);
+    session
+        .server_mut()
+        .set("key".into(), "value")
+        .await
+        .unwrap();
+    session.finalize().await.unwrap().unwrap();
+
+    let oplog = call_tracker.operation_log().await;
+    let last = oplog.last().unwrap();
+    assert_eq!(last, &format!("create <id> {}s", 60 * 60 * 24));
+}

libs/pavex_session/tests/session/fixtures.rs

@@ -123,6 +123,10 @@ impl CallTracker {
         self.0.lock().await.oplog.clone()
     }
 
+    pub async fn reset_operation_log(&self) {
+        self.0.lock().await.oplog = Vec::new();
+    }
+
     async fn push_operation(&self, op: impl Into<String>) {
         self.0.lock().await.oplog.push(op.into());
     }
@@ -142,7 +146,7 @@ impl<B: SessionStorageBackend> SessionStorageBackend for SpyBackend<B> {
         record: SessionRecordRef<'_>,
     ) -> Result<(), CreateError> {
         self.call_tracker
-            .push_operation(format!("create {}", id.inner()))
+            .push_operation(format!("create <id> {}s", record.ttl.as_secs()))
             .await;
         self.backend.create(id, record).await
     }
@@ -156,7 +160,7 @@ impl<B: SessionStorageBackend> SessionStorageBackend for SpyBackend<B> {
         record: SessionRecordRef<'_>,
     ) -> Result<(), UpdateError> {
         self.call_tracker
-            .push_operation(format!("update {}", id.inner()))
+            .push_operation(format!("update <id> {}s", record.ttl.as_secs()))
             .await;
         self.backend.update(id, record).await
     }
@@ -167,29 +171,25 @@ impl<B: SessionStorageBackend> SessionStorageBackend for SpyBackend<B> {
         ttl: std::time::Duration,
     ) -> Result<(), UpdateTtlError> {
         self.call_tracker
-            .push_operation(format!("update-ttl {}", id.inner()))
+            .push_operation(format!("update-ttl <id> {}s", ttl.as_secs()))
             .await;
         self.backend.update_ttl(id, ttl).await
     }
 
     async fn load(&self, session_id: &SessionId) -> Result<Option<SessionRecord>, LoadError> {
-        self.call_tracker
-            .push_operation(format!("load {}", session_id.inner()))
-            .await;
+        self.call_tracker.push_operation("load <id>").await;
         self.call_tracker.0.lock().await.has_invoked_load = true;
         self.backend.load(session_id).await
     }
 
     async fn delete(&self, session_id: &SessionId) -> Result<(), DeleteError> {
-        self.call_tracker
-            .push_operation(format!("delete {}", session_id.inner()))
-            .await;
+        self.call_tracker.push_operation("delete <id>").await;
         self.backend.delete(session_id).await
     }
 
     async fn change_id(&self, old_id: &SessionId, new_id: &SessionId) -> Result<(), ChangeIdError> {
         self.call_tracker
-            .push_operation(format!("change {} {}", old_id.inner(), new_id.inner()))
+            .push_operation("change <old_id> <new_id>")
             .await;
         self.backend.change_id(old_id, new_id).await
     }

libs/pavex_session/tests/session/main.rs

@@ -4,13 +4,13 @@ use assertions::is_removal_cookie;
 use fixtures::{SessionFixture, spy_store, store};
 use googletest::{
     assert_that,
-    prelude::{eq, none, not},
+    prelude::{empty, eq, len, none, not},
 };
 use helpers::SetCookie;
 use itertools::Itertools;
 use pavex_session::{
     IncomingSession, Session, SessionConfig, SessionId,
-    config::{MissingServerState, ServerStateCreation},
+    config::{MissingServerState, ServerStateCreation, TtlExtensionTrigger},
 };
 
 mod assertions;
@@ -200,8 +200,109 @@ async fn server_state_can_be_cleared_without_invalidating_the_session() {
     assert_eq!(cookie.id(), fixture.id());
 
     // The server state is present, but empty.
-    let server_state = store.load(&fixture.id).await.unwrap();
-    assert!(server_state.is_some());
+    let server_state = store.load(&fixture.id).await.unwrap().unwrap();
+    assert_that!(server_state.state, empty());
+}
+
+#[tokio::test]
+async fn store_is_not_touched_if_you_clear_an_empty_server_state_and_ttl_is_configured_to_update_on_changes()
+ {
+    let ((store, call_tracker), mut config) = (spy_store(), SessionConfig::default());
+    config.state.extend_ttl = TtlExtensionTrigger::OnStateChanges;
+
+    let fixture = SessionFixture::default();
+    let incoming = fixture.setup(&store).await;
+    let mut session = Session::new(&store, &config, Some(incoming));
+    assert!(session.server_mut().is_empty().await.unwrap());
+    // Otherwise `create` and `load` will show up in the operation log.
+    call_tracker.reset_operation_log().await;
+
+    session.server_mut().clear().await.unwrap();
+
+    let cookie = session.finalize().await.unwrap().unwrap();
+
+    // It's not a removal cookie!
+    let cookie = SetCookie::parse(cookie);
+    assert_eq!(cookie.id(), fixture.id());
+
+    call_tracker.assert_store_was_untouched().await;
+}
+
+#[tokio::test]
+async fn ttl_is_updated_if_server_state_is_loaded_but_unchanged() {
+    let ((store, call_tracker), mut config) = (spy_store(), SessionConfig::default());
+    // Always extend TTL
+    config.state.ttl_extension_threshold = None;
+
+    let mut fixture = SessionFixture::default();
+    fixture.server_ttl = Some(config.state.ttl);
+    let incoming = fixture.setup(&store).await;
+    let mut session = Session::new(&store, &config, Some(incoming));
+    assert!(session.server_mut().is_empty().await.unwrap());
+    // Otherwise `create` and `load` will show up in the operation log.
+    call_tracker.reset_operation_log().await;
+
+    let cookie = session.finalize().await.unwrap().unwrap();
+
+    // It's not a removal cookie!
+    let cookie = SetCookie::parse(cookie);
+    assert_eq!(cookie.id(), fixture.id());
+
+    let oplog = call_tracker.operation_log().await;
+    assert_that!(oplog, len(eq(1)));
+    assert!(oplog[0].starts_with("update-ttl"));
+}
+
+#[tokio::test]
+async fn ttl_is_not_updated_if_server_state_is_unchanged_but_ttl_threshold_is_not_met() {
+    let ((store, call_tracker), config) = (spy_store(), SessionConfig::default());
+    let ttl_extension_threshold = config.state.ttl_extension_threshold.unwrap();
+    assert!(ttl_extension_threshold.inner() < 0.9);
+
+    let mut fixture = SessionFixture::default();
+    // We start at full TTL
+    fixture.server_ttl = Some(config.state.ttl);
+    let incoming = fixture.setup(&store).await;
+    let mut session = Session::new(&store, &config, Some(incoming));
+    assert!(session.server_mut().is_empty().await.unwrap());
+    // Otherwise `create` and `load` will show up in the operation log.
+    call_tracker.reset_operation_log().await;
+
+    let cookie = session.finalize().await.unwrap().unwrap();
+
+    // It's not a removal cookie!
+    let cookie = SetCookie::parse(cookie);
+    assert_eq!(cookie.id(), fixture.id());
+
+    let oplog = call_tracker.operation_log().await;
+    assert_that!(oplog, empty());
+}
+
+#[tokio::test]
+async fn ttl_is_updated_if_server_state_is_unchanged_and_ttl_threshold_is_met() {
+    let ((store, call_tracker), config) = (spy_store(), SessionConfig::default());
+    let ttl_extension_threshold = config.state.ttl_extension_threshold.unwrap();
+
+    let mut fixture = SessionFixture::default();
+    // We start below the threshold
+    let ttl = config.state.ttl.as_secs_f32() * (ttl_extension_threshold.inner() - 0.1);
+    assert!(ttl > 0.);
+    fixture.server_ttl = Some(std::time::Duration::from_secs_f32(ttl));
+    let incoming = fixture.setup(&store).await;
+    let mut session = Session::new(&store, &config, Some(incoming));
+    assert!(session.server_mut().is_empty().await.unwrap());
+    // Otherwise `create` and `load` will show up in the operation log.
+    call_tracker.reset_operation_log().await;
+
+    let cookie = session.finalize().await.unwrap().unwrap();
+
+    // It's not a removal cookie!
+    let cookie = SetCookie::parse(cookie);
+    assert_eq!(cookie.id(), fixture.id());
+
+    let oplog = call_tracker.operation_log().await;
+    assert_that!(oplog, len(eq(1)));
+    assert!(oplog[0].starts_with("update-ttl"));
 }
 
 #[tokio::test]