test: standardize no-proxy localhost HTTP clients

This commit is contained in:
Bryan Helmkamp 2026-04-05 12:17:29 -04:00
parent e8ab762984
commit d644ca1e95
No known key found for this signature in database
18 changed files with 87 additions and 51 deletions

View file

@ -107,3 +107,5 @@ Never run `cargo insta accept` without first checking what's pending — it acce
- `fabro run <name>` — run a workflow by name (resolves `fabro/workflows/<name>/workflow.toml`), e.g. `fabro run repl`
- Use `--no-retro` to skip the retro step and finish faster
- `#[e2e_test(twin, live("VAR"))]` — dual-mode test that runs against twin-openai or real API. `#[e2e_test(twin)]` for twin-only tests (e.g., scripted failures). `#[e2e_test(live("VAR"))]` for live-only tests requiring secrets. `#[e2e_test()]` for sandbox tests with no API deps. Behavior is controlled by `FABRO_TEST_MODE` (`live`, `strict`; default is `twin`), and `cargo nextest run --profile e2e ...` implies `strict`. Use `fabro_test::e2e_openai!()` in twin/dual-mode tests to get `(base_url, api_key)`.
- Local test HTTP clients must use `.no_proxy()`. Prefer shared helpers like `fabro_test::test_http_client()` or crate-local equivalents instead of `reqwest::Client::new()`, bare `Client::builder().build()`, or `reqwest::get(...)`.
- This is not cosmetic: macOS proxy discovery adds hidden startup overhead to repeated localhost reqwest clients and can surface as misleading nextest timeouts.

View file

@ -227,6 +227,7 @@ Shared integration-test helpers may:
- normalize output
- compact structured events
- poll for stable command-created conditions
- centralize localhost HTTP client construction
Shared integration-test helpers should not:
@ -234,6 +235,20 @@ Shared integration-test helpers should not:
- write runtime files the engine is supposed to own
- hide broad scenario setup behind opaque helper functions
### Local HTTP clients
When test code talks to a local server or twin over HTTP, always create the client through a shared test helper that calls `.no_proxy()`.
Do not open-code localhost clients with:
- `reqwest::Client::new()`
- bare `Client::builder().build()`
- `reqwest::get(...)`
Use a crate-local helper or a shared helper such as `fabro_test::test_http_client()` instead.
This rule exists because macOS proxy discovery adds hidden startup overhead to repeated reqwest client creation. The result looks like random nextest timeouts even when the server under test is only talking to `127.0.0.1`.
### Fixtures
Prefer checked-in fixtures when they express a reusable workflow or scenario shape.

View file

