From e07ee8f2e44ec5a86c8f4ff5e3c05c16fcb12fa2 Mon Sep 17 00:00:00 2001 From: Tony Bierman Date: Thu, 14 May 2026 03:49:13 -0500 Subject: [PATCH 1/3] Add blitz-net Provider integration tests Workspace-member test crate (tests/blitz-net) covering Provider public-API behavior that WPT can't reach: scheme dispatch, HTTP status mapping, body encoding, abort signals, per-host concurrency limits, and feature-gated paths (cookies, cache, multipart). Uses wiremock for in-process HTTP servers. Also fixes file:// URL handling on Windows (the new tests surfaced it): url.path() returns a URL-encoded path with a leading `/` that isn't a valid filesystem path on Windows; use url::Url::to_file_path instead. CI runs default-feature tests via the existing workspace job and adds a dedicated job for cookies+cache and cookies+multipart combos (mutually incompatible in blitz-net). Clippy lints both feature combinations. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/ci.yml | 14 + Cargo.lock | 67 +++++ Cargo.toml | 1 + packages/blitz-net/src/lib.rs | 8 +- tests/README.md | 39 +++ tests/blitz-net/Cargo.toml | 22 ++ tests/blitz-net/tests/common/mod.rs | 101 +++++++ tests/blitz-net/tests/concurrency.rs | 303 +++++++++++++++++++++ tests/blitz-net/tests/features.rs | 210 +++++++++++++++ tests/blitz-net/tests/http_behavior.rs | 354 +++++++++++++++++++++++++ tests/blitz-net/tests/schemes.rs | 166 ++++++++++++ 11 files changed, 1284 insertions(+), 1 deletion(-) create mode 100644 tests/README.md create mode 100644 tests/blitz-net/Cargo.toml create mode 100644 tests/blitz-net/tests/common/mod.rs create mode 100644 tests/blitz-net/tests/concurrency.rs create mode 100644 tests/blitz-net/tests/features.rs create mode 100644 tests/blitz-net/tests/http_behavior.rs create mode 100644 tests/blitz-net/tests/schemes.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 90e511696e..a8d25b8dbb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -77,6 +77,16 @@ jobs: version: 1.0 - run: cargo test --workspace + test-blitz-net-features: + name: "Test blitz-net [cookies,cache,multipart]" + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@stable + - uses: Swatinem/rust-cache@v2 + - run: cargo test -p blitz-net-tests --features cookies,cache + - run: cargo test -p blitz-net-tests --features cookies,multipart + build-counter: name: "Build counter example" runs-on: ubuntu-latest @@ -135,6 +145,10 @@ jobs: packages: libgtk-3-dev libxdo-dev version: 1.0 - run: cargo clippy --workspace -- -D warnings + # Lint blitz-net-tests' feature-gated modules — cache and multipart are + # mutually incompatible in blitz-net, so they need separate passes. + - run: cargo clippy -p blitz-net-tests --tests --features cookies,cache -- -D warnings + - run: cargo clippy -p blitz-net-tests --tests --features cookies,multipart -- -D warnings doc: name: Documentation diff --git a/Cargo.lock b/Cargo.lock index 4fb5372e3f..97db34d6e0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -484,6 +484,16 @@ dependencies = [ "raw-window-metal 0.4.0", ] +[[package]] +name = "assert-json-diff" +version = "2.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "47e4f2b81832e72834d7518d8487a0396a28cc408186a2e8854c0f98011faf12" +dependencies = [ + "serde", + "serde_json", +] + [[package]] name = "async-broadcast" version = "0.7.2" @@ -918,6 +928,21 @@ dependencies = [ "wasm-bindgen-futures", ] +[[package]] +name = "blitz-net-tests" +version = "0.0.0" +dependencies = [ + "blitz-net", + "blitz-traits", + "bytes", + "futures-util", + "http", + "tempfile", + "tokio", + "url", + "wiremock", +] + [[package]] name = "blitz-paint" version = "0.3.0-alpha.2" @@ -1889,6 +1914,24 @@ dependencies = [ "system-deps 7.0.8", ] +[[package]] +name = "deadpool" +version = "0.12.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0be2b1d1d6ec8d846f05e137292d0b89133caf95ef33695424c09568bdd39b1b" +dependencies = [ + "deadpool-runtime", + "lazy_static", + "num_cpus", + "tokio", +] + +[[package]] +name = "deadpool-runtime" +version = "0.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "092966b41edc516079bdf31ec78a2e0588d1d0c08f78b91d8307215928642b2b" + [[package]] name = "debug_timer" version = "0.1.3" @@ -3542,6 +3585,7 @@ dependencies = [ "http", "http-body", "httparse", + "httpdate", "itoa", "pin-project-lite", "smallvec", @@ -9464,6 +9508,29 @@ dependencies = [ "memchr", ] +[[package]] +name = "wiremock" +version = "0.6.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "08db1edfb05d9b3c1542e521aea074442088292f00b5f28e435c714a98f85031" +dependencies = [ + "assert-json-diff", + "base64 0.22.1", + "deadpool", + "futures", + "http", + "http-body-util", + "hyper", + "hyper-util", + "log", + "once_cell", + "regex", + "serde", + "serde_json", + "tokio", + "url", +] + [[package]] name = "wit-bindgen" version = "0.51.0" diff --git a/Cargo.toml b/Cargo.toml index fef8e5e407..ad37270be8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,6 +17,7 @@ members = [ "apps/readme", "apps/bump", "wpt/runner", + "tests/blitz-net", "examples/counter", "examples/seven_guis", "examples/todomvc", diff --git a/packages/blitz-net/src/lib.rs b/packages/blitz-net/src/lib.rs index cad2ae9adc..981f0aa750 100644 --- a/packages/blitz-net/src/lib.rs +++ b/packages/blitz-net/src/lib.rs @@ -129,7 +129,13 @@ impl Provider { Ok((request.url.to_string(), Bytes::from(decoded.0))) } "file" => { - let file_content = std::fs::read(request.url.path())?; + let path = request.url.to_file_path().map_err(|_| { + std::io::Error::new( + std::io::ErrorKind::InvalidInput, + format!("invalid file URL: {}", request.url), + ) + })?; + let file_content = std::fs::read(path)?; Ok((request.url.to_string(), Bytes::from(file_content))) } _ => Self::fetch_http(client, request, per_host_limits).await, diff --git a/tests/README.md b/tests/README.md new file mode 100644 index 0000000000..0268e541a4 --- /dev/null +++ b/tests/README.md @@ -0,0 +1,39 @@ +# Integration tests + +This directory hosts workspace-member test crates that exercise individual Blitz packages from the outside. + +## Layout + +- `blitz-net/` — public-API regression tests for `packages/blitz-net`. Uses `wiremock` to stand up in-process HTTP servers; asserts on `Provider` behavior, scheme dispatch, body encoding, abort signals, per-host concurrency limits, and feature-gated paths (`cookies`, `cache`, `multipart`). +- `stylo_usage.rs` — historical example file; not wired into a crate. + +`cargo test --workspace` runs the default-feature tests. Feature-gated tests need explicit flags (see the crate's `Cargo.toml`). + +## Why these tests exist alongside WPT + +Blitz already has a large test surface in `wpt/runner/` — the [web-platform-tests](https://github.com/web-platform-tests/wpt) conformance suite. The crates here cover what WPT can't, for two reasons: + +**WPT doesn't use `blitz-net::Provider`.** The WPT runner ships its own `NetProvider` impl (`wpt/runner/src/net_provider.rs`) that resolves URLs against a local checkout of the WPT git repo. No HTTP, no `reqwest`, no semaphore, no cookies. Every behavior worth regressing on in `blitz-net` — per-host limiting, User-Agent injection, status-code mapping, cache middleware, multipart bodies, abort signals — is bypassed by the runner. Wiring `blitz-net::Provider` into WPT would require running a `wptserve` instance and adapting the runner's loader, which is a much larger project than testing the crate directly. + +**WPT's assertion model is shaped for renderer conformance, not API regression.** WPT compares rendered bitmaps against reference images via `dify` pixel-diffs. The blitz-net tests need structural assertions on Rust values — `matches!(err, ProviderError::HttpStatus { status, .. })`, `provider.count() == 1`, `received_requests().len() == 6` — that have no natural expression in a visual-diff framework. + +The two suites are complementary: WPT answers "does Blitz render this HTML/CSS correctly per spec?"; the crates here answer "does this package's public API still behave as documented?" + +## Running the tests + +```sh +# Default-feature tests (also covered by `cargo test --workspace`) +cargo test -p blitz-net-tests + +# Feature-gated tests — `cookies,cache` and `cookies,multipart` are run separately +# because the `cache` and `multipart` features are currently incompatible in blitz-net. +cargo test -p blitz-net-tests --features cookies,cache +cargo test -p blitz-net-tests --features cookies,multipart + +# Single test +cargo test -p blitz-net-tests injects_user_agent_header + +# Single test file +cargo test -p blitz-net-tests --test concurrency +``` + diff --git a/tests/blitz-net/Cargo.toml b/tests/blitz-net/Cargo.toml new file mode 100644 index 0000000000..49d43b0993 --- /dev/null +++ b/tests/blitz-net/Cargo.toml @@ -0,0 +1,22 @@ +[package] +name = "blitz-net-tests" +version = "0.0.0" +edition = "2024" +publish = false + +[features] +default = [] +cookies = ["blitz-net/cookies"] +cache = ["blitz-net/cache"] +multipart = ["blitz-net/multipart"] + +[dev-dependencies] +blitz-net = { path = "../../packages/blitz-net" } +blitz-traits = { workspace = true } +tokio = { workspace = true, features = ["macros", "rt-multi-thread", "sync", "time"] } +wiremock = "0.6" +tempfile = "3" +futures-util = { workspace = true } +url = { workspace = true } +http = { workspace = true } +bytes = { workspace = true } diff --git a/tests/blitz-net/tests/common/mod.rs b/tests/blitz-net/tests/common/mod.rs new file mode 100644 index 0000000000..ed9e3246db --- /dev/null +++ b/tests/blitz-net/tests/common/mod.rs @@ -0,0 +1,101 @@ +// Each helper is used by at least one test binary but not by all of them, so per-item +// `#[allow(dead_code)]` is required — without it, every binary's compile would warn on +// the helpers it doesn't reference. Prefer the per-item form over a blanket file-level +// allow so genuinely unreferenced helpers still surface in review. +use blitz_traits::net::{Bytes, NetHandler, NetWaker}; +use std::io::Write as _; +use std::sync::{Arc, Mutex}; +use std::time::{Duration, Instant}; +use wiremock::matchers::method; +use wiremock::{Mock, MockServer, ResponseTemplate}; + +// 300ms is wide enough to keep mid-flight observation windows deterministic on slow CI. +#[allow(dead_code)] +pub const RESPONSE_DELAY: Duration = Duration::from_millis(300); + +#[allow(dead_code)] +pub struct CaptureHandler(pub tokio::sync::oneshot::Sender<(String, Bytes)>); + +impl NetHandler for CaptureHandler { + fn bytes(self: Box, url: String, b: Bytes) { + let _ = self.0.send((url, b)); + } +} + +#[allow(dead_code)] +#[derive(Default, Clone)] +pub struct CaptureWaker(pub Arc>>); + +impl NetWaker for CaptureWaker { + fn wake(&self, id: usize) { + self.0.lock().unwrap().push(id); + } +} + +#[allow(dead_code)] +pub fn make_url(s: &str) -> url::Url { + url::Url::parse(s).expect("valid url") +} + +#[allow(dead_code)] +pub fn write_tempfile(contents: &[u8]) -> tempfile::NamedTempFile { + let mut tmp = tempfile::NamedTempFile::new().unwrap(); + tmp.write_all(contents).unwrap(); + tmp +} + +#[allow(dead_code)] +pub async fn mount_get_ok(server: &MockServer) { + Mock::given(method("GET")) + .respond_with(ResponseTemplate::new(200)) + .mount(server) + .await; +} + +#[allow(dead_code)] +pub async fn mount_get_body(server: &MockServer, body: &'static str) { + Mock::given(method("GET")) + .respond_with(ResponseTemplate::new(200).set_body_string(body)) + .mount(server) + .await; +} + +#[allow(dead_code)] +pub async fn mount_get_status(server: &MockServer, status: u16) { + Mock::given(method("GET")) + .respond_with(ResponseTemplate::new(status)) + .mount(server) + .await; +} + +#[allow(dead_code)] +pub async fn mount_post_ok(server: &MockServer) { + Mock::given(method("POST")) + .respond_with(ResponseTemplate::new(200)) + .mount(server) + .await; +} + +#[allow(dead_code)] +pub async fn wait_until bool>(timeout: Duration, mut condition: F) -> bool { + let deadline = Instant::now() + timeout; + while Instant::now() < deadline { + if condition() { + return true; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + condition() +} + +#[allow(dead_code)] +pub async fn wait_for_received(server: &MockServer, target: usize, timeout: Duration) -> usize { + let deadline = Instant::now() + timeout; + loop { + let n = server.received_requests().await.unwrap().len(); + if n >= target || Instant::now() >= deadline { + return n; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } +} diff --git a/tests/blitz-net/tests/concurrency.rs b/tests/blitz-net/tests/concurrency.rs new file mode 100644 index 0000000000..30fe7cc82c --- /dev/null +++ b/tests/blitz-net/tests/concurrency.rs @@ -0,0 +1,303 @@ +mod common; + +use blitz_net::Provider; +use blitz_traits::net::{AbortController, NetProvider, Request}; +use common::{ + CaptureHandler, CaptureWaker, RESPONSE_DELAY, make_url, mount_get_ok, wait_for_received, + wait_until, +}; +use futures_util::future::join_all; +use std::sync::Arc; +use std::time::{Duration, Instant}; +use tokio::sync::Notify; +use wiremock::matchers::method; +use wiremock::{Mock, MockServer, Request as WireRequest, Respond, ResponseTemplate}; + +#[tokio::test] +async fn is_empty_true_on_fresh_provider() { + let provider = Provider::new(None); + assert!(provider.is_empty()); +} + +#[tokio::test] +async fn count_zero_on_fresh_provider() { + let provider = Provider::new(None); + assert_eq!(provider.count(), 0); +} + +struct NotifyOnArrival { + notify: Arc, + delay: Duration, +} + +impl Respond for NotifyOnArrival { + fn respond(&self, _req: &WireRequest) -> ResponseTemplate { + self.notify.notify_one(); + ResponseTemplate::new(200).set_delay(self.delay) + } +} + +#[tokio::test(flavor = "multi_thread")] +async fn count_increments_during_in_flight_fetch() { + let arrived = Arc::new(Notify::new()); + let server = MockServer::start().await; + Mock::given(method("GET")) + .respond_with(NotifyOnArrival { + notify: arrived.clone(), + delay: RESPONSE_DELAY, + }) + .mount(&server) + .await; + + let waker = CaptureWaker::default(); + let provider = Provider::new(Some(Arc::new(waker.clone()))); + + let url = make_url(&server.uri()); + let (tx, rx) = tokio::sync::oneshot::channel(); + NetProvider::fetch( + &provider, + 1, + Request::get(url), + Box::new(CaptureHandler(tx)), + ); + + arrived.notified().await; + assert_eq!(provider.count(), 1, "one in-flight fetch"); + assert!(!provider.is_empty()); + + let _ = rx.await; + assert!( + wait_until(Duration::from_secs(1), || provider.is_empty()).await, + "provider should become empty after fetch completes" + ); + assert_eq!(provider.count(), 0); +} + +#[tokio::test(flavor = "multi_thread")] +async fn is_empty_returns_true_after_completion() { + let server = MockServer::start().await; + mount_get_ok(&server).await; + + let waker = CaptureWaker::default(); + let provider = Provider::new(Some(Arc::new(waker))); + + let url = make_url(&server.uri()); + let (tx, rx) = tokio::sync::oneshot::channel(); + NetProvider::fetch( + &provider, + 1, + Request::get(url), + Box::new(CaptureHandler(tx)), + ); + let _ = rx.await; + + assert!( + wait_until(Duration::from_secs(1), || provider.is_empty()).await, + "provider should become empty" + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn per_host_limit_caps_concurrent_requests_at_six() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .respond_with(ResponseTemplate::new(200).set_delay(RESPONSE_DELAY)) + .expect(8) + .mount(&server) + .await; + + let provider = Arc::new(Provider::new(None)); + let base_url = server.uri(); + + let fetches_handle = tokio::spawn({ + let provider = provider.clone(); + async move { + let futs: Vec<_> = (0..8) + .map(|i| { + let p = provider.clone(); + let url = make_url(&format!("{base_url}/?i={i}")); + async move { p.fetch_async(Request::get(url)).await } + }) + .collect(); + join_all(futs).await + } + }); + + // Each response is held for RESPONSE_DELAY, so no permit is released and no + // 7th request can dispatch while we observe — the count we read is the cap, + // not a racy snapshot. + let observed = wait_for_received(&server, 6, Duration::from_secs(2)).await; + assert_eq!( + observed, 6, + "per-host limit should cap in-flight at exactly 6" + ); + + let results = fetches_handle.await.unwrap(); + assert!( + results.iter().all(|r| r.is_ok()), + "all 8 fetches should succeed" + ); + let received_total = server.received_requests().await.unwrap().len(); + assert_eq!( + received_total, 8, + "all 8 requests should eventually be served" + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn per_host_limit_shared_across_ports_for_same_hostname() { + // The semaphore is keyed by url.host_str(), so two mock servers on 127.0.0.1 + // with different ports share a single limiter — combined in-flight stays ≤ 6. + let server_a = MockServer::start().await; + let server_b = MockServer::start().await; + + Mock::given(method("GET")) + .respond_with(ResponseTemplate::new(200).set_delay(RESPONSE_DELAY)) + .mount(&server_a) + .await; + Mock::given(method("GET")) + .respond_with(ResponseTemplate::new(200).set_delay(RESPONSE_DELAY)) + .mount(&server_b) + .await; + + let provider = Arc::new(Provider::new(None)); + let url_a = server_a.uri(); + let url_b = server_b.uri(); + + let fetches_handle = tokio::spawn({ + let provider = provider.clone(); + async move { + let urls: Vec<_> = (0..6) + .map(|i| make_url(&format!("{url_a}/?i={i}"))) + .chain((0..6).map(|i| make_url(&format!("{url_b}/?i={i}")))) + .collect(); + let futs: Vec<_> = urls + .into_iter() + .map(|u| { + let p = provider.clone(); + async move { p.fetch_async(Request::get(u)).await } + }) + .collect(); + join_all(futs).await + } + }); + + let deadline = Instant::now() + Duration::from_secs(2); + let mut combined = 0; + while Instant::now() < deadline { + let a = server_a.received_requests().await.unwrap().len(); + let b = server_b.received_requests().await.unwrap().len(); + combined = a + b; + if combined >= 6 { + break; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + assert_eq!( + combined, 6, + "same hostname → shared semaphore; combined in-flight should equal 6, got {combined}" + ); + + let results = fetches_handle.await.unwrap(); + assert!( + results.iter().all(|r| r.is_ok()), + "all fetches should succeed" + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn per_host_limit_not_shared_across_hostnames() { + // Contrast to `per_host_limit_shared_across_ports_for_same_hostname`: the + // same wiremock addressed via two host strings ("127.0.0.1" and "localhost") + // gets two distinct semaphores, so combined in-flight can exceed 6. + let server = MockServer::start().await; + Mock::given(method("GET")) + .respond_with(ResponseTemplate::new(200).set_delay(RESPONSE_DELAY)) + .mount(&server) + .await; + + let port = server.address().port(); + let url_ip = format!("http://127.0.0.1:{port}"); + let url_local = format!("http://localhost:{port}"); + + let provider = Arc::new(Provider::new(None)); + let fetches_handle = tokio::spawn({ + let provider = provider.clone(); + async move { + let urls: Vec<_> = (0..6) + .map(|i| make_url(&format!("{url_ip}/?i={i}"))) + .chain((0..6).map(|i| make_url(&format!("{url_local}/?i={i}")))) + .collect(); + let futs: Vec<_> = urls + .into_iter() + .map(|u| { + let p = provider.clone(); + async move { p.fetch_async(Request::get(u)).await } + }) + .collect(); + join_all(futs).await + } + }); + + let observed = wait_for_received(&server, 7, Duration::from_secs(2)).await; + assert!( + observed > 6, + "different hostnames should not share a semaphore; only saw {observed} concurrent" + ); + + let results = fetches_handle.await.unwrap(); + assert!( + results.iter().all(|r| r.is_ok()), + "all fetches should succeed" + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn abort_signal_aborted_mid_flight_returns_abort() { + let arrived = Arc::new(Notify::new()); + let server = MockServer::start().await; + Mock::given(method("GET")) + .respond_with(NotifyOnArrival { + notify: arrived.clone(), + delay: RESPONSE_DELAY, + }) + .mount(&server) + .await; + + let waker = CaptureWaker::default(); + let provider = Provider::new(Some(Arc::new(waker.clone()))); + + let controller = AbortController::default(); + let signal = controller.signal.clone(); + + let url = make_url(&server.uri()); + let req = Request::get(url).signal(signal); + let (tx, mut rx) = tokio::sync::oneshot::channel(); + // AbortFetch only wraps the NetProvider::fetch path, not fetch_async or fetch_with_callback. + NetProvider::fetch(&provider, 55, req, Box::new(CaptureHandler(tx))); + + // Wait for the request to land at wiremock so we cancel a real in-flight + // fetch rather than racing dispatch. + arrived.notified().await; + controller.abort(); + + let waker_vec = waker.0.clone(); + assert!( + wait_until(Duration::from_secs(2), || !waker_vec + .lock() + .unwrap() + .is_empty()) + .await, + "waker should fire after the aborted task completes" + ); + + let woken = waker_vec.lock().unwrap().clone(); + assert!( + woken.contains(&55), + "waker should fire with the right doc_id" + ); + assert!( + rx.try_recv().is_err(), + "handler should not be called when aborted" + ); +} diff --git a/tests/blitz-net/tests/features.rs b/tests/blitz-net/tests/features.rs new file mode 100644 index 0000000000..3d77013173 --- /dev/null +++ b/tests/blitz-net/tests/features.rs @@ -0,0 +1,210 @@ +mod common; + +#[cfg(feature = "multipart")] +mod multipart_tests { + use crate::common::{make_url, mount_post_ok, write_tempfile}; + use blitz_net::Provider; + use blitz_traits::net::{Body, Entry, EntryValue, FormData, Request}; + use wiremock::MockServer; + + #[tokio::test] + async fn body_multipart_sends_parts() { + let server = MockServer::start().await; + mount_post_ok(&server).await; + + let url = make_url(&server.uri()); + let form = FormData(vec![Entry { + name: "field".to_string(), + value: EntryValue::String("hello".to_string()), + }]); + let mut req = Request::get(url); + req.method = http::Method::POST; + req.body = Body::Form(form); + req.content_type = Some("multipart/form-data".to_string()); + + let provider = Provider::new(None); + provider + .fetch_async(req) + .await + .expect("multipart POST should succeed"); + + let requests = server.received_requests().await.unwrap(); + let ct = requests[0] + .headers + .get("content-type") + .and_then(|v| v.to_str().ok()) + .unwrap_or_default(); + assert!( + ct.starts_with("multipart/form-data"), + "content-type should be multipart/form-data, got: {ct}" + ); + let body_str = std::str::from_utf8(&requests[0].body).unwrap_or_default(); + assert!( + body_str.contains("hello"), + "multipart body should contain field value" + ); + } + + #[tokio::test] + async fn body_multipart_file_part_reads_from_disk() { + let tmp = write_tempfile(b"file part contents"); + let path = tmp.path().to_path_buf(); + + let server = MockServer::start().await; + mount_post_ok(&server).await; + + let url = make_url(&server.uri()); + let form = FormData(vec![Entry { + name: "upload".to_string(), + value: EntryValue::File(path), + }]); + let mut req = Request::get(url); + req.method = http::Method::POST; + req.body = Body::Form(form); + req.content_type = Some("multipart/form-data".to_string()); + + let provider = Provider::new(None); + provider + .fetch_async(req) + .await + .expect("multipart file POST should succeed"); + + let requests = server.received_requests().await.unwrap(); + let body_str = std::str::from_utf8(&requests[0].body).unwrap_or_default(); + assert!( + body_str.contains("file part contents"), + "multipart body should contain file contents" + ); + } +} + +#[cfg(feature = "cache")] +mod cache_tests { + use crate::common::make_url; + use blitz_net::Provider; + use blitz_traits::net::Request; + use std::sync::OnceLock; + use tokio::sync::Mutex as AsyncMutex; + use wiremock::matchers::method; + use wiremock::{Mock, MockServer, ResponseTemplate}; + + /// `blitz_net::Provider`'s cache lives in a fixed on-disk directory shared + /// across all `Provider` instances in-process. Serialize cache-touching tests + /// so concurrent tests don't observe partially-cleared state. + fn cache_lock() -> &'static AsyncMutex<()> { + static LOCK: OnceLock> = OnceLock::new(); + LOCK.get_or_init(|| AsyncMutex::new(())) + } + + #[tokio::test] + async fn clear_cache_succeeds_on_fresh_provider() { + let _guard = cache_lock().lock().await; + let provider = Provider::new(None); + provider.clear_cache().await; + } + + #[tokio::test] + async fn second_fetch_served_from_cache_and_clear_cache_invalidates_entries() { + let _guard = cache_lock().lock().await; + let server = MockServer::start().await; + Mock::given(method("GET")) + .respond_with( + ResponseTemplate::new(200) + .insert_header("cache-control", "max-age=3600") + .set_body_string("cached"), + ) + .mount(&server) + .await; + + let url = make_url(&server.uri()); + let provider = Provider::new(None); + provider.clear_cache().await; + + let (_, b1) = provider + .fetch_async(Request::get(url.clone())) + .await + .expect("first fetch should succeed"); + assert_eq!(b1.as_ref(), b"cached"); + + let (_, b2) = provider + .fetch_async(Request::get(url.clone())) + .await + .expect("second fetch should succeed"); + assert_eq!(b2.as_ref(), b"cached"); + + let count_after_two = server.received_requests().await.unwrap().len(); + assert_eq!( + count_after_two, 1, + "second request should be served from cache" + ); + + provider.clear_cache().await; + + provider + .fetch_async(Request::get(url)) + .await + .expect("fetch after clear"); + + let count_after_clear = server.received_requests().await.unwrap().len(); + assert_eq!( + count_after_clear, 2, + "after clearing cache, next fetch should hit network" + ); + } +} + +#[cfg(feature = "cookies")] +mod cookie_tests { + use crate::common::make_url; + use blitz_net::Provider; + use blitz_traits::net::Request; + use wiremock::matchers::method; + use wiremock::{Mock, MockServer, ResponseTemplate}; + + #[tokio::test] + async fn cookie_jar_persists_set_cookie_across_requests() { + let server = MockServer::start().await; + + Mock::given(method("GET")) + .and(wiremock::matchers::path("/set")) + .respond_with( + ResponseTemplate::new(200).insert_header("set-cookie", "session=abc123; Path=/"), + ) + .mount(&server) + .await; + + Mock::given(method("GET")) + .and(wiremock::matchers::path("/check")) + .respond_with(ResponseTemplate::new(200).set_body_string("ok")) + .mount(&server) + .await; + + let base = server.uri(); + let provider = Provider::new(None); + + provider + .fetch_async(Request::get(make_url(&format!("{base}/set")))) + .await + .expect("set-cookie request should succeed"); + + provider + .fetch_async(Request::get(make_url(&format!("{base}/check")))) + .await + .expect("cookie check request should succeed"); + + let requests = server.received_requests().await.unwrap(); + let check_req = requests + .iter() + .find(|r| r.url.path() == "/check") + .expect("should have /check request"); + let cookie_header = check_req + .headers + .get("cookie") + .and_then(|v| v.to_str().ok()) + .unwrap_or_default(); + assert!( + cookie_header.contains("session=abc123"), + "cookie should be sent on subsequent request, got: '{cookie_header}'" + ); + } +} diff --git a/tests/blitz-net/tests/http_behavior.rs b/tests/blitz-net/tests/http_behavior.rs new file mode 100644 index 0000000000..4c481598db --- /dev/null +++ b/tests/blitz-net/tests/http_behavior.rs @@ -0,0 +1,354 @@ +mod common; + +use blitz_net::{Provider, ProviderError}; +use blitz_traits::net::{Body, Entry, EntryValue, FormData, NetProvider, Request}; +use bytes::Bytes; +use common::{ + CaptureHandler, CaptureWaker, make_url, mount_get_body, mount_get_ok, mount_get_status, + mount_post_ok, wait_until, +}; +use std::sync::Arc; +use std::time::Duration; +use wiremock::matchers::method; +use wiremock::{Mock, MockServer, ResponseTemplate}; + +#[tokio::test] +async fn injects_user_agent_header() { + let server = MockServer::start().await; + mount_get_ok(&server).await; + + let url = make_url(&server.uri()); + let provider = Provider::new(None); + provider + .fetch_async(Request::get(url)) + .await + .expect("request should succeed"); + + let requests = server.received_requests().await.unwrap(); + let ua = requests[0] + .headers + .get("user-agent") + .and_then(|v| v.to_str().ok()) + .unwrap_or_default(); + assert!( + ua.starts_with("Mozilla/5.0"), + "user-agent should be set, got: '{ua}'" + ); +} + +#[tokio::test] +async fn forwards_request_headers() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(wiremock::matchers::header("x-custom", "myvalue")) + .respond_with(ResponseTemplate::new(200)) + .mount(&server) + .await; + + let url = make_url(&server.uri()); + let mut req = Request::get(url); + req.headers + .insert("x-custom", http::HeaderValue::from_static("myvalue")); + + let provider = Provider::new(None); + let result = provider.fetch_async(req).await; + assert!(result.is_ok(), "forwarded header should match: {result:?}"); +} + +#[tokio::test] +async fn forwards_method_post() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .respond_with(ResponseTemplate::new(200).set_body_string("posted")) + .mount(&server) + .await; + + let url = make_url(&server.uri()); + let mut req = Request::get(url); + req.method = http::Method::POST; + + let provider = Provider::new(None); + let (_, bytes) = provider + .fetch_async(req) + .await + .expect("POST should succeed"); + assert_eq!(bytes.as_ref(), b"posted"); +} + +#[tokio::test] +async fn body_empty_sends_no_body() { + let server = MockServer::start().await; + mount_post_ok(&server).await; + + let url = make_url(&server.uri()); + let mut req = Request::get(url); + req.method = http::Method::POST; + req.body = Body::Empty; + + let provider = Provider::new(None); + let result = provider.fetch_async(req).await; + assert!(result.is_ok()); + + let requests = server.received_requests().await.unwrap(); + let body = requests[0].body.clone(); + assert!(body.is_empty(), "empty body sends no bytes"); +} + +#[tokio::test] +async fn body_bytes_sends_raw_payload() { + let server = MockServer::start().await; + mount_post_ok(&server).await; + + let url = make_url(&server.uri()); + let mut req = Request::get(url); + req.method = http::Method::POST; + req.body = Body::Bytes(Bytes::from_static(b"raw payload")); + + let provider = Provider::new(None); + provider + .fetch_async(req) + .await + .expect("body bytes should send"); + + let requests = server.received_requests().await.unwrap(); + assert_eq!(requests[0].body, b"raw payload"); +} + +#[tokio::test] +async fn body_form_urlencoded_sends_encoded() { + let server = MockServer::start().await; + mount_post_ok(&server).await; + + let url = make_url(&server.uri()); + let form = FormData(vec![Entry { + name: "key".to_string(), + value: EntryValue::String("value".to_string()), + }]); + let mut req = Request::get(url); + req.method = http::Method::POST; + req.body = Body::Form(form); + req.content_type = Some("application/x-www-form-urlencoded".to_string()); + + let provider = Provider::new(None); + provider + .fetch_async(req) + .await + .expect("url-encoded form should send"); + + let requests = server.received_requests().await.unwrap(); + let body_str = std::str::from_utf8(&requests[0].body).unwrap(); + assert!( + body_str.contains("key=value"), + "body should contain encoded form: {body_str}" + ); +} + +#[tokio::test] +async fn content_type_header_set_when_provided() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(wiremock::matchers::header( + "content-type", + "application/json", + )) + .respond_with(ResponseTemplate::new(200)) + .mount(&server) + .await; + + let url = make_url(&server.uri()); + let mut req = Request::get(url); + req.method = http::Method::POST; + req.content_type = Some("application/json".to_string()); + req.body = Body::Bytes(Bytes::from_static(b"{}")); + + let provider = Provider::new(None); + let result = provider.fetch_async(req).await; + assert!( + result.is_ok(), + "content-type header should be forwarded: {result:?}" + ); +} + +#[tokio::test] +async fn fetch_with_callback_invokes_callback_on_ok() { + let server = MockServer::start().await; + mount_get_body(&server, "callback ok").await; + + let url = make_url(&server.uri()); + let (tx, rx) = tokio::sync::oneshot::channel(); + let provider = Provider::new(None); + provider.fetch_with_callback( + Request::get(url), + Box::new(move |result| { + let _ = tx.send(result); + }), + ); + + let result = rx.await.expect("callback should be invoked"); + let (_, bytes) = result.expect("callback result should be ok"); + assert_eq!(bytes.as_ref(), b"callback ok"); +} + +#[tokio::test] +async fn fetch_with_callback_invokes_callback_on_404() { + let server = MockServer::start().await; + mount_get_status(&server, 404).await; + + let url = make_url(&server.uri()); + let (tx, rx) = tokio::sync::oneshot::channel(); + let provider = Provider::new(None); + provider.fetch_with_callback( + Request::get(url), + Box::new(move |result| { + let _ = tx.send(result); + }), + ); + + let result = rx.await.expect("callback should be invoked"); + let err = result.expect_err("callback result should be error on 404"); + assert!( + matches!(err, ProviderError::HttpStatus { status, .. } if status.as_u16() == 404), + "expected HttpStatus 404, got: {err}" + ); +} + +#[tokio::test] +async fn net_provider_trait_fetch_delivers_bytes_to_handler() { + let server = MockServer::start().await; + mount_get_body(&server, "handler bytes").await; + + let waker = CaptureWaker::default(); + let provider = Provider::new(Some(Arc::new(waker.clone()))); + + let url = make_url(&server.uri()); + let (tx, rx) = tokio::sync::oneshot::channel(); + let handler = CaptureHandler(tx); + + NetProvider::fetch(&provider, 42, Request::get(url), Box::new(handler)); + + let (_, bytes) = rx.await.expect("handler should be called"); + assert_eq!(bytes.as_ref(), b"handler bytes"); +} + +#[tokio::test] +async fn net_provider_trait_fetch_skips_handler_on_error() { + let server = MockServer::start().await; + mount_get_status(&server, 500).await; + + let waker = CaptureWaker::default(); + let provider = Provider::new(Some(Arc::new(waker.clone()))); + + let url = make_url(&server.uri()); + let (tx, mut rx) = tokio::sync::oneshot::channel(); + let handler = CaptureHandler(tx); + + NetProvider::fetch(&provider, 99, Request::get(url), Box::new(handler)); + + // The waker fires when the fetch task completes (success or error). Once it + // has fired, the handler dispatch decision has been made — if the impl is + // correct, the handler is skipped on error and rx remains empty. + let waker_vec = waker.0.clone(); + assert!( + wait_until(Duration::from_secs(2), || waker_vec + .lock() + .unwrap() + .contains(&99)) + .await, + "waker should fire after error" + ); + + assert!( + rx.try_recv().is_err(), + "handler should not be called on error" + ); +} + +#[tokio::test] +async fn net_provider_trait_wakes_waker_with_doc_id() { + let server = MockServer::start().await; + mount_get_ok(&server).await; + + let waker = CaptureWaker::default(); + let provider = Provider::new(Some(Arc::new(waker.clone()))); + + let url = make_url(&server.uri()); + let (tx, rx) = tokio::sync::oneshot::channel(); + let handler = CaptureHandler(tx); + NetProvider::fetch(&provider, 77, Request::get(url), Box::new(handler)); + + let _ = rx.await; + + let woken = waker.0.lock().unwrap().clone(); + assert!(woken.contains(&77), "waker should be called with doc_id=77"); +} + +#[tokio::test] +async fn shared_returns_arc_dyn_netprovider() { + let provider: Arc = Provider::shared(None); + let server = MockServer::start().await; + mount_get_body(&server, "shared").await; + + let url = make_url(&server.uri()); + let (tx, rx) = tokio::sync::oneshot::channel(); + let handler = CaptureHandler(tx); + provider.fetch(1, Request::get(url), Box::new(handler)); + + rx.await.expect("shared provider should work"); +} + +// Display strings are part of blitz-net's public surface; variant identity is covered by +// schemes.rs. Asserting Display rather than variant also keeps the test stable under +// workspace feature unification (e.g., `cache` wrapping reqwest errors in +// reqwest-middleware when apps/browser pulls the feature in transitively). +#[tokio::test] +async fn provider_error_display_renders_each_variant() { + let provider = Provider::new(None); + + let display = provider + .fetch_async(Request::get(make_url("file:///nonexistent/path.txt"))) + .await + .expect_err("missing file") + .to_string(); + assert!(!display.is_empty(), "Io display empty"); + + let display = provider + .fetch_async(Request::get(make_url("data:not valid"))) + .await + .expect_err("invalid data url") + .to_string(); + assert!(!display.is_empty(), "DataUrl display empty"); + + let display = provider + .fetch_async(Request::get(make_url("data:text/plain;base64,!!!"))) + .await + .expect_err("invalid base64") + .to_string(); + assert!(!display.is_empty(), "DataUrlBase64 display empty"); + + let display = provider + .fetch_async(Request::get(make_url( + "http://intentionally-nonexistent-host.invalid/", + ))) + .await + .expect_err("dns failure") + .to_string(); + assert!( + display.starts_with("reqwest error:") || display.starts_with("reqwest middleware error:"), + "reqwest/middleware display: {display}" + ); + + let server = MockServer::start().await; + mount_get_status(&server, 404).await; + let display = provider + .fetch_async(Request::get(make_url(&server.uri()))) + .await + .expect_err("404") + .to_string(); + assert!(display.contains("404"), "HttpStatus display: {display}"); + assert!( + display.contains(&server.uri()), + "HttpStatus display missing url: {display}" + ); + + assert!(ProviderError::Abort.to_string().contains("aborted")); +} diff --git a/tests/blitz-net/tests/schemes.rs b/tests/blitz-net/tests/schemes.rs new file mode 100644 index 0000000000..3ba6b4a7d4 --- /dev/null +++ b/tests/blitz-net/tests/schemes.rs @@ -0,0 +1,166 @@ +mod common; + +use blitz_net::{Provider, ProviderError}; +use blitz_traits::net::Request; +use common::{make_url, mount_get_body, mount_get_status, write_tempfile}; +use wiremock::matchers::method; +use wiremock::{Mock, MockServer, ResponseTemplate}; + +#[tokio::test] +async fn data_url_plain_decodes() { + let provider = Provider::new(None); + let url = make_url("data:text/plain,hello%20world"); + let (_, bytes) = provider + .fetch_async(Request::get(url)) + .await + .expect("data url should decode"); + assert_eq!(bytes.as_ref(), b"hello world"); +} + +#[tokio::test] +async fn data_url_base64_decodes() { + let provider = Provider::new(None); + let url = make_url("data:text/plain;base64,aGVsbG8="); + let (_, bytes) = provider + .fetch_async(Request::get(url)) + .await + .expect("base64 data url should decode"); + assert_eq!(bytes.as_ref(), b"hello"); +} + +#[tokio::test] +async fn data_url_invalid_returns_data_url_error() { + let provider = Provider::new(None); + let url = make_url("data:not valid at all without comma"); + let err = provider + .fetch_async(Request::get(url)) + .await + .expect_err("invalid data url should error"); + assert!( + matches!(err, ProviderError::DataUrl(_)), + "expected DataUrl error, got: {err}" + ); +} + +#[tokio::test] +async fn data_url_invalid_base64_returns_base64_error() { + let provider = Provider::new(None); + let url = make_url("data:text/plain;base64,!!!invalid!!!"); + let err = provider + .fetch_async(Request::get(url)) + .await + .expect_err("invalid base64 should error"); + assert!( + matches!(err, ProviderError::DataUrlBase64(_)), + "expected DataUrlBase64 error, got: {err}" + ); +} + +#[tokio::test] +async fn file_url_reads_existing_file() { + let tmp = write_tempfile(b"file content"); + let url = url::Url::from_file_path(tmp.path()).expect("path → file url"); + + let provider = Provider::new(None); + let (_, bytes) = provider + .fetch_async(Request::get(url)) + .await + .expect("existing file should be readable"); + assert_eq!(bytes.as_ref(), b"file content"); +} + +#[tokio::test] +async fn file_url_missing_returns_io_error() { + let url = make_url("file:///nonexistent/path/that/does/not/exist.txt"); + let provider = Provider::new(None); + let err = provider + .fetch_async(Request::get(url)) + .await + .expect_err("missing file should error"); + assert!( + matches!(err, ProviderError::Io(_)), + "expected Io error, got: {err}" + ); +} + +#[tokio::test] +async fn http_200_returns_bytes_and_resolved_url() { + let server = MockServer::start().await; + mount_get_body(&server, "ok").await; + + let url = make_url(&server.uri()); + let provider = Provider::new(None); + let (resolved, bytes) = provider + .fetch_async(Request::get(url.clone())) + .await + .expect("200 should succeed"); + assert_eq!(bytes.as_ref(), b"ok"); + assert!( + resolved.starts_with(&server.uri()), + "resolved url should reference the mock server, got: {resolved}" + ); +} + +#[tokio::test] +async fn http_404_returns_http_status_error() { + let server = MockServer::start().await; + mount_get_status(&server, 404).await; + + let url = make_url(&server.uri()); + let provider = Provider::new(None); + let err = provider + .fetch_async(Request::get(url)) + .await + .expect_err("404 should error"); + assert!( + matches!(err, ProviderError::HttpStatus { status, .. } if status.as_u16() == 404), + "expected HttpStatus 404, got: {err}" + ); +} + +#[tokio::test] +async fn http_500_returns_http_status_error() { + let server = MockServer::start().await; + mount_get_status(&server, 500).await; + + let url = make_url(&server.uri()); + let provider = Provider::new(None); + let err = provider + .fetch_async(Request::get(url)) + .await + .expect_err("500 should error"); + assert!( + matches!(err, ProviderError::HttpStatus { status, .. } if status.as_u16() == 500), + "expected HttpStatus 500, got: {err}" + ); +} + +#[tokio::test] +async fn http_redirect_resolved_url_is_final() { + let server = MockServer::start().await; + let final_url = format!("{}/final", server.uri()); + + Mock::given(method("GET")) + .and(wiremock::matchers::path("/")) + .respond_with(ResponseTemplate::new(301).insert_header("location", final_url.as_str())) + .mount(&server) + .await; + + Mock::given(method("GET")) + .and(wiremock::matchers::path("/final")) + .respond_with(ResponseTemplate::new(200).set_body_string("final")) + .mount(&server) + .await; + + let url = make_url(&server.uri()); + let provider = Provider::new(None); + let (resolved, bytes) = provider + .fetch_async(Request::get(url)) + .await + .expect("redirect should follow and succeed"); + assert_eq!(bytes.as_ref(), b"final"); + assert!( + resolved.ends_with("/final"), + "resolved url should end at /final, got: {resolved}" + ); +} From 5cabab68fa9b92fe0626b0f2795790327b8a43a7 Mon Sep 17 00:00:00 2001 From: Tony Bierman Date: Thu, 14 May 2026 05:37:13 -0500 Subject: [PATCH 2/3] Expand blitz-net test coverage and isolate cache tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add Provider::with_cache_dir(waker, dir) under the cache feature so tests can point at a TempDir instead of the production cache directory shared with the running app. Cache tests now each use an isolated dir; the OnceLock serialization is dropped since concurrency no longer matters when state is per-test. Redirect coverage: loop detection, 307 POST method preservation, 302 POST→GET (the last pins reqwest's browser-compat behavior so future drift surfaces). Abort coverage: pre-dispatch abort returns immediately with handler skipped, abort-after-completion is a no-op, single AbortController shared across two in-flight requests aborts both. CI adds a Windows job running cargo test -p blitz-net-tests to verify the file:// handling fix on the platform it targets. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/ci.yml | 9 ++ packages/blitz-net/src/lib.rs | 30 +++++-- tests/blitz-net/tests/concurrency.rs | 121 +++++++++++++++++++++++++++ tests/blitz-net/tests/features.rs | 19 +---- tests/blitz-net/tests/schemes.rs | 98 +++++++++++++++++++++- 5 files changed, 254 insertions(+), 23 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a8d25b8dbb..e63b73fbe9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -87,6 +87,15 @@ jobs: - run: cargo test -p blitz-net-tests --features cookies,cache - run: cargo test -p blitz-net-tests --features cookies,multipart + test-blitz-net-windows: + name: "Test blitz-net (Windows)" + runs-on: windows-latest + steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@stable + - uses: Swatinem/rust-cache@v2 + - run: cargo test -p blitz-net-tests + build-counter: name: "Build counter example" runs-on: ubuntu-latest diff --git a/packages/blitz-net/src/lib.rs b/packages/blitz-net/src/lib.rs index 981f0aa750..d604ecb483 100644 --- a/packages/blitz-net/src/lib.rs +++ b/packages/blitz-net/src/lib.rs @@ -70,15 +70,21 @@ pub struct Provider { } impl Provider { pub fn new(waker: Option>) -> Self { + #[cfg(feature = "cache")] + return Self::with_cache_dir(waker, get_cache_path()); + + #[cfg(not(feature = "cache"))] + Self::build(waker) + } + + #[cfg(feature = "cache")] + pub fn with_cache_dir(waker: Option>, cache_dir: std::path::PathBuf) -> Self { let builder = reqwest::Client::builder(); #[cfg(feature = "cookies")] let builder = builder.cookie_store(true); let client = builder.build().unwrap(); - #[cfg(feature = "cache")] - let cache_manager = CACacheManager::new(get_cache_path(), true); - - #[cfg(feature = "cache")] + let cache_manager = CACacheManager::new(cache_dir, true); let client = reqwest_middleware::ClientBuilder::new(client) .with(Cache(HttpCache { mode: CacheMode::Default, @@ -92,10 +98,24 @@ impl Provider { client, waker, per_host_limits: Arc::new(Mutex::new(HashMap::new())), - #[cfg(feature = "cache")] cache_manager, } } + + #[cfg(not(feature = "cache"))] + fn build(waker: Option>) -> Self { + let builder = reqwest::Client::builder(); + #[cfg(feature = "cookies")] + let builder = builder.cookie_store(true); + let client = builder.build().unwrap(); + + let waker = waker.unwrap_or(Arc::new(DummyNetWaker)); + Self { + client, + waker, + per_host_limits: Arc::new(Mutex::new(HashMap::new())), + } + } pub fn shared(waker: Option>) -> Arc { Arc::new(Self::new(waker)) } diff --git a/tests/blitz-net/tests/concurrency.rs b/tests/blitz-net/tests/concurrency.rs index 30fe7cc82c..af7b93360b 100644 --- a/tests/blitz-net/tests/concurrency.rs +++ b/tests/blitz-net/tests/concurrency.rs @@ -252,6 +252,127 @@ async fn per_host_limit_not_shared_across_hostnames() { ); } +#[tokio::test(flavor = "multi_thread")] +async fn abort_before_dispatch_returns_abort_immediately() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .respond_with(ResponseTemplate::new(200).set_delay(RESPONSE_DELAY)) + .mount(&server) + .await; + + let waker = CaptureWaker::default(); + let provider = Provider::new(Some(Arc::new(waker.clone()))); + + let controller = AbortController::default(); + let signal = controller.signal.clone(); + controller.abort(); + + let url = make_url(&server.uri()); + let req = Request::get(url).signal(signal); + let (tx, mut rx) = tokio::sync::oneshot::channel(); + NetProvider::fetch(&provider, 77, req, Box::new(CaptureHandler(tx))); + + let waker_vec = waker.0.clone(); + assert!( + wait_until(Duration::from_secs(2), || !waker_vec + .lock() + .unwrap() + .is_empty()) + .await, + "waker should fire even when pre-aborted" + ); + + let woken = waker_vec.lock().unwrap().clone(); + assert!(woken.contains(&77), "waker should fire with doc_id 77"); + assert!( + rx.try_recv().is_err(), + "handler should not be called when pre-aborted" + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn abort_after_completion_is_noop() { + let server = MockServer::start().await; + mount_get_ok(&server).await; + + let waker = CaptureWaker::default(); + let provider = Provider::new(Some(Arc::new(waker))); + + let controller = AbortController::default(); + let signal = controller.signal.clone(); + + let url = make_url(&server.uri()); + let req = Request::get(url).signal(signal); + let (tx, rx) = tokio::sync::oneshot::channel(); + NetProvider::fetch(&provider, 88, req, Box::new(CaptureHandler(tx))); + + let _ = rx.await; + controller.abort(); + + assert!( + wait_until(Duration::from_secs(1), || provider.is_empty()).await, + "provider should become empty after fetch completes" + ); + assert!(provider.is_empty()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn same_signal_shared_across_requests() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .respond_with(ResponseTemplate::new(200).set_delay(RESPONSE_DELAY)) + .mount(&server) + .await; + + let waker = CaptureWaker::default(); + let provider = Provider::new(Some(Arc::new(waker.clone()))); + + let controller = AbortController::default(); + let signal_a = controller.signal.clone(); + let signal_b = controller.signal.clone(); + + let base = server.uri(); + let url_a = make_url(&format!("{base}/?r=a")); + let url_b = make_url(&format!("{base}/?r=b")); + + let (tx_a, mut rx_a) = tokio::sync::oneshot::channel(); + let (tx_b, mut rx_b) = tokio::sync::oneshot::channel(); + + NetProvider::fetch( + &provider, + 91, + Request::get(url_a).signal(signal_a), + Box::new(CaptureHandler(tx_a)), + ); + NetProvider::fetch( + &provider, + 92, + Request::get(url_b).signal(signal_b), + Box::new(CaptureHandler(tx_b)), + ); + + controller.abort(); + + let waker_vec = waker.0.clone(); + assert!( + wait_until(Duration::from_secs(3), || { + let v = waker_vec.lock().unwrap(); + v.contains(&91) && v.contains(&92) + }) + .await, + "both wakers should fire after shared-signal abort" + ); + + assert!( + rx_a.try_recv().is_err(), + "handler for request 91 should not be called" + ); + assert!( + rx_b.try_recv().is_err(), + "handler for request 92 should not be called" + ); +} + #[tokio::test(flavor = "multi_thread")] async fn abort_signal_aborted_mid_flight_returns_abort() { let arrived = Arc::new(Notify::new()); diff --git a/tests/blitz-net/tests/features.rs b/tests/blitz-net/tests/features.rs index 3d77013173..38dbdbe312 100644 --- a/tests/blitz-net/tests/features.rs +++ b/tests/blitz-net/tests/features.rs @@ -83,29 +83,19 @@ mod cache_tests { use crate::common::make_url; use blitz_net::Provider; use blitz_traits::net::Request; - use std::sync::OnceLock; - use tokio::sync::Mutex as AsyncMutex; use wiremock::matchers::method; use wiremock::{Mock, MockServer, ResponseTemplate}; - /// `blitz_net::Provider`'s cache lives in a fixed on-disk directory shared - /// across all `Provider` instances in-process. Serialize cache-touching tests - /// so concurrent tests don't observe partially-cleared state. - fn cache_lock() -> &'static AsyncMutex<()> { - static LOCK: OnceLock> = OnceLock::new(); - LOCK.get_or_init(|| AsyncMutex::new(())) - } - #[tokio::test] async fn clear_cache_succeeds_on_fresh_provider() { - let _guard = cache_lock().lock().await; - let provider = Provider::new(None); + let tmp = tempfile::TempDir::new().unwrap(); + let provider = Provider::with_cache_dir(None, tmp.path().to_path_buf()); provider.clear_cache().await; } #[tokio::test] async fn second_fetch_served_from_cache_and_clear_cache_invalidates_entries() { - let _guard = cache_lock().lock().await; + let tmp = tempfile::TempDir::new().unwrap(); let server = MockServer::start().await; Mock::given(method("GET")) .respond_with( @@ -117,8 +107,7 @@ mod cache_tests { .await; let url = make_url(&server.uri()); - let provider = Provider::new(None); - provider.clear_cache().await; + let provider = Provider::with_cache_dir(None, tmp.path().to_path_buf()); let (_, b1) = provider .fetch_async(Request::get(url.clone())) diff --git a/tests/blitz-net/tests/schemes.rs b/tests/blitz-net/tests/schemes.rs index 3ba6b4a7d4..58219b1491 100644 --- a/tests/blitz-net/tests/schemes.rs +++ b/tests/blitz-net/tests/schemes.rs @@ -3,7 +3,7 @@ mod common; use blitz_net::{Provider, ProviderError}; use blitz_traits::net::Request; use common::{make_url, mount_get_body, mount_get_status, write_tempfile}; -use wiremock::matchers::method; +use wiremock::matchers::{method, path}; use wiremock::{Mock, MockServer, ResponseTemplate}; #[tokio::test] @@ -141,13 +141,13 @@ async fn http_redirect_resolved_url_is_final() { let final_url = format!("{}/final", server.uri()); Mock::given(method("GET")) - .and(wiremock::matchers::path("/")) + .and(path("/")) .respond_with(ResponseTemplate::new(301).insert_header("location", final_url.as_str())) .mount(&server) .await; Mock::given(method("GET")) - .and(wiremock::matchers::path("/final")) + .and(path("/final")) .respond_with(ResponseTemplate::new(200).set_body_string("final")) .mount(&server) .await; @@ -164,3 +164,95 @@ async fn http_redirect_resolved_url_is_final() { "resolved url should end at /final, got: {resolved}" ); } + +#[tokio::test] +async fn http_redirect_loop_returns_error() { + let server = MockServer::start().await; + let loop_url = format!("{}/loop", server.uri()); + let root_url = server.uri(); + + Mock::given(method("GET")) + .and(path("/")) + .respond_with(ResponseTemplate::new(302).insert_header("location", loop_url.as_str())) + .mount(&server) + .await; + + Mock::given(method("GET")) + .and(path("/loop")) + .respond_with(ResponseTemplate::new(302).insert_header("location", root_url.as_str())) + .mount(&server) + .await; + + let url = make_url(&server.uri()); + let provider = Provider::new(None); + let err = provider + .fetch_async(Request::get(url)) + .await + .expect_err("redirect loop should error"); + let msg = err.to_string().to_lowercase(); + assert!( + msg.contains("redirect") || msg.contains("too many"), + "expected redirect loop error, got: {err}" + ); +} + +#[tokio::test] +async fn http_307_preserves_post_method() { + let server = MockServer::start().await; + let end_url = format!("{}/end", server.uri()); + + Mock::given(method("POST")) + .and(path("/start")) + .respond_with(ResponseTemplate::new(307).insert_header("location", end_url.as_str())) + .mount(&server) + .await; + + Mock::given(method("POST")) + .and(path("/end")) + .respond_with(ResponseTemplate::new(200).set_body_string("ok")) + .mount(&server) + .await; + + let start_url = make_url(&format!("{}/start", server.uri())); + let mut req = Request::get(start_url); + req.method = http::Method::POST; + + let provider = Provider::new(None); + let (_, bytes) = provider + .fetch_async(req) + .await + .expect("307 POST should preserve method and succeed"); + assert_eq!(bytes.as_ref(), b"ok"); +} + +// 302 with POST: reqwest (following browser compat) converts to GET. +// This test pins the current behavior so any future change is visible. +#[tokio::test] +async fn http_302_post_redirect_method() { + let server = MockServer::start().await; + let end_url = format!("{}/end", server.uri()); + + Mock::given(method("POST")) + .and(path("/start")) + .respond_with(ResponseTemplate::new(302).insert_header("location", end_url.as_str())) + .mount(&server) + .await; + + // reqwest converts POST→GET on 302; mount GET so the redirected request matches. + Mock::given(method("GET")) + .and(path("/end")) + .respond_with(ResponseTemplate::new(200).set_body_string("ok")) + .mount(&server) + .await; + + let start_url = make_url(&format!("{}/start", server.uri())); + let mut req = Request::get(start_url); + req.method = http::Method::POST; + + let provider = Provider::new(None); + let (_, bytes) = provider + .fetch_async(req) + .await + .expect("302 POST redirect should succeed after method conversion"); + assert_eq!(bytes.as_ref(), b"ok"); +} From dfec0ec7f7dc80b0733a72bb4ef8c31233503408 Mon Sep 17 00:00:00 2001 From: Tony Bierman Date: Thu, 14 May 2026 05:49:03 -0500 Subject: [PATCH 3/3] Fix flaky redirect-loop test on macOS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two-mock arrangement (path("/") + path("/loop") cycling) returned Ok((url, b"ok")) on macOS — body that doesn't come from either mock, suggesting cross-test response bleed in wiremock under macOS parallelism. Rewrite as a single catch-all GET mock that 302s every request to /loop. reqwest follows up to its limit then errors with TooManyRedirects. No path matching, no second mock to time-race. Also panic with the unexpected Ok value so any future regression surfaces with the body content. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/blitz-net/tests/schemes.rs | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/tests/blitz-net/tests/schemes.rs b/tests/blitz-net/tests/schemes.rs index 58219b1491..55840dce76 100644 --- a/tests/blitz-net/tests/schemes.rs +++ b/tests/blitz-net/tests/schemes.rs @@ -165,30 +165,25 @@ async fn http_redirect_resolved_url_is_final() { ); } +// Single catch-all mock that 302s to itself: every GET, regardless of path, redirects +// to /loop, which itself 302s back. Avoids the two-mock + path("/") arrangement which +// flaked on macOS with apparent cross-test response bleed. #[tokio::test] async fn http_redirect_loop_returns_error() { let server = MockServer::start().await; - let loop_url = format!("{}/loop", server.uri()); - let root_url = server.uri(); + let target = format!("{}/loop", server.uri()); Mock::given(method("GET")) - .and(path("/")) - .respond_with(ResponseTemplate::new(302).insert_header("location", loop_url.as_str())) - .mount(&server) - .await; - - Mock::given(method("GET")) - .and(path("/loop")) - .respond_with(ResponseTemplate::new(302).insert_header("location", root_url.as_str())) + .respond_with(ResponseTemplate::new(302).insert_header("location", target.as_str())) .mount(&server) .await; - let url = make_url(&server.uri()); let provider = Provider::new(None); - let err = provider - .fetch_async(Request::get(url)) - .await - .expect_err("redirect loop should error"); + let result = provider.fetch_async(Request::get(make_url(&target))).await; + let err = match result { + Ok((url, body)) => panic!("redirect loop should error; got Ok(url={url}, body={body:?})"), + Err(e) => e, + }; let msg = err.to_string().to_lowercase(); assert!( msg.contains("redirect") || msg.contains("too many"),