From 3be48244c51f0d6018c869e28f52577f82f48dab Mon Sep 17 00:00:00 2001 From: ahanot Date: Thu, 19 Mar 2026 15:37:34 -0400 Subject: [PATCH 01/10] feat(ads-client): add integration tests against MARS staging --- .github/workflows/ads-client-tests.yaml | 28 ++++ Cargo.lock | 1 + components/ads-client/Cargo.toml | 2 + components/ads-client/README.md | 10 +- .../ads-client/tests/integration_test.rs | 139 ++++++++---------- 5 files changed, 96 insertions(+), 84 deletions(-) create mode 100644 .github/workflows/ads-client-tests.yaml diff --git a/.github/workflows/ads-client-tests.yaml b/.github/workflows/ads-client-tests.yaml new file mode 100644 index 0000000000..379584a5b2 --- /dev/null +++ b/.github/workflows/ads-client-tests.yaml @@ -0,0 +1,28 @@ +name: Ads Client Tests + +on: + push: + branches: [ main ] + paths: + - 'components/ads-client/**' + pull_request: + branches: [ main ] + paths: + - 'components/ads-client/**' + + workflow_dispatch: + +jobs: + integration-tests: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + submodules: 'recursive' + - name: Install Rust + run: | + curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y + source $HOME/.cargo/env + rustup toolchain install + - name: Run ads-client integration tests against MARS staging + run: cargo test -p ads-client --features integration-tests --test integration_test diff --git a/Cargo.lock b/Cargo.lock index 4e0d6b2cd6..f76e0cc2fc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -38,6 +38,7 @@ dependencies = [ "uuid", "viaduct", "viaduct-dev", + "viaduct-hyper", ] [[package]] diff --git a/components/ads-client/Cargo.toml b/components/ads-client/Cargo.toml index 67d93c1ed5..2323d4bcd3 100644 --- a/components/ads-client/Cargo.toml +++ b/components/ads-client/Cargo.toml @@ -8,6 +8,7 @@ exclude = ["/android"] [features] dev = [] default = [] +integration-tests = [] [dependencies] chrono = "0.4" @@ -34,6 +35,7 @@ sql-support = { path = "../support/sql" } mockall = "0.12" mockito = { version = "0.31", default-features = false } viaduct-dev = { path = "../support/viaduct-dev" } +viaduct-hyper = { path = "../support/viaduct-hyper" } [build-dependencies] uniffi = { version = "0.31", features = ["build"] } diff --git a/components/ads-client/README.md b/components/ads-client/README.md index e6473eb7ef..6879f43837 100644 --- a/components/ads-client/README.md +++ b/components/ads-client/README.md @@ -24,21 +24,21 @@ cargo test -p ads-client ### Integration Tests -Integration tests make real HTTP calls to the Mozilla Ads Routing Service (MARS) and are not run automatically in CI. They are marked with `#[ignore]` and must be run manually. +Integration tests make real HTTP calls to the Mozilla Ads Routing Service (MARS) staging environment. They are gated behind the `integration-tests` Cargo feature and are only compiled and run by the dedicated GitHub Actions workflow (`.github/workflows/ads-client-tests.yaml`). -To run integration tests: +To run integration tests locally: ```shell -cargo test -p ads-client --test integration_test -- --ignored +cargo test -p ads-client --features integration-tests --test integration_test ``` To run a specific integration test: ```shell -cargo test -p ads-client --test integration_test -- --ignored test_mock_pocket_billboard_1_placement +cargo test -p ads-client --features integration-tests --test integration_test test_contract_image_staging ``` -**Note:** Integration tests require network access and will make real HTTP requests to the MARS API. +**Note:** Integration tests require network access and will make real HTTP requests to the MARS staging API. ## Usage diff --git a/components/ads-client/tests/integration_test.rs b/components/ads-client/tests/integration_test.rs index b73d8fa542..39566945a9 100644 --- a/components/ads-client/tests/integration_test.rs +++ b/components/ads-client/tests/integration_test.rs @@ -3,12 +3,16 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +#![cfg(feature = "integration-tests")] + use std::hash::{Hash, Hasher}; +use std::sync::Arc; use std::time::Duration; use ads_client::{ http_cache::{ByteSize, CacheOutcome, HttpCache, RequestCachePolicy}, - MozAdsClientBuilder, MozAdsPlacementRequest, MozAdsPlacementRequestWithCount, + MozAdsClientBuilder, MozAdsEnvironment, MozAdsPlacementRequest, + MozAdsPlacementRequestWithCount, }; use url::Url; use viaduct::Request; @@ -30,104 +34,81 @@ impl From for Request { } } -#[test] -#[ignore] -fn test_mock_pocket_billboard_1_placement() { - viaduct_dev::init_backend_dev(); - - let client = MozAdsClientBuilder::new().build(); - - let placement_request = MozAdsPlacementRequest { - placement_id: "mock_pocket_billboard_1".to_string(), - iab_content: None, - }; - - let result = client.request_image_ads(vec![placement_request], None); - - assert!(result.is_ok(), "Failed to request ads: {:?}", result.err()); - - let placements = result.unwrap(); - - assert!( - placements.contains_key("mock_pocket_billboard_1"), - "Response should contain placement_id 'mock_pocket_billboard_1'" - ); +fn init_backend() { + // Err means the backend is already initialized. + let _ = viaduct_hyper::viaduct_init_backend_hyper(); +} - placements - .get("mock_pocket_billboard_1") - .expect("Placement should exist"); +fn staging_client() -> ads_client::MozAdsClient { + Arc::new(MozAdsClientBuilder::new()) + .environment(MozAdsEnvironment::Staging) + .build() } #[test] -#[ignore] -fn test_newtab_spocs_placement() { - viaduct_dev::init_backend_dev(); - - let client = MozAdsClientBuilder::new().build(); - - let count = 3; - let placement_request = MozAdsPlacementRequestWithCount { - placement_id: "newtab_spocs".to_string(), - count, - iab_content: None, - }; - - let result = client.request_spoc_ads(vec![placement_request], None); - - assert!(result.is_ok(), "Failed to request ads: {:?}", result.err()); - - let placements = result.unwrap(); - - assert!( - placements.contains_key("newtab_spocs"), - "Response should contain placement_id 'newtab_spocs'" +fn test_contract_image_staging() { + init_backend(); + + let client = staging_client(); + let result = client.request_image_ads( + vec![MozAdsPlacementRequest { + placement_id: "mock_billboard_1".to_string(), + iab_content: None, + }], + None, ); - let spocs = placements - .get("newtab_spocs") - .expect("Placement should exist"); - - assert_eq!( - spocs.len(), - count as usize, - "Number of spocs should equal count parameter" + assert!( + result.is_ok(), + "Image ad request failed: {:?}", + result.err() ); + let placements = result.unwrap(); + assert!(placements.contains_key("mock_billboard_1")); } #[test] -#[ignore] -fn test_newtab_tile_1_placement() { - viaduct_dev::init_backend_dev(); - - let client = MozAdsClientBuilder::new().build(); - - let placement_request = MozAdsPlacementRequest { - placement_id: "newtab_tile_1".to_string(), - iab_content: None, - }; - - let result = client.request_tile_ads(vec![placement_request], None); - - assert!(result.is_ok(), "Failed to request ads: {:?}", result.err()); +fn test_contract_spoc_staging() { + init_backend(); + + let client = staging_client(); + let result = client.request_spoc_ads( + vec![MozAdsPlacementRequestWithCount { + placement_id: "mock_spoc_1".to_string(), + count: 1, + iab_content: None, + }], + None, + ); + assert!(result.is_ok(), "Spoc ad request failed: {:?}", result.err()); let placements = result.unwrap(); + assert!(placements.contains_key("mock_spoc_1")); +} - assert!( - placements.contains_key("newtab_tile_1"), - "Response should contain placement_id 'newtab_tile_1'" +#[test] +fn test_contract_tile_staging() { + init_backend(); + + let client = staging_client(); + let result = client.request_tile_ads( + vec![MozAdsPlacementRequest { + placement_id: "mock_tile_1".to_string(), + iab_content: None, + }], + None, ); - placements - .get("newtab_tile_1") - .expect("Placement should exist"); + assert!(result.is_ok(), "Tile ad request failed: {:?}", result.err()); + let placements = result.unwrap(); + assert!(placements.contains_key("mock_tile_1")); } #[test] -#[ignore] fn test_cache_works_using_real_timeouts() { - viaduct_dev::init_backend_dev(); + init_backend(); - let cache: HttpCache = HttpCache::builder("integration_tests.db") + let cache = HttpCache::::builder("integration_tests.db") .default_ttl(Duration::from_secs(60)) .max_size(ByteSize::mib(1)) .build() From e16b554339c2560c1ea8b152c4ff64f3fb56fb36 Mon Sep 17 00:00:00 2001 From: ahanot Date: Thu, 19 Mar 2026 16:50:44 -0400 Subject: [PATCH 02/10] refactor(ads-client): move integration tests to a dedicated crate Extracts the integration tests into a new `ads-client-integration-tests` crate under `components/ads-client/integration-tests/`. This keeps `viaduct-hyper` out of `ads-client`'s dev-dependencies entirely, since it was only needed to back the HTTP calls in those tests. - Remove `integration-tests` feature flag from `ads-client` - Remove `viaduct-hyper` from `ads-client` dev-dependencies - Add new crate to workspace `members` (not `default-members`) - Update CI workflow to run `cargo test -p ads-client-integration-tests` --- .github/workflows/ads-client-tests.yaml | 2 +- Cargo.lock | 10 ++++++++++ Cargo.toml | 4 +++- components/ads-client/Cargo.toml | 2 -- .../ads-client/integration-tests/Cargo.toml | 15 +++++++++++++++ .../tests/integration_test.rs | 2 -- 6 files changed, 29 insertions(+), 6 deletions(-) create mode 100644 components/ads-client/integration-tests/Cargo.toml rename components/ads-client/{ => integration-tests}/tests/integration_test.rs (99%) diff --git a/.github/workflows/ads-client-tests.yaml b/.github/workflows/ads-client-tests.yaml index 379584a5b2..917347e38b 100644 --- a/.github/workflows/ads-client-tests.yaml +++ b/.github/workflows/ads-client-tests.yaml @@ -25,4 +25,4 @@ jobs: source $HOME/.cargo/env rustup toolchain install - name: Run ads-client integration tests against MARS staging - run: cargo test -p ads-client --features integration-tests --test integration_test + run: cargo test -p ads-client-integration-tests --test integration_test diff --git a/Cargo.lock b/Cargo.lock index f76e0cc2fc..95876709b5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -38,6 +38,16 @@ dependencies = [ "uuid", "viaduct", "viaduct-dev", +] + +[[package]] +name = "ads-client-integration-tests" +version = "0.1.0" +dependencies = [ + "ads-client", + "serde_json", + "url", + "viaduct", "viaduct-hyper", ] diff --git a/Cargo.toml b/Cargo.toml index ad07d4126f..fa6dfbf011 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,9 +1,11 @@ [workspace] resolver = "2" -# Note: Any additions here should be repeated in default-members below. +# Note: Any additions here should be repeated in default-members below (unless +# the crate should not be built by default, e.g. integration-test-only crates). members = [ "components/ads-client", + "components/ads-client/integration-tests", "components/as-ohttp-client", "components/autofill", "components/context_id", diff --git a/components/ads-client/Cargo.toml b/components/ads-client/Cargo.toml index 2323d4bcd3..67d93c1ed5 100644 --- a/components/ads-client/Cargo.toml +++ b/components/ads-client/Cargo.toml @@ -8,7 +8,6 @@ exclude = ["/android"] [features] dev = [] default = [] -integration-tests = [] [dependencies] chrono = "0.4" @@ -35,7 +34,6 @@ sql-support = { path = "../support/sql" } mockall = "0.12" mockito = { version = "0.31", default-features = false } viaduct-dev = { path = "../support/viaduct-dev" } -viaduct-hyper = { path = "../support/viaduct-hyper" } [build-dependencies] uniffi = { version = "0.31", features = ["build"] } diff --git a/components/ads-client/integration-tests/Cargo.toml b/components/ads-client/integration-tests/Cargo.toml new file mode 100644 index 0000000000..8a0baedbeb --- /dev/null +++ b/components/ads-client/integration-tests/Cargo.toml @@ -0,0 +1,15 @@ +[package] +name = "ads-client-integration-tests" +version = "0.1.0" +edition = "2021" +license = "MPL-2.0" +publish = false + +[dependencies] + +[dev-dependencies] +ads-client = { path = ".." } +serde_json = "1" +url = "2" +viaduct = { path = "../../../components/viaduct" } +viaduct-hyper = { path = "../../../components/support/viaduct-hyper" } diff --git a/components/ads-client/tests/integration_test.rs b/components/ads-client/integration-tests/tests/integration_test.rs similarity index 99% rename from components/ads-client/tests/integration_test.rs rename to components/ads-client/integration-tests/tests/integration_test.rs index 39566945a9..f0ca1261b7 100644 --- a/components/ads-client/tests/integration_test.rs +++ b/components/ads-client/integration-tests/tests/integration_test.rs @@ -3,8 +3,6 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -#![cfg(feature = "integration-tests")] - use std::hash::{Hash, Hasher}; use std::sync::Arc; use std::time::Duration; From 79cfac6d86a3f3eeab7ceb4180ccedefd0e6610d Mon Sep 17 00:00:00 2001 From: ahanot Date: Thu, 19 Mar 2026 16:52:41 -0400 Subject: [PATCH 03/10] docs(ads-client): update integration test instructions for new crate --- components/ads-client/README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/components/ads-client/README.md b/components/ads-client/README.md index 6879f43837..a99d883c1d 100644 --- a/components/ads-client/README.md +++ b/components/ads-client/README.md @@ -24,18 +24,18 @@ cargo test -p ads-client ### Integration Tests -Integration tests make real HTTP calls to the Mozilla Ads Routing Service (MARS) staging environment. They are gated behind the `integration-tests` Cargo feature and are only compiled and run by the dedicated GitHub Actions workflow (`.github/workflows/ads-client-tests.yaml`). +Integration tests make real HTTP calls to the Mozilla Ads Routing Service (MARS) staging environment. They live in a dedicated crate (`integration-tests/`) and are only run by the dedicated GitHub Actions workflow (`.github/workflows/ads-client-tests.yaml`). To run integration tests locally: ```shell -cargo test -p ads-client --features integration-tests --test integration_test +cargo test -p ads-client-integration-tests --test integration_test ``` To run a specific integration test: ```shell -cargo test -p ads-client --features integration-tests --test integration_test test_contract_image_staging +cargo test -p ads-client-integration-tests --test integration_test test_contract_image_staging ``` **Note:** Integration tests require network access and will make real HTTP requests to the MARS staging API. From 3a616869636b5915f12367e68914f2174b9c3153 Mon Sep 17 00:00:00 2001 From: ahanot Date: Thu, 19 Mar 2026 16:53:46 -0400 Subject: [PATCH 04/10] refactor(ads-client): split integration tests into mars and http_cache files --- .github/workflows/ads-client-tests.yaml | 2 +- components/ads-client/README.md | 8 +- .../{integration_test.rs => http_cache.rs} | 74 +---------------- .../integration-tests/tests/mars.rs | 81 +++++++++++++++++++ 4 files changed, 89 insertions(+), 76 deletions(-) rename components/ads-client/integration-tests/tests/{integration_test.rs => http_cache.rs} (54%) create mode 100644 components/ads-client/integration-tests/tests/mars.rs diff --git a/.github/workflows/ads-client-tests.yaml b/.github/workflows/ads-client-tests.yaml index 917347e38b..067a1895bc 100644 --- a/.github/workflows/ads-client-tests.yaml +++ b/.github/workflows/ads-client-tests.yaml @@ -25,4 +25,4 @@ jobs: source $HOME/.cargo/env rustup toolchain install - name: Run ads-client integration tests against MARS staging - run: cargo test -p ads-client-integration-tests --test integration_test + run: cargo test -p ads-client-integration-tests diff --git a/components/ads-client/README.md b/components/ads-client/README.md index a99d883c1d..ac9357994c 100644 --- a/components/ads-client/README.md +++ b/components/ads-client/README.md @@ -29,13 +29,15 @@ Integration tests make real HTTP calls to the Mozilla Ads Routing Service (MARS) To run integration tests locally: ```shell -cargo test -p ads-client-integration-tests --test integration_test +cargo test -p ads-client-integration-tests ``` -To run a specific integration test: +To run a specific test file or test: ```shell -cargo test -p ads-client-integration-tests --test integration_test test_contract_image_staging +cargo test -p ads-client-integration-tests --test mars +cargo test -p ads-client-integration-tests --test http_cache +cargo test -p ads-client-integration-tests --test mars test_contract_image_staging ``` **Note:** Integration tests require network access and will make real HTTP requests to the MARS staging API. diff --git a/components/ads-client/integration-tests/tests/integration_test.rs b/components/ads-client/integration-tests/tests/http_cache.rs similarity index 54% rename from components/ads-client/integration-tests/tests/integration_test.rs rename to components/ads-client/integration-tests/tests/http_cache.rs index f0ca1261b7..988b257089 100644 --- a/components/ads-client/integration-tests/tests/integration_test.rs +++ b/components/ads-client/integration-tests/tests/http_cache.rs @@ -4,14 +4,9 @@ */ use std::hash::{Hash, Hasher}; -use std::sync::Arc; use std::time::Duration; -use ads_client::{ - http_cache::{ByteSize, CacheOutcome, HttpCache, RequestCachePolicy}, - MozAdsClientBuilder, MozAdsEnvironment, MozAdsPlacementRequest, - MozAdsPlacementRequestWithCount, -}; +use ads_client::http_cache::{ByteSize, CacheOutcome, CacheMode, HttpCache, RequestCachePolicy}; use url::Url; use viaduct::Request; @@ -37,71 +32,6 @@ fn init_backend() { let _ = viaduct_hyper::viaduct_init_backend_hyper(); } -fn staging_client() -> ads_client::MozAdsClient { - Arc::new(MozAdsClientBuilder::new()) - .environment(MozAdsEnvironment::Staging) - .build() -} - -#[test] -fn test_contract_image_staging() { - init_backend(); - - let client = staging_client(); - let result = client.request_image_ads( - vec![MozAdsPlacementRequest { - placement_id: "mock_billboard_1".to_string(), - iab_content: None, - }], - None, - ); - - assert!( - result.is_ok(), - "Image ad request failed: {:?}", - result.err() - ); - let placements = result.unwrap(); - assert!(placements.contains_key("mock_billboard_1")); -} - -#[test] -fn test_contract_spoc_staging() { - init_backend(); - - let client = staging_client(); - let result = client.request_spoc_ads( - vec![MozAdsPlacementRequestWithCount { - placement_id: "mock_spoc_1".to_string(), - count: 1, - iab_content: None, - }], - None, - ); - - assert!(result.is_ok(), "Spoc ad request failed: {:?}", result.err()); - let placements = result.unwrap(); - assert!(placements.contains_key("mock_spoc_1")); -} - -#[test] -fn test_contract_tile_staging() { - init_backend(); - - let client = staging_client(); - let result = client.request_tile_ads( - vec![MozAdsPlacementRequest { - placement_id: "mock_tile_1".to_string(), - iab_content: None, - }], - None, - ); - - assert!(result.is_ok(), "Tile ad request failed: {:?}", result.err()); - let placements = result.unwrap(); - assert!(placements.contains_key("mock_tile_1")); -} - #[test] fn test_cache_works_using_real_timeouts() { init_backend(); @@ -130,7 +60,7 @@ fn test_cache_works_using_real_timeouts() { .send_with_policy( req.clone(), &RequestCachePolicy { - mode: ads_client::http_cache::CacheMode::CacheFirst, + mode: CacheMode::CacheFirst, ttl_seconds: Some(test_ttl), }, ) diff --git a/components/ads-client/integration-tests/tests/mars.rs b/components/ads-client/integration-tests/tests/mars.rs new file mode 100644 index 0000000000..6e711fa88a --- /dev/null +++ b/components/ads-client/integration-tests/tests/mars.rs @@ -0,0 +1,81 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public +* License, v. 2.0. If a copy of the MPL was not distributed with this +* file, You can obtain one at http://mozilla.org/MPL/2.0/. +*/ + +use std::sync::Arc; + +use ads_client::{ + MozAdsClientBuilder, MozAdsEnvironment, MozAdsPlacementRequest, + MozAdsPlacementRequestWithCount, +}; + +fn init_backend() { + // Err means the backend is already initialized. + let _ = viaduct_hyper::viaduct_init_backend_hyper(); +} + +fn staging_client() -> ads_client::MozAdsClient { + Arc::new(MozAdsClientBuilder::new()) + .environment(MozAdsEnvironment::Staging) + .build() +} + +#[test] +fn test_contract_image_staging() { + init_backend(); + + let client = staging_client(); + let result = client.request_image_ads( + vec![MozAdsPlacementRequest { + placement_id: "mock_billboard_1".to_string(), + iab_content: None, + }], + None, + ); + + assert!( + result.is_ok(), + "Image ad request failed: {:?}", + result.err() + ); + let placements = result.unwrap(); + assert!(placements.contains_key("mock_billboard_1")); +} + +#[test] +fn test_contract_spoc_staging() { + init_backend(); + + let client = staging_client(); + let result = client.request_spoc_ads( + vec![MozAdsPlacementRequestWithCount { + placement_id: "mock_spoc_1".to_string(), + count: 1, + iab_content: None, + }], + None, + ); + + assert!(result.is_ok(), "Spoc ad request failed: {:?}", result.err()); + let placements = result.unwrap(); + assert!(placements.contains_key("mock_spoc_1")); +} + +#[test] +fn test_contract_tile_staging() { + init_backend(); + + let client = staging_client(); + let result = client.request_tile_ads( + vec![MozAdsPlacementRequest { + placement_id: "mock_tile_1".to_string(), + iab_content: None, + }], + None, + ); + + assert!(result.is_ok(), "Tile ad request failed: {:?}", result.err()); + let placements = result.unwrap(); + assert!(placements.contains_key("mock_tile_1")); +} From 346089848b4c697c80a8bbd92711d8c88f8aefda Mon Sep 17 00:00:00 2001 From: ahanot Date: Thu, 19 Mar 2026 16:54:14 -0400 Subject: [PATCH 05/10] chore(ads-client): cargo fmt --- components/ads-client/integration-tests/tests/http_cache.rs | 2 +- components/ads-client/integration-tests/tests/mars.rs | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/components/ads-client/integration-tests/tests/http_cache.rs b/components/ads-client/integration-tests/tests/http_cache.rs index 988b257089..6d2d2c8622 100644 --- a/components/ads-client/integration-tests/tests/http_cache.rs +++ b/components/ads-client/integration-tests/tests/http_cache.rs @@ -6,7 +6,7 @@ use std::hash::{Hash, Hasher}; use std::time::Duration; -use ads_client::http_cache::{ByteSize, CacheOutcome, CacheMode, HttpCache, RequestCachePolicy}; +use ads_client::http_cache::{ByteSize, CacheMode, CacheOutcome, HttpCache, RequestCachePolicy}; use url::Url; use viaduct::Request; diff --git a/components/ads-client/integration-tests/tests/mars.rs b/components/ads-client/integration-tests/tests/mars.rs index 6e711fa88a..2c76d8ae14 100644 --- a/components/ads-client/integration-tests/tests/mars.rs +++ b/components/ads-client/integration-tests/tests/mars.rs @@ -6,8 +6,7 @@ use std::sync::Arc; use ads_client::{ - MozAdsClientBuilder, MozAdsEnvironment, MozAdsPlacementRequest, - MozAdsPlacementRequestWithCount, + MozAdsClientBuilder, MozAdsEnvironment, MozAdsPlacementRequest, MozAdsPlacementRequestWithCount, }; fn init_backend() { From 5786edfcdc43f6fb57eef861740458179f1aa919 Mon Sep 17 00:00:00 2001 From: ahanot Date: Tue, 24 Mar 2026 14:12:40 -0400 Subject: [PATCH 06/10] fix(ads-client): fix spoc staging test assertion --- components/ads-client/integration-tests/tests/mars.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/components/ads-client/integration-tests/tests/mars.rs b/components/ads-client/integration-tests/tests/mars.rs index 2c76d8ae14..638d0cd7a7 100644 --- a/components/ads-client/integration-tests/tests/mars.rs +++ b/components/ads-client/integration-tests/tests/mars.rs @@ -50,7 +50,7 @@ fn test_contract_spoc_staging() { let result = client.request_spoc_ads( vec![MozAdsPlacementRequestWithCount { placement_id: "mock_spoc_1".to_string(), - count: 1, + count: 3, iab_content: None, }], None, @@ -59,6 +59,7 @@ fn test_contract_spoc_staging() { assert!(result.is_ok(), "Spoc ad request failed: {:?}", result.err()); let placements = result.unwrap(); assert!(placements.contains_key("mock_spoc_1")); + assert!(placements.get("mock_spoc_1").unwrap().len() == 3); } #[test] From aa61c9678e9935e78b848c1b1ab77eb0ee1f6c45 Mon Sep 17 00:00:00 2001 From: ahanot Date: Tue, 24 Mar 2026 16:03:23 -0400 Subject: [PATCH 07/10] fix(ads-client): assert matches! in http_cache integration test --- components/ads-client/integration-tests/tests/http_cache.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/components/ads-client/integration-tests/tests/http_cache.rs b/components/ads-client/integration-tests/tests/http_cache.rs index 6d2d2c8622..71c7f3de1a 100644 --- a/components/ads-client/integration-tests/tests/http_cache.rs +++ b/components/ads-client/integration-tests/tests/http_cache.rs @@ -65,13 +65,13 @@ fn test_cache_works_using_real_timeouts() { }, ) .unwrap(); - matches!(o1.cache_outcome, CacheOutcome::MissStored); + assert!(matches!(o1.cache_outcome, CacheOutcome::MissStored)); // Second call: hit (no extra HTTP) but no refresh let o2 = cache .send_with_policy(req.clone(), &RequestCachePolicy::default()) .unwrap(); - matches!(o2.cache_outcome, CacheOutcome::Hit); + assert!(matches!(o2.cache_outcome, CacheOutcome::Hit)); assert_eq!(o2.response.status, 200); // Third call: Miss due to timeout for the test_ttl duration @@ -79,6 +79,6 @@ fn test_cache_works_using_real_timeouts() { let o3 = cache .send_with_policy(req, &RequestCachePolicy::default()) .unwrap(); - matches!(o3.cache_outcome, CacheOutcome::MissStored); + assert!(matches!(o3.cache_outcome, CacheOutcome::MissStored)); assert_eq!(o3.response.status, 200); } From 29f06263677593adbb8ae4dabbb5e677d775f4f2 Mon Sep 17 00:00:00 2001 From: ahanot Date: Tue, 24 Mar 2026 17:02:08 -0400 Subject: [PATCH 08/10] fix(ads-client): fix TTL expiry and refactor cache outcome reporting - Prune expired entries before every cache lookup in send_with_policy so expired entries are never served as hits - Change expiry comparison from strict < to <= so entries expire exactly at their TTL boundary, not one second after - Change send_with_policy return type to Vec so both CleanupFailed and the primary outcome (Hit/MissStored/etc.) can be reported in the same call; HttpCacheSendResult is now a concrete type alias with no generic parameter - Remove delete_expired_entries from fetch_and_cache (now handled entirely in send_with_policy) - Add regression test: advancing the TestClock past TTL must yield MissStored, not Hit - Mark integration test as #[ignore] for manual runs; update CI workflow to use dtolnay/rust-toolchain and pass -- --ignored; consolidate stray ads-client-integration-tests.yml into ads-client-tests.yaml --- .github/workflows/ads-client-tests.yaml | 12 +- components/ads-client/README.md | 12 +- .../integration-tests/tests/http_cache.rs | 17 +-- .../integration-tests/tests/mars.rs | 3 + components/ads-client/src/http_cache.rs | 125 +++++++++++------- components/ads-client/src/http_cache/store.rs | 4 +- components/ads-client/src/mars.rs | 10 +- 7 files changed, 107 insertions(+), 76 deletions(-) diff --git a/.github/workflows/ads-client-tests.yaml b/.github/workflows/ads-client-tests.yaml index 067a1895bc..b6bfb8fa64 100644 --- a/.github/workflows/ads-client-tests.yaml +++ b/.github/workflows/ads-client-tests.yaml @@ -2,13 +2,13 @@ name: Ads Client Tests on: push: - branches: [ main ] + branches: [main] paths: - - 'components/ads-client/**' + - "components/ads-client/**" pull_request: - branches: [ main ] + branches: [main] paths: - - 'components/ads-client/**' + - "components/ads-client/**" workflow_dispatch: @@ -18,11 +18,11 @@ jobs: steps: - uses: actions/checkout@v4 with: - submodules: 'recursive' + submodules: "recursive" - name: Install Rust run: | curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y source $HOME/.cargo/env rustup toolchain install - name: Run ads-client integration tests against MARS staging - run: cargo test -p ads-client-integration-tests + run: cargo test -p ads-client-integration-tests -- --ignored diff --git a/components/ads-client/README.md b/components/ads-client/README.md index ac9357994c..fbe9a0a85b 100644 --- a/components/ads-client/README.md +++ b/components/ads-client/README.md @@ -24,20 +24,20 @@ cargo test -p ads-client ### Integration Tests -Integration tests make real HTTP calls to the Mozilla Ads Routing Service (MARS) staging environment. They live in a dedicated crate (`integration-tests/`) and are only run by the dedicated GitHub Actions workflow (`.github/workflows/ads-client-tests.yaml`). +Integration tests make real HTTP calls to the Mozilla Ads Routing Service (MARS) staging environment. They live in a dedicated crate (`integration-tests/`) and are marked `#[ignore]` so they do not run with a plain `cargo test`. -To run integration tests locally: +They are run by the dedicated GitHub Actions workflow (`.github/workflows/ads-client-tests.yaml`), and can also be run manually: ```shell -cargo test -p ads-client-integration-tests +cargo test -p ads-client-integration-tests -- --ignored ``` To run a specific test file or test: ```shell -cargo test -p ads-client-integration-tests --test mars -cargo test -p ads-client-integration-tests --test http_cache -cargo test -p ads-client-integration-tests --test mars test_contract_image_staging +cargo test -p ads-client-integration-tests --test mars -- --ignored +cargo test -p ads-client-integration-tests --test http_cache -- --ignored +cargo test -p ads-client-integration-tests --test mars test_contract_image_staging -- --ignored ``` **Note:** Integration tests require network access and will make real HTTP requests to the MARS staging API. diff --git a/components/ads-client/integration-tests/tests/http_cache.rs b/components/ads-client/integration-tests/tests/http_cache.rs index 71c7f3de1a..ed2a1e55e5 100644 --- a/components/ads-client/integration-tests/tests/http_cache.rs +++ b/components/ads-client/integration-tests/tests/http_cache.rs @@ -33,6 +33,7 @@ fn init_backend() { } #[test] +#[ignore = "integration test: run manually with -- --ignored"] fn test_cache_works_using_real_timeouts() { init_backend(); @@ -56,7 +57,7 @@ fn test_cache_works_using_real_timeouts() { let test_ttl = 2; // First call: miss -> store - let o1 = cache + let (_, outcomes) = cache .send_with_policy( req.clone(), &RequestCachePolicy { @@ -65,20 +66,20 @@ fn test_cache_works_using_real_timeouts() { }, ) .unwrap(); - assert!(matches!(o1.cache_outcome, CacheOutcome::MissStored)); + assert!(matches!(outcomes.last().unwrap(), CacheOutcome::MissStored)); // Second call: hit (no extra HTTP) but no refresh - let o2 = cache + let (response, outcomes) = cache .send_with_policy(req.clone(), &RequestCachePolicy::default()) .unwrap(); - assert!(matches!(o2.cache_outcome, CacheOutcome::Hit)); - assert_eq!(o2.response.status, 200); + assert!(matches!(outcomes.last().unwrap(), CacheOutcome::Hit)); + assert_eq!(response.status, 200); // Third call: Miss due to timeout for the test_ttl duration std::thread::sleep(Duration::from_secs(test_ttl)); - let o3 = cache + let (response, outcomes) = cache .send_with_policy(req, &RequestCachePolicy::default()) .unwrap(); - assert!(matches!(o3.cache_outcome, CacheOutcome::MissStored)); - assert_eq!(o3.response.status, 200); + assert!(matches!(outcomes.last().unwrap(), CacheOutcome::MissStored)); + assert_eq!(response.status, 200); } diff --git a/components/ads-client/integration-tests/tests/mars.rs b/components/ads-client/integration-tests/tests/mars.rs index 638d0cd7a7..24a12da4bc 100644 --- a/components/ads-client/integration-tests/tests/mars.rs +++ b/components/ads-client/integration-tests/tests/mars.rs @@ -21,6 +21,7 @@ fn staging_client() -> ads_client::MozAdsClient { } #[test] +#[ignore = "integration test: run manually with -- --ignored"] fn test_contract_image_staging() { init_backend(); @@ -43,6 +44,7 @@ fn test_contract_image_staging() { } #[test] +#[ignore = "integration test: run manually with -- --ignored"] fn test_contract_spoc_staging() { init_backend(); @@ -63,6 +65,7 @@ fn test_contract_spoc_staging() { } #[test] +#[ignore = "integration test: run manually with -- --ignored"] fn test_contract_tile_staging() { init_backend(); diff --git a/components/ads-client/src/http_cache.rs b/components/ads-client/src/http_cache.rs index 41de9da039..049fe7efdf 100644 --- a/components/ads-client/src/http_cache.rs +++ b/components/ads-client/src/http_cache.rs @@ -22,7 +22,8 @@ use std::cmp; use std::path::Path; use std::time::Duration; -pub type HttpCacheSendResult = std::result::Result; +pub type HttpCacheSendResult = + std::result::Result<(Response, Vec), viaduct::ViaductError>; #[derive(Clone, Copy, Debug, Default)] pub struct RequestCachePolicy { @@ -57,11 +58,6 @@ pub enum CacheOutcome { CleanupFailed(HttpCacheError), // cleaning expired objects failed } -pub struct SendOutcome { - pub response: Response, - pub cache_outcome: CacheOutcome, -} - pub struct HttpCache> { max_size: ByteSize, store: HttpCacheStore, @@ -90,7 +86,8 @@ impl> HttpCache { &self, item: T, request_policy: &RequestCachePolicy, - ) -> HttpCacheSendResult { + ) -> HttpCacheSendResult { + let mut outcomes = vec![]; let request_hash = RequestHash::new(&item); let request: Request = item.into(); let request_policy_ttl = match request_policy.ttl_seconds { @@ -98,25 +95,31 @@ impl> HttpCache { None => self.default_ttl, }; + if let Err(e) = self.store.delete_expired_entries() { + outcomes.push(CacheOutcome::CleanupFailed(e.into())); + } + if request_policy.mode == CacheMode::CacheFirst { match self.store.lookup(&request_hash) { Ok(Some(response)) => { - return Ok(SendOutcome { - response, - cache_outcome: CacheOutcome::Hit, - }); + outcomes.push(CacheOutcome::Hit); + return Ok((response, outcomes)); } Err(e) => { - let mut outcome = + outcomes.push(CacheOutcome::LookupFailed(e)); + let (response, mut fetch_outcomes) = self.fetch_and_cache(&request, &request_hash, &request_policy_ttl)?; - outcome.cache_outcome = CacheOutcome::LookupFailed(e); - return Ok(outcome); + outcomes.append(&mut fetch_outcomes); + return Ok((response, outcomes)); } Ok(None) => {} } } - self.fetch_and_cache(&request, &request_hash, &request_policy_ttl) + let (response, mut fetch_outcomes) = + self.fetch_and_cache(&request, &request_hash, &request_policy_ttl)?; + outcomes.append(&mut fetch_outcomes); + Ok((response, outcomes)) } fn fetch_and_cache( @@ -124,14 +127,8 @@ impl> HttpCache { request: &Request, request_hash: &RequestHash, request_policy_ttl: &Duration, - ) -> HttpCacheSendResult { + ) -> HttpCacheSendResult { let response = request.clone().send()?; - if let Err(e) = self.store.delete_expired_entries() { - return Ok(SendOutcome { - response, - cache_outcome: CacheOutcome::CleanupFailed(e.into()), - }); - } let cache_control = CacheControl::from(&response); let cache_outcome = if cache_control.should_cache() { let response_ttl = match cache_control.max_age { @@ -146,10 +143,7 @@ impl> HttpCache { ); if final_ttl.as_secs() == 0 { - return Ok(SendOutcome { - response, - cache_outcome: CacheOutcome::NoCache, - }); + return Ok((response, vec![CacheOutcome::NoCache])); } match self.cache_object(request_hash, &response, &final_ttl) { @@ -160,10 +154,7 @@ impl> HttpCache { CacheOutcome::MissNotCacheable }; - Ok(SendOutcome { - response, - cache_outcome, - }) + Ok((response, vec![cache_outcome])) } fn cache_object( @@ -289,17 +280,17 @@ mod tests { let req = make_post_request(); // First call: miss -> store - let o1 = cache + let (_, outcomes) = cache .send_with_policy(req.clone(), &RequestCachePolicy::default()) .unwrap(); - matches!(o1.cache_outcome, CacheOutcome::MissStored); + matches!(outcomes.last().unwrap(), CacheOutcome::MissStored); // Second call: hit (no extra HTTP request due to expect(1)) - let o2 = cache + let (response, outcomes) = cache .send_with_policy(req, &RequestCachePolicy::default()) .unwrap(); - matches!(o2.cache_outcome, CacheOutcome::Hit); - assert_eq!(o2.response.status, 200); + matches!(outcomes.last().unwrap(), CacheOutcome::Hit); + assert_eq!(response.status, 200); } #[test] @@ -324,7 +315,7 @@ mod tests { let req = make_post_request(); // First refresh: live -> MissStored - let o1 = cache + let (_, outcomes) = cache .send_with_policy( req.clone(), &RequestCachePolicy { @@ -333,10 +324,10 @@ mod tests { }, ) .unwrap(); - matches!(o1.cache_outcome, CacheOutcome::MissStored); + matches!(outcomes.last().unwrap(), CacheOutcome::MissStored); // Second refresh: live again (different body), still MissStored - let o2 = cache + let (response, outcomes) = cache .send_with_policy( req, &RequestCachePolicy { @@ -345,8 +336,8 @@ mod tests { }, ) .unwrap(); - matches!(o2.cache_outcome, CacheOutcome::MissStored); - assert_eq!(o2.response.status, 200); + matches!(outcomes.last().unwrap(), CacheOutcome::MissStored); + assert_eq!(response.status, 200); } #[test] @@ -364,10 +355,10 @@ mod tests { let cache = make_cache(); let req = make_post_request(); - let o = cache + let (_, outcomes) = cache .send_with_policy(req.clone(), &RequestCachePolicy::default()) .unwrap(); - matches!(o.cache_outcome, CacheOutcome::MissNotCacheable); + matches!(outcomes.last().unwrap(), CacheOutcome::MissNotCacheable); // Next call should hit network again (since we didn't cache) let _m2 = mock("POST", "/ads") @@ -376,12 +367,12 @@ mod tests { .with_body(r#"{"ok":true}"#) .expect(1) .create(); - let o2 = cache + let (_, outcomes) = cache .send_with_policy(req, &RequestCachePolicy::default()) .unwrap(); // Either MissStored (if headers differ) or MissNotCacheable if still no-store assert!(matches!( - o2.cache_outcome, + outcomes.last().unwrap(), CacheOutcome::MissStored | CacheOutcome::MissNotCacheable )); } @@ -407,8 +398,8 @@ mod tests { }; // Store ttl should resolve to 1s as specified by response headers - let out = cache.send_with_policy(req, &policy).unwrap(); - assert!(matches!(out.cache_outcome, CacheOutcome::MissStored)); + let (_, outcomes) = cache.send_with_policy(req, &policy).unwrap(); + assert!(matches!(outcomes.last().unwrap(), CacheOutcome::MissStored)); // After ~>1s, cleanup should remove it cache.store.get_clock().advance(2); @@ -437,8 +428,8 @@ mod tests { }; // Store with effective TTL = 2s - let out = cache.send_with_policy(req, &policy).unwrap(); - assert!(matches!(out.cache_outcome, CacheOutcome::MissStored)); + let (_, outcomes) = cache.send_with_policy(req, &policy).unwrap(); + assert!(matches!(outcomes.last().unwrap(), CacheOutcome::MissStored)); // Not expired yet at ~1s cache.store.get_clock().advance(1); @@ -470,8 +461,8 @@ mod tests { let policy = RequestCachePolicy::default(); // Store with effective TTL = 2s from client default - let out = cache.send_with_policy(req, &policy).unwrap(); - assert!(matches!(out.cache_outcome, CacheOutcome::MissStored)); + let (_, outcomes) = cache.send_with_policy(req, &policy).unwrap(); + assert!(matches!(outcomes.last().unwrap(), CacheOutcome::MissStored)); // Not expired at ~1s cache.store.get_clock().advance(1); @@ -484,6 +475,40 @@ mod tests { assert!(cache.store.lookup(&hash).unwrap().is_none()); } + #[test] + fn test_expired_entry_is_a_miss_on_next_send() { + viaduct_dev::init_backend_dev(); + + let _m1 = 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}"#) + .create(); + + let cache = make_cache_with_ttl(2); + let req = make_post_request(); + + // First call: miss -> store with 2s TTL + let (_, outcomes) = cache + .send_with_policy(req.clone(), &RequestCachePolicy::default()) + .unwrap(); + assert!(matches!(outcomes.last().unwrap(), CacheOutcome::MissStored)); + + // Advance clock past the TTL + cache.store.get_clock().advance(3); + + // Second call: expired entry must be a miss, not a hit + let (_, outcomes) = cache + .send_with_policy(req, &RequestCachePolicy::default()) + .unwrap(); + assert!(matches!(outcomes.last().unwrap(), CacheOutcome::MissStored)); + } + #[test] fn test_invalidate_by_hash() { let cache: HttpCache = diff --git a/components/ads-client/src/http_cache/store.rs b/components/ads-client/src/http_cache/store.rs index 0eaf283c57..74dab586f2 100644 --- a/components/ads-client/src/http_cache/store.rs +++ b/components/ads-client/src/http_cache/store.rs @@ -74,7 +74,7 @@ impl HttpCacheStore { Ok(ByteSize::b(size_bytes)) } - /// Removes all entries from the store who's expires_at is before the current time. + /// Removes all entries from the store whose expires_at is at or before the current time. pub fn delete_expired_entries(&self) -> SqliteResult { #[cfg(test)] if *self.fault.lock() == FaultKind::Cleanup { @@ -82,7 +82,7 @@ impl HttpCacheStore { } let conn = self.conn.lock(); conn.execute( - "DELETE FROM http_cache WHERE expires_at < ?1", + "DELETE FROM http_cache WHERE expires_at <= ?1", params![self.clock.now_epoch_seconds()], ) } diff --git a/components/ads-client/src/mars.rs b/components/ads-client/src/mars.rs index c8bc420811..f9b25ab97a 100644 --- a/components/ads-client/src/mars.rs +++ b/components/ads-client/src/mars.rs @@ -55,10 +55,12 @@ where let request_hash = RequestHash::new(&ad_request); let response: AdResponse = if let Some(cache) = self.http_cache.as_ref() { - let outcome = cache.send_with_policy(ad_request, cache_policy)?; - self.telemetry.record(&outcome.cache_outcome); - check_http_status_for_error(&outcome.response)?; - AdResponse::::parse(outcome.response.json()?, &self.telemetry)? + let (response, cache_outcomes) = cache.send_with_policy(ad_request, cache_policy)?; + for outcome in &cache_outcomes { + self.telemetry.record(outcome); + } + check_http_status_for_error(&response)?; + AdResponse::::parse(response.json()?, &self.telemetry)? } else { let request: Request = ad_request.into(); let response = request.send()?; From 5b07ea90976871bb5c65227cb0a6731705134bf6 Mon Sep 17 00:00:00 2001 From: ahanot Date: Wed, 25 Mar 2026 10:46:12 -0400 Subject: [PATCH 09/10] refactor(ads-client): use mockito in http_cache integration test; fix stray matches! - Replace real HTTP calls in http_cache integration test with mockito, removing the viaduct-hyper dependency from that test path; add viaduct-dev and mockito to the integration-tests crate - Wrap 5 bare matches! calls in assert! in http_cache unit tests - Consolidate CI workflow: use dtolnay/rust-toolchain, pass -- --ignored to pick up ignored integration tests, remove stray workflow file --- Cargo.lock | 2 ++ .../ads-client/integration-tests/Cargo.toml | 2 ++ .../integration-tests/tests/http_cache.rs | 22 +++++++++++++++---- components/ads-client/src/http_cache.rs | 10 ++++----- 4 files changed, 27 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 95876709b5..c2f6923806 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -45,9 +45,11 @@ name = "ads-client-integration-tests" version = "0.1.0" dependencies = [ "ads-client", + "mockito", "serde_json", "url", "viaduct", + "viaduct-dev", "viaduct-hyper", ] diff --git a/components/ads-client/integration-tests/Cargo.toml b/components/ads-client/integration-tests/Cargo.toml index 8a0baedbeb..d78ce2c8e1 100644 --- a/components/ads-client/integration-tests/Cargo.toml +++ b/components/ads-client/integration-tests/Cargo.toml @@ -11,5 +11,7 @@ publish = false ads-client = { path = ".." } serde_json = "1" url = "2" +mockito = { version = "0.31", default-features = false } viaduct = { path = "../../../components/viaduct" } +viaduct-dev = { path = "../../../components/support/viaduct-dev" } viaduct-hyper = { path = "../../../components/support/viaduct-hyper" } diff --git a/components/ads-client/integration-tests/tests/http_cache.rs b/components/ads-client/integration-tests/tests/http_cache.rs index ed2a1e55e5..005aaa7fa5 100644 --- a/components/ads-client/integration-tests/tests/http_cache.rs +++ b/components/ads-client/integration-tests/tests/http_cache.rs @@ -7,7 +7,7 @@ use std::hash::{Hash, Hasher}; use std::time::Duration; use ads_client::http_cache::{ByteSize, CacheMode, CacheOutcome, HttpCache, RequestCachePolicy}; -use url::Url; +use mockito::mock; use viaduct::Request; /// Test-only hashable wrapper around Request. @@ -29,7 +29,7 @@ impl From for Request { fn init_backend() { // Err means the backend is already initialized. - let _ = viaduct_hyper::viaduct_init_backend_hyper(); + let _ = viaduct_dev::init_backend_dev(); } #[test] @@ -43,7 +43,7 @@ fn test_cache_works_using_real_timeouts() { .build() .expect("cache build should succeed"); - let url = Url::parse("https://ads.mozilla.org/v1/ads").unwrap(); + let url = format!("{}/v1/ads", mockito::server_url()).parse().unwrap(); let req = TestRequest(Request::post(url).json(&serde_json::json!({ "context_id": "12347fff-00b0-aaaa-0978-189231239808", "placements": [ @@ -56,6 +56,13 @@ fn test_cache_works_using_real_timeouts() { let test_ttl = 2; + let _m1 = mock("POST", "/v1/ads") + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"ok":true}"#) + .expect(1) + .create(); + // First call: miss -> store let (_, outcomes) = cache .send_with_policy( @@ -68,13 +75,20 @@ fn test_cache_works_using_real_timeouts() { .unwrap(); assert!(matches!(outcomes.last().unwrap(), CacheOutcome::MissStored)); - // Second call: hit (no extra HTTP) but no refresh + // Second call: hit (no extra HTTP due to expect(1)) let (response, outcomes) = cache .send_with_policy(req.clone(), &RequestCachePolicy::default()) .unwrap(); assert!(matches!(outcomes.last().unwrap(), CacheOutcome::Hit)); assert_eq!(response.status, 200); + let _m2 = mock("POST", "/v1/ads") + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"ok":true}"#) + .expect(1) + .create(); + // Third call: Miss due to timeout for the test_ttl duration std::thread::sleep(Duration::from_secs(test_ttl)); let (response, outcomes) = cache diff --git a/components/ads-client/src/http_cache.rs b/components/ads-client/src/http_cache.rs index 049fe7efdf..4851959dbb 100644 --- a/components/ads-client/src/http_cache.rs +++ b/components/ads-client/src/http_cache.rs @@ -283,13 +283,13 @@ mod tests { let (_, outcomes) = cache .send_with_policy(req.clone(), &RequestCachePolicy::default()) .unwrap(); - matches!(outcomes.last().unwrap(), CacheOutcome::MissStored); + assert!(matches!(outcomes.last().unwrap(), CacheOutcome::MissStored)); // Second call: hit (no extra HTTP request due to expect(1)) let (response, outcomes) = cache .send_with_policy(req, &RequestCachePolicy::default()) .unwrap(); - matches!(outcomes.last().unwrap(), CacheOutcome::Hit); + assert!(matches!(outcomes.last().unwrap(), CacheOutcome::Hit)); assert_eq!(response.status, 200); } @@ -324,7 +324,7 @@ mod tests { }, ) .unwrap(); - matches!(outcomes.last().unwrap(), CacheOutcome::MissStored); + assert!(matches!(outcomes.last().unwrap(), CacheOutcome::MissStored)); // Second refresh: live again (different body), still MissStored let (response, outcomes) = cache @@ -336,7 +336,7 @@ mod tests { }, ) .unwrap(); - matches!(outcomes.last().unwrap(), CacheOutcome::MissStored); + assert!(matches!(outcomes.last().unwrap(), CacheOutcome::MissStored)); assert_eq!(response.status, 200); } @@ -358,7 +358,7 @@ mod tests { let (_, outcomes) = cache .send_with_policy(req.clone(), &RequestCachePolicy::default()) .unwrap(); - matches!(outcomes.last().unwrap(), CacheOutcome::MissNotCacheable); + assert!(matches!(outcomes.last().unwrap(), CacheOutcome::MissNotCacheable)); // Next call should hit network again (since we didn't cache) let _m2 = mock("POST", "/ads") From 43eb42bb15870f65a0f67c7f7fedaedb62b844cd Mon Sep 17 00:00:00 2001 From: ahanot Date: Wed, 25 Mar 2026 11:47:08 -0400 Subject: [PATCH 10/10] chore(ads-client): cargo fmt --- .../ads-client/integration-tests/tests/http_cache.rs | 7 +------ components/ads-client/src/http_cache.rs | 5 ++++- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/components/ads-client/integration-tests/tests/http_cache.rs b/components/ads-client/integration-tests/tests/http_cache.rs index 005aaa7fa5..32a9f4889f 100644 --- a/components/ads-client/integration-tests/tests/http_cache.rs +++ b/components/ads-client/integration-tests/tests/http_cache.rs @@ -27,15 +27,10 @@ impl From for Request { } } -fn init_backend() { - // Err means the backend is already initialized. - let _ = viaduct_dev::init_backend_dev(); -} - #[test] #[ignore = "integration test: run manually with -- --ignored"] fn test_cache_works_using_real_timeouts() { - init_backend(); + viaduct_dev::init_backend_dev(); let cache = HttpCache::::builder("integration_tests.db") .default_ttl(Duration::from_secs(60)) diff --git a/components/ads-client/src/http_cache.rs b/components/ads-client/src/http_cache.rs index 4851959dbb..b3492916e3 100644 --- a/components/ads-client/src/http_cache.rs +++ b/components/ads-client/src/http_cache.rs @@ -358,7 +358,10 @@ mod tests { let (_, outcomes) = cache .send_with_policy(req.clone(), &RequestCachePolicy::default()) .unwrap(); - assert!(matches!(outcomes.last().unwrap(), CacheOutcome::MissNotCacheable)); + assert!(matches!( + outcomes.last().unwrap(), + CacheOutcome::MissNotCacheable + )); // Next call should hit network again (since we didn't cache) let _m2 = mock("POST", "/ads")