@ -168,17 +168,13 @@ async fn enable_auto_merge_persists() {
.unwrap();
let jwt = sign_app_jwt(&creds.app_id, &creds.private_key_pem).unwrap();
let token = create_installation_access_token_for_pr(
&reqwest::Client::new(),
&jwt,
"acme",
"widgets",
&twin.base_url,
)
.await
.unwrap();
let client = fabro_test::test_http_client();
let token =
create_installation_access_token_for_pr(&client, &jwt, "acme", "widgets", &twin.base_url)
.await
.unwrap();
let detail: serde_json::Value = reqwest::Client::new()
let detail: serde_json::Value = fabro_test::test_http_client()
.get(format!(
"{}/repos/acme/widgets/pulls/{}",
twin.base_url, created.number

View file

@ -79,7 +79,7 @@ async fn openai_server_error() {
.strip_suffix("/v1")
.expect("OpenAI base URL should end with /v1");
reqwest::Client::new()
fabro_test::test_http_client()
.post(format!("{admin_url}/__admin/scenarios"))
.bearer_auth(&api_key)
.json(&serde_json::json!({

View file

@ -873,7 +873,7 @@ pub struct TwinGitHub {
server: twin_github::TestServer,
}
fn test_http_client() -> reqwest::Client {
pub fn test_http_client() -> reqwest::Client {
reqwest::Client::builder().no_proxy().build().unwrap()
}

View file

@ -113,7 +113,7 @@ pub async fn patch_webhook_config(
mod tests {
use crate::server::TestServer;
use crate::state::{AppOptions, AppState};
use crate::test_support::{sign_test_jwt, test_rsa_private_key};
use crate::test_support::{sign_test_jwt, test_http_client, test_rsa_private_key};
#[tokio::test]
async fn get_app_returns_app_info() {
@ -130,7 +130,7 @@ mod tests {
let server = TestServer::start(state).await;
let jwt = sign_test_jwt("12345", pem);
let client = reqwest::Client::new();
let client = test_http_client();
let resp = client
.get(&format!("{}/app", server.url()))
.header("Authorization", format!("Bearer {jwt}"))
@ -161,7 +161,7 @@ mod tests {
});
let server = TestServer::start(state).await;
let client = reqwest::Client::new();
let client = test_http_client();
let resp = client
.get(&format!("{}/app", server.url()))
.header("Authorization", "Bearer invalid-jwt")
@ -187,12 +187,16 @@ mod tests {
});
let server = TestServer::start(state).await;
let resp = reqwest::get(&format!("{}/apps/my-app", server.url()))
let resp = test_http_client()
.get(format!("{}/apps/my-app", server.url()))
.send()
.await
.unwrap();
assert_eq!(resp.status(), 200);
let resp404 = reqwest::get(&format!("{}/apps/nonexistent", server.url()))
let resp404 = test_http_client()
.get(format!("{}/apps/nonexistent", server.url()))
.send()
.await
.unwrap();
assert_eq!(resp404.status(), 404);
@ -214,7 +218,9 @@ mod tests {
});
let server = TestServer::start(state).await;
let resp = reqwest::get(&format!("{}/apps/private-app", server.url()))
let resp = test_http_client()
.get(format!("{}/apps/private-app", server.url()))
.send()
.await
.unwrap();
assert_eq!(resp.status(), 404);

View file

@ -93,7 +93,7 @@ pub async fn get_branch(
mod tests {
use crate::server::TestServer;
use crate::state::{AppOptions, AppState};
use crate::test_support::{sign_test_jwt, test_rsa_private_key};
use crate::test_support::{sign_test_jwt, test_http_client, test_rsa_private_key};
async fn get_installation_token(
client: &reqwest::Client,
@ -151,7 +151,7 @@ mod tests {
let server = TestServer::start(state).await;
let jwt = sign_test_jwt("100", pem);
let client = reqwest::Client::new();
let client = test_http_client();
let token = get_installation_token(&client, &jwt, "owner", "repo", server.url()).await;
let resp = client
@ -189,7 +189,7 @@ mod tests {
let server = TestServer::start(state).await;
let jwt = sign_test_jwt("100", pem);
let client = reqwest::Client::new();
let client = test_http_client();
let token = get_installation_token(&client, &jwt, "owner", "repo", server.url()).await;
let resp = client

View file

@ -383,7 +383,7 @@ mod tests {
state.add_repository("owner", "repo", vec!["main".to_string()], false);
let server = TestServer::start(state).await;
let client = reqwest::Client::new();
let client = crate::test_support::test_http_client();
let resp = client
.get(format!(
"{}/owner/repo.git/info/refs?service=git-upload-pack",

View file

@ -614,7 +614,7 @@ fn extract_unquoted_value(s: &str, key: &str) -> Option<String> {
mod tests {
use crate::server::TestServer;
use crate::state::{AppOptions, AppState, PullRequest};
use crate::test_support::{sign_test_jwt, test_rsa_private_key};
use crate::test_support::{sign_test_jwt, test_http_client, test_rsa_private_key};
async fn get_installation_token(
client: &reqwest::Client,
@ -674,7 +674,7 @@ mod tests {
let server = TestServer::start(state.clone()).await;
let jwt = sign_test_jwt("100", pem);
let client = reqwest::Client::new();
let client = test_http_client();
let token = get_installation_token(&client, &jwt, "owner", "repo", server.url()).await;
(server, client, token)
@ -770,7 +770,7 @@ mod tests {
let state = AppState::new();
let server = TestServer::start(state).await;
let client = reqwest::Client::new();
let client = test_http_client();
let resp = client
.post(&format!("{}/graphql", server.url()))
.header("Authorization", "Bearer invalid-token")

View file

@ -178,7 +178,7 @@ pub async fn create_access_token(
mod tests {
use crate::server::TestServer;
use crate::state::{AppOptions, AppState};
use crate::test_support::{sign_test_jwt, test_rsa_private_key};
use crate::test_support::{sign_test_jwt, test_http_client, test_rsa_private_key};
#[tokio::test]
async fn get_installation_returns_id() {
@ -197,7 +197,7 @@ mod tests {
let server = TestServer::start(state).await;
let jwt = sign_test_jwt("100", pem);
let client = reqwest::Client::new();
let client = test_http_client();
let resp = client
.get(&format!("{}/repos/owner/repo/installation", server.url()))
.header("Authorization", format!("Bearer {jwt}"))
@ -228,7 +228,7 @@ mod tests {
let server = TestServer::start(state).await;
let jwt = sign_test_jwt("100", pem);
let client = reqwest::Client::new();
let client = test_http_client();
let resp = client
.get(&format!("{}/repos/owner/repo/installation", server.url()))
.header("Authorization", format!("Bearer {jwt}"))
@ -256,7 +256,7 @@ mod tests {
let server = TestServer::start(state).await;
let jwt = sign_test_jwt("100", pem);
let client = reqwest::Client::new();
let client = test_http_client();
let resp = client
.get(&format!("{}/repos/owner/repo/installation", server.url()))
.header("Authorization", format!("Bearer {jwt}"))
@ -284,7 +284,7 @@ mod tests {
let server = TestServer::start(state).await;
let jwt = sign_test_jwt("100", pem);
let client = reqwest::Client::new();
let client = test_http_client();
let resp = client
.post(&format!(
"{}/app/installations/{install_id}/access_tokens",
@ -322,7 +322,7 @@ mod tests {
let server = TestServer::start(state).await;
let jwt = sign_test_jwt("100", pem);
let client = reqwest::Client::new();
let client = test_http_client();
let resp = client
.post(&format!(
"{}/app/installations/{install_id}/access_tokens",

View file

@ -56,7 +56,7 @@ mod tests {
);
let server = TestServer::start(state).await;
let client = reqwest::Client::new();
let client = crate::test_support::test_http_client();
let resp = client
.post(&format!(
"{}/app-manifests/test-code/conversions",
@ -82,7 +82,7 @@ mod tests {
let state = AppState::new();
let server = TestServer::start(state).await;
let client = reqwest::Client::new();
let client = crate::test_support::test_http_client();
let resp = client
.post(&format!(
"{}/app-manifests/unknown/conversions",

View file

@ -303,7 +303,7 @@ fn pr_to_json(pr: &PullRequest) -> serde_json::Value {
mod tests {
use crate::server::TestServer;
use crate::state::{AppOptions, AppState};
use crate::test_support::{sign_test_jwt, test_rsa_private_key};
use crate::test_support::{sign_test_jwt, test_http_client, test_rsa_private_key};
async fn get_installation_token(
client: &reqwest::Client,
@ -361,7 +361,7 @@ mod tests {
let server = TestServer::start(state.clone()).await;
let jwt = sign_test_jwt("100", pem);
let client = reqwest::Client::new();
let client = test_http_client();
let token = get_installation_token(&client, &jwt, "owner", "repo", server.url()).await;
(server, client, token)

View file

@ -44,12 +44,14 @@ mod tests {
);
let server = TestServer::start(state).await;
let resp = reqwest::get(&format!(
"{}/repos/test-org/test-project/releases/latest",
server.url()
))
.await
.unwrap();
let resp = crate::test_support::test_http_client()
.get(format!(
"{}/repos/test-org/test-project/releases/latest",
server.url()
))
.send()
.await
.unwrap();
assert_eq!(resp.status(), 200);
let body: serde_json::Value = resp.json().await.unwrap();
@ -62,12 +64,11 @@ mod tests {
let state = AppState::new();
let server = TestServer::start(state).await;
let resp = reqwest::get(&format!(
"{}/repos/owner/repo/releases/latest",
server.url()
))
.await
.unwrap();
let resp = crate::test_support::test_http_client()
.get(format!("{}/repos/owner/repo/releases/latest", server.url()))
.send()
.await
.unwrap();
assert_eq!(resp.status(), 404);
server.shutdown().await;

View file

@ -75,7 +75,9 @@ mod tests {
async fn server_starts_and_responds() {
let state = AppState::new();
let server = TestServer::start(state).await;
let resp = reqwest::get(&format!("{}/nonexistent", server.url()))
let resp = crate::test_support::test_http_client()
.get(format!("{}/nonexistent", server.url()))
.send()
.await
.unwrap();
assert_eq!(resp.status(), 404);

View file

@ -1,3 +1,8 @@
#[cfg(test)]
pub fn test_http_client() -> reqwest::Client {
reqwest::Client::builder().no_proxy().build().unwrap()
}
#[cfg(test)]
pub fn test_rsa_private_key() -> &'static str {
include_str!("testdata/rsa_private.pem")

View file

@ -61,6 +61,10 @@ pub struct ParsedSseEvent {
static NEXT_BEARER_TOKEN: AtomicU64 = AtomicU64::new(1);
pub fn test_http_client() -> Result<Client> {
Client::builder().no_proxy().build().map_err(Into::into)
}
pub async fn spawn_server() -> Result<TestServer> {
let listener = TcpListener::bind("127.0.0.1:0").await?;
let addr: SocketAddr = listener.local_addr()?;
@ -90,6 +94,7 @@ fn authorization_header_value(bearer_token: &str) -> String {
fn build_authenticated_client(bearer_token: &str) -> Result<Client> {
Client::builder()
.no_proxy()
.default_headers(
[(
reqwest::header::AUTHORIZATION,
@ -113,7 +118,10 @@ impl ApiClient {
) -> Result<Self> {
Ok(Self {
base_url: base_url.into(),
client: Client::builder().timeout(Duration::from_secs(30)).build()?,
client: Client::builder()
.no_proxy()
.timeout(Duration::from_secs(30))
.build()?,
bearer_token,
organization,
project,
@ -183,7 +191,7 @@ impl ApiClient {
impl TestServer {
fn new(base_url: String, bearer_token: String) -> Result<Self> {
let client = Client::builder().build()?;
let client = test_http_client()?;
let auth_client = build_authenticated_client(&bearer_token)?;
Ok(Self {

View file

@ -224,7 +224,7 @@ async fn debug_routes_not_accessible_when_admin_disabled() {
});
let base_url = format!("http://{addr}");
let client = reqwest::Client::new();
let client = common::test_http_client().expect("test client");
let html_response = client
.get(format!("{base_url}/__debug"))

View file

@ -106,6 +106,7 @@ async fn scripted_hang_times_out_client_side() {
.await;
let client = reqwest::Client::builder()
.no_proxy()
.timeout(Duration::from_millis(150))
.default_headers(
[(