test(ads-client): assert mockito expectations to prevent silent false-positives

components/ads-client/src/client.rs

@@ -280,7 +280,7 @@ mod tests {
         viaduct_dev::init_backend_dev();
 
         let expected_response = get_example_happy_image_response();
-        let _m = mockito::mock("POST", "/ads")
+        let m = mockito::mock("POST", "/ads")
             .with_status(200)
             .with_header("content-type", "application/json")
             .with_body(serde_json::to_string(&expected_response.data).unwrap())
@@ -291,14 +291,15 @@ mod tests {
 
         let result = ads_client.request_image_ads(make_happy_placement_requests(), None, false);
         assert!(result.is_ok());
+        m.assert();
     }
 
     #[test]
     fn test_request_spocs_happy() {
         viaduct_dev::init_backend_dev();
 
         let expected_response = get_example_happy_spoc_response();
-        let _m = mockito::mock("POST", "/ads")
+        let m = mockito::mock("POST", "/ads")
             .with_status(200)
             .with_header("content-type", "application/json")
             .with_body(serde_json::to_string(&expected_response.data).unwrap())
@@ -309,14 +310,15 @@ mod tests {
 
         let result = ads_client.request_spoc_ads(make_happy_placement_requests(), None, false);
         assert!(result.is_ok());
+        m.assert();
     }
 
     #[test]
     fn test_request_tiles_happy() {
         viaduct_dev::init_backend_dev();
 
         let expected_response = get_example_happy_uatile_response();
-        let _m = mockito::mock("POST", "/ads")
+        let m = mockito::mock("POST", "/ads")
             .with_status(200)
             .with_header("content-type", "application/json")
             .with_body(serde_json::to_string(&expected_response.data).unwrap())
@@ -327,6 +329,7 @@ mod tests {
 
         let result = ads_client.request_tile_ads(make_happy_placement_requests(), None, false);
         assert!(result.is_ok());
+        m.assert();
     }
 
     #[test]
@@ -341,7 +344,7 @@ mod tests {
         }
 
         let expected_response = get_example_happy_image_response();
-        let _m = mockito::mock("POST", "/ads")
+        let m = mockito::mock("POST", "/ads")
             .match_body(mockito::Matcher::PartialJsonString(
                 r#"{"context_id":"custom-context-id-12345"}"#.to_string(),
             ))
@@ -362,6 +365,7 @@ mod tests {
 
         let result = client.request_image_ads(make_happy_placement_requests(), None, false);
         assert!(result.is_ok());
+        m.assert();
     }
 
     #[test]
@@ -380,7 +384,7 @@ mod tests {
 
         let response = get_example_happy_image_response();
 
-        let _m1 = mockito::mock("POST", "/ads")
+        let m1 = mockito::mock("POST", "/ads")
             .with_status(200)
             .with_header("content-type", "application/json")
             .with_body(serde_json::to_string(&response.data).unwrap())
@@ -392,7 +396,7 @@ mod tests {
             .unwrap();
         let callback_url = response.values().next().unwrap().callbacks.click.clone();
 
-        let _m2 = mockito::mock("GET", callback_url.path())
+        let m2 = mockito::mock("GET", callback_url.path())
             .with_status(200)
             .create();
 
@@ -409,5 +413,8 @@ mod tests {
                 false,
             )
             .unwrap();
+
+        m1.assert();
+        m2.assert();
     }
 }

components/ads-client/src/http_cache.rs

@@ -214,7 +214,7 @@ mod tests {
         viaduct_dev::init_backend_dev();
 
         let body = r#"{"ok":true}"#;
-        let _m = mock("POST", "/ads")
+        let m = mock("POST", "/ads")
             .with_status(200)
             .with_header("content-type", "application/json")
             .with_body(body)
@@ -237,24 +237,19 @@ mod tests {
             .unwrap();
         assert!(matches!(outcomes.last().unwrap(), CacheOutcome::Hit));
         assert_eq!(response.status, 200);
+        m.assert();
     }
 
     #[test]
     fn test_refresh_policy_always_uses_network_then_caches() {
         viaduct_dev::init_backend_dev();
 
-        let body1 = r#"{"ok":true,"n":1}"#;
-        let body2 = r#"{"ok":true,"n":2}"#;
-        // Two live responses expected on refresh
-        let _m1 = mock("POST", "/ads")
-            .with_status(200)
-            .with_header("content-type", "application/json")
-            .with_body(body1)
-            .create();
-        let _m2 = mock("POST", "/ads")
+        let body = r#"{"ok":true}"#;
+        let m = mock("POST", "/ads")
             .with_status(200)
             .with_header("content-type", "application/json")
-            .with_body(body2)
+            .with_body(body)
+            .expect(2)
             .create();
 
         let cache = make_cache();
@@ -271,24 +266,25 @@ mod tests {
             .unwrap();
         assert!(matches!(outcomes.last().unwrap(), CacheOutcome::MissStored));
 
-        // Second refresh: live again (different body), still MissStored
+        // Second refresh: live again, still MissStored
         let (response, outcomes) = cache
             .send_with_policy(&client, req, &CachePolicy::NetworkFirst { ttl: None })
             .unwrap();
         assert!(matches!(outcomes.last().unwrap(), CacheOutcome::MissStored));
         assert_eq!(response.status, 200);
+        m.assert();
     }
 
     #[test]
     fn test_not_cacheable_no_store() {
         viaduct_dev::init_backend_dev();
 
-        let _m = mock("POST", "/ads")
+        let m = mock("POST", "/ads")
             .with_status(200)
             .with_header("content-type", "application/json")
             .with_header("cache-control", "no-store") // should block caching
             .with_body(r#"{"ok":true}"#)
-            .expect(1)
+            .expect(2) // both calls should hit the network since the response isn't cacheable
             .create();
 
         let cache = make_cache();
@@ -304,27 +300,21 @@ mod tests {
         ));
 
         // Next call should hit network again (since we didn't cache)
-        let _m2 = mock("POST", "/ads")
-            .with_status(200)
-            .with_header("content-type", "application/json")
-            .with_body(r#"{"ok":true}"#)
-            .expect(1)
-            .create();
         let (_, outcomes) = cache
             .send_with_policy(&client, req, &CachePolicy::default())
             .unwrap();
-        // Either MissStored (if headers differ) or MissNotCacheable if still no-store
         assert!(matches!(
             outcomes.last().unwrap(),
-            CacheOutcome::MissStored | CacheOutcome::MissNotCacheable
+            CacheOutcome::MissNotCacheable
         ));
+        m.assert();
     }
 
     #[test]
     fn ttl_resolution_min_of_server_request_default() {
         viaduct_dev::init_backend_dev();
 
-        let _m = mockito::mock("POST", "/ads")
+        let m = mockito::mock("POST", "/ads")
             .with_status(200)
             .with_header("content-type", "application/json")
             .with_header("cache-control", "max-age=1") // Set max age to 1 second
@@ -349,13 +339,14 @@ mod tests {
         cache.store.delete_expired_entries().unwrap();
 
         assert!(cache.store.lookup(&hash).unwrap().is_none());
+        m.assert();
     }
 
     #[test]
     fn ttl_resolution_request_overrides_default_when_smaller() {
         viaduct_dev::init_backend_dev();
 
-        let _m = mockito::mock("POST", "/ads")
+        let m = mockito::mock("POST", "/ads")
             .with_status(200)
             .with_header("content-type", "application/json")
             .with_body(r#"{"ok":true}"#)
@@ -383,13 +374,14 @@ mod tests {
         cache.store.get_clock().advance(2);
         cache.store.delete_expired_entries().unwrap();
         assert!(cache.store.lookup(&hash).unwrap().is_none());
+        m.assert();
     }
 
     #[test]
     fn ttl_resolution_uses_default_when_no_server_and_no_request_override() {
         viaduct_dev::init_backend_dev();
 
-        let _m = mockito::mock("POST", "/ads")
+        let m = mockito::mock("POST", "/ads")
             .with_status(200)
             // No response policy ttl
             .with_header("content-type", "application/json")
@@ -416,21 +408,18 @@ mod tests {
         cache.store.get_clock().advance(3);
         cache.store.delete_expired_entries().unwrap();
         assert!(cache.store.lookup(&hash).unwrap().is_none());
+        m.assert();
     }
 
     #[test]
     fn test_expired_entry_is_a_miss_on_next_send() {
         viaduct_dev::init_backend_dev();
 
-        let _m1 = mockito::mock("POST", "/ads")
+        let m = mockito::mock("POST", "/ads")
             .with_status(200)
             .with_header("content-type", "application/json")
-            .with_body(r#"{"ok":true,"n":1}"#)
-            .create();
-        let _m2 = mockito::mock("POST", "/ads")
-            .with_status(200)
-            .with_header("content-type", "application/json")
-            .with_body(r#"{"ok":true,"n":2}"#)
+            .with_body(r#"{"ok":true}"#)
+            .expect(2) // both calls should hit the network — second after the entry expires
             .create();
 
         let cache = make_cache_with_ttl(2);
@@ -451,6 +440,7 @@ mod tests {
             .send_with_policy(&client, req, &CachePolicy::default())
             .unwrap();
         assert!(matches!(outcomes.last().unwrap(), CacheOutcome::MissStored));
+        m.assert();
     }
 
     #[test]

components/ads-client/src/mars.rs

@@ -161,7 +161,7 @@ mod tests {
     #[test]
     fn test_record_impression_with_valid_url_should_succeed() {
         viaduct_dev::init_backend_dev();
-        let _m = mock("GET", "/impression_callback_url")
+        let m = mock("GET", "/impression_callback_url")
             .with_status(200)
             .create();
         let client = make_test_client(None);
@@ -172,23 +172,25 @@ mod tests {
         .unwrap();
         let result = client.record_impression(url, false);
         assert!(result.is_ok());
+        m.assert();
     }
 
     #[test]
     fn test_record_click_with_valid_url_should_succeed() {
         viaduct_dev::init_backend_dev();
-        let _m = mock("GET", "/click_callback_url").with_status(200).create();
+        let m = mock("GET", "/click_callback_url").with_status(200).create();
 
         let client = make_test_client(None);
         let url = Url::parse(&format!("{}/click_callback_url", &mockito::server_url())).unwrap();
         let result = client.record_click(url, false);
         assert!(result.is_ok());
+        m.assert();
     }
 
     #[test]
     fn test_report_ad_with_valid_url_should_succeed() {
         viaduct_dev::init_backend_dev();
-        let _m = mock("GET", "/report_ad_callback_url")
+        let m = mock("GET", "/report_ad_callback_url")
             .match_query(mockito::Matcher::UrlEncoded(
                 "reason".into(),
                 "not_interested".into(),
@@ -204,14 +206,15 @@ mod tests {
         .unwrap();
         let result = client.report_ad(url, ReportReason::NotInterested, false);
         assert!(result.is_ok());
+        m.assert();
     }
 
     #[test]
     fn test_fetch_ads_success() {
         viaduct_dev::init_backend_dev();
         let expected_response = get_example_happy_image_response();
 
-        let _m = mock("POST", "/ads")
+        let m = mock("POST", "/ads")
             .match_header("content-type", "application/json")
             .with_status(200)
             .with_header("content-type", "application/json")
@@ -229,20 +232,26 @@ mod tests {
         assert!(result.is_ok());
         let (response, _request_hash) = result.unwrap();
         assert_eq!(expected_response, response);
+        m.assert();
     }
 
     #[test]
     fn test_fetch_ads_cache_hit_skips_network() {
         viaduct_dev::init_backend_dev();
         let expected = get_example_happy_image_response();
-        let _m = mock("POST", "/ads")
+        let m = mock("POST", "/ads")
             .with_status(200)
             .with_header("content-type", "application/json")
             .with_body(serde_json::to_string(&expected.data).unwrap())
             .expect(1) // only first request goes to network
             .create();
 
-        let client = make_test_client(None);
+        let cache = HttpCache::builder("test_fetch_ads_cache_hit_skips_network.db")
+            .default_ttl(std::time::Duration::from_secs(300))
+            .max_size(crate::http_cache::ByteSize::mib(1))
+            .build()
+            .unwrap();
+        let client = make_test_client(Some(cache));
 
         // First call should be a miss then warm the cache
         let (response1, _) = client
@@ -265,6 +274,7 @@ mod tests {
             )
             .unwrap();
         assert_eq!(response2, expected);
+        m.assert();
     }
 
     #[test]
@@ -279,10 +289,11 @@ mod tests {
         let client = make_test_client(Some(cache));
         let callback_url = Url::parse(&format!("{}/click", mockito::server_url())).unwrap();
 
-        let _m = mock("GET", "/click").with_status(200).create();
+        let m = mock("GET", "/click").with_status(200).create();
 
         let result = client.record_click(callback_url, false);
         assert!(result.is_ok());
+        m.assert();
     }
 
     #[test]
@@ -297,9 +308,10 @@ mod tests {
         let client = make_test_client(Some(cache));
         let callback_url = Url::parse(&format!("{}/impression", mockito::server_url())).unwrap();
 
-        let _m = mock("GET", "/impression").with_status(200).create();
+        let m = mock("GET", "/impression").with_status(200).create();
 
         let result = client.record_impression(callback_url, false);
         assert!(result.is_ok());
+        m.assert();
     }
 }