From 0781e3a2c15437167dd297125cd7b6ea63511810 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Wed, 22 Apr 2026 07:50:34 -0400 Subject: [PATCH] test(cli): suppress browser launch under spawned fabro test subprocesses Add FABRO_SUPPRESS_OPEN_BROWSER env knob via fabro_util::browser::try_open. apply_test_isolation now sets it, so install-mode and auth-login tests that spawn a real fabro binary no longer pop real browser windows. All six open::that call sites route through the helper; consolidates the direct open crate dep into fabro-util. Co-Authored-By: Claude Opus 4.7 (1M context) --- Cargo.lock | 5 +++-- lib/crates/fabro-cli/Cargo.toml | 1 - lib/crates/fabro-cli/src/commands/auth/login.rs | 3 ++- lib/crates/fabro-cli/src/commands/install.rs | 4 ++-- lib/crates/fabro-cli/src/commands/server/mod.rs | 3 ++- lib/crates/fabro-cli/src/main.rs | 6 +++--- lib/crates/fabro-oauth/Cargo.toml | 2 +- lib/crates/fabro-oauth/src/lib.rs | 3 ++- lib/crates/fabro-test/Cargo.toml | 1 + lib/crates/fabro-test/src/lib.rs | 4 +++- lib/crates/fabro-util/Cargo.toml | 1 + lib/crates/fabro-util/src/browser.rs | 12 ++++++++++++ lib/crates/fabro-util/src/lib.rs | 1 + 13 files changed, 33 insertions(+), 13 deletions(-) create mode 100644 lib/crates/fabro-util/src/browser.rs diff --git a/Cargo.lock b/Cargo.lock index c460037f4..f171e3b26 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1655,7 +1655,6 @@ dependencies = [ "jsonwebtoken", "libc", "object_store", - "open", "openssl", "paste", "predicates", @@ -1924,9 +1923,9 @@ dependencies = [ "base64", "fabro-http", "fabro-test", + "fabro-util", "hex", "httpmock", - "open", "rand 0.9.4", "serde", "serde_json", @@ -2167,6 +2166,7 @@ dependencies = [ "fabro-http", "fabro-proc", "fabro-types", + "fabro-util", "insta", "regex", "serde", @@ -2220,6 +2220,7 @@ dependencies = [ "console 0.15.11", "dirs", "insta", + "open", "rand 0.9.4", "regex", "serde", diff --git a/lib/crates/fabro-cli/Cargo.toml b/lib/crates/fabro-cli/Cargo.toml index 4687f7df9..1b0283f29 100644 --- a/lib/crates/fabro-cli/Cargo.toml +++ b/lib/crates/fabro-cli/Cargo.toml @@ -81,7 +81,6 @@ rand.workspace = true dialoguer.workspace = true git2.workspace = true axum.workspace = true -open = "5" serde_json.workspace = true serde_yaml = "0.9" tempfile = "3" diff --git a/lib/crates/fabro-cli/src/commands/auth/login.rs b/lib/crates/fabro-cli/src/commands/auth/login.rs index 0419def51..8ccf866ca 100644 --- a/lib/crates/fabro-cli/src/commands/auth/login.rs +++ b/lib/crates/fabro-cli/src/commands/auth/login.rs @@ -6,6 +6,7 @@ use fabro_client::{AuthEntry, AuthStore, StoredSubject}; use fabro_http::header::CONTENT_TYPE; use fabro_types::settings::CliSettings; use fabro_types::settings::cli::CliLayer; +use fabro_util::browser; use fabro_util::printer::Printer; use serde::Deserialize; use tokio::time::timeout; @@ -189,7 +190,7 @@ fn open_browser_or_print(browser_url: &str, no_browser: bool, printer: Printer) return; } - if let Err(error) = open::that(browser_url) { + if let Err(error) = browser::try_open(browser_url) { fabro_util::printerr!(printer, "Could not open a browser automatically: {error}"); fabro_util::printerr!(printer, "Open this URL to continue login:"); fabro_util::printerr!(printer, "{browser_url}"); diff --git a/lib/crates/fabro-cli/src/commands/install.rs b/lib/crates/fabro-cli/src/commands/install.rs index 776756bdd..5f20020d1 100644 --- a/lib/crates/fabro-cli/src/commands/install.rs +++ b/lib/crates/fabro-cli/src/commands/install.rs @@ -37,7 +37,7 @@ use fabro_types::settings::{CliSettings, SettingsLayer}; use fabro_util::printer::Printer; use fabro_util::terminal::Styles; use fabro_util::version::FABRO_VERSION; -use fabro_util::{dev_token, path, session_secret}; +use fabro_util::{browser, dev_token, path, session_secret}; use fabro_vault::{SecretType as VaultSecretType, Vault}; use futures::future::BoxFuture; use rand::Rng; @@ -1003,7 +1003,7 @@ async fn setup_github_app( match handoff_mode { GitHubAppHandoffMode::Interactive => { fabro_util::printerr!(printer, " {}", s.dim.apply_to("Opening browser...")); - if let Err(e) = open::that(&url) { + if let Err(e) = browser::try_open(&url) { fabro_util::printerr!(printer, " Could not open browser automatically: {e}"); fabro_util::printerr!(printer, " Please open this URL manually: {url}"); } diff --git a/lib/crates/fabro-cli/src/commands/server/mod.rs b/lib/crates/fabro-cli/src/commands/server/mod.rs index df730f5bf..a36d27333 100644 --- a/lib/crates/fabro-cli/src/commands/server/mod.rs +++ b/lib/crates/fabro-cli/src/commands/server/mod.rs @@ -13,6 +13,7 @@ use fabro_config::user::{FABRO_CONFIG_ENV, active_settings_path, default_storage use fabro_server::bind::{self, Bind, BindRequest}; use fabro_server::install::{self, InstallAppState}; use fabro_server::serve::{self, ServeArgs}; +use fabro_util::browser; use fabro_util::printer::Printer; use fabro_util::terminal::Styles; use ring::rand::{SecureRandom, SystemRandom}; @@ -223,7 +224,7 @@ fn announce_install_mode(bind: &Bind, token: &str, styles: &Styles, printer: Pri Some(url) => { fabro_util::printerr!(printer, " Open this URL in your browser to finish setup:"); fabro_util::printerr!(printer, " {url}"); - if let Err(e) = open::that(&url) { + if let Err(e) = browser::try_open(&url) { fabro_util::printerr!(printer, ""); fabro_util::printerr!(printer, " Could not open a browser automatically: {e}"); fabro_util::printerr!(printer, " Open the URL above manually to continue."); diff --git a/lib/crates/fabro-cli/src/main.rs b/lib/crates/fabro-cli/src/main.rs index 37eabceba..8299f1208 100644 --- a/lib/crates/fabro-cli/src/main.rs +++ b/lib/crates/fabro-cli/src/main.rs @@ -31,9 +31,9 @@ use fabro_config::merge::combine_files; use fabro_telemetry::{git, panic as tel_panic, sanitize, sender}; use fabro_types::settings::SettingsLayer; use fabro_types::settings::cli::OutputVerbosity; -use fabro_util::exit; use fabro_util::printer::Printer; use fabro_util::terminal::Styles; +use fabro_util::{browser, exit}; use rustls::crypto::ring::default_provider; use tracing::debug; @@ -276,7 +276,7 @@ async fn main_inner() -> (String, Result<()>) { "url": "https://fabro.sh/discord", }))?; } else { - open::that("https://fabro.sh/discord")?; + browser::try_open("https://fabro.sh/discord")?; } } Commands::Docs => { @@ -285,7 +285,7 @@ async fn main_inner() -> (String, Result<()>) { "url": "https://docs.fabro.sh/", }))?; } else { - open::that("https://docs.fabro.sh/")?; + browser::try_open("https://docs.fabro.sh/")?; } } Commands::Repo(ns) => { diff --git a/lib/crates/fabro-oauth/Cargo.toml b/lib/crates/fabro-oauth/Cargo.toml index fe664e243..1baa78079 100644 --- a/lib/crates/fabro-oauth/Cargo.toml +++ b/lib/crates/fabro-oauth/Cargo.toml @@ -23,7 +23,7 @@ hex.workspace = true tokio.workspace = true tracing.workspace = true axum.workspace = true -open = "5" +fabro-util = { path = "../fabro-util" } [dev-dependencies] httpmock = "0.8" diff --git a/lib/crates/fabro-oauth/src/lib.rs b/lib/crates/fabro-oauth/src/lib.rs index b7e531b97..d193f3164 100644 --- a/lib/crates/fabro-oauth/src/lib.rs +++ b/lib/crates/fabro-oauth/src/lib.rs @@ -6,6 +6,7 @@ use axum::response::Html; use axum::routing::get; use base64::Engine; use base64::engine::general_purpose::URL_SAFE_NO_PAD; +use fabro_util::browser; use serde::Deserialize; use sha2::{Digest, Sha256}; use tokio::net::TcpListener; @@ -653,7 +654,7 @@ pub async fn run_browser_flow( "OAuth browser flow started" ); - if let Err(e) = open::that(&auth_url) { + if let Err(e) = browser::try_open(&auth_url) { tracing::warn!("Could not open browser: {e}"); } diff --git a/lib/crates/fabro-test/Cargo.toml b/lib/crates/fabro-test/Cargo.toml index 3ba0f9d07..8fde15dbf 100644 --- a/lib/crates/fabro-test/Cargo.toml +++ b/lib/crates/fabro-test/Cargo.toml @@ -18,6 +18,7 @@ axum = { workspace = true } fabro-config = { path = "../fabro-config" } fabro-proc = { path = "../fabro-proc" } fabro-types = { path = "../fabro-types" } +fabro-util = { path = "../fabro-util" } fabro-http.workspace = true insta = { workspace = true, features = ["filters"] } regex = { workspace = true } diff --git a/lib/crates/fabro-test/src/lib.rs b/lib/crates/fabro-test/src/lib.rs index b428cc482..18cb518f0 100644 --- a/lib/crates/fabro-test/src/lib.rs +++ b/lib/crates/fabro-test/src/lib.rs @@ -15,6 +15,7 @@ use std::time::{Duration, SystemTime, UNIX_EPOCH}; use assert_cmd::Command; use fabro_config::Storage; use fabro_types::RunId; +use fabro_util::browser; use regex::Regex; use serde_json::{Map, Value, json}; use toml::Value as TomlValue; @@ -163,7 +164,8 @@ fn apply_test_isolation_with_lookup( cmd.env("HOME", home_dir); cmd.env("FABRO_NO_UPGRADE_CHECK", "true") .env("FABRO_HTTP_PROXY_POLICY", "disabled") - .env("FABRO_TELEMETRY", "off"); + .env("FABRO_TELEMETRY", "off") + .env(browser::SUPPRESS_ENV_VAR, "1"); cmd.env("FABRO_SERVER_MAX_CONCURRENT_RUNS", "64"); cmd.env(TEST_IN_MEMORY_STORE_ENV, "1"); } diff --git a/lib/crates/fabro-util/Cargo.toml b/lib/crates/fabro-util/Cargo.toml index d12653a4c..68efa2166 100644 --- a/lib/crates/fabro-util/Cargo.toml +++ b/lib/crates/fabro-util/Cargo.toml @@ -25,6 +25,7 @@ dirs.workspace = true tracing.workspace = true tracing-subscriber.workspace = true anyhow.workspace = true +open = "5" [build-dependencies] toml = "0.8" diff --git a/lib/crates/fabro-util/src/browser.rs b/lib/crates/fabro-util/src/browser.rs new file mode 100644 index 000000000..5de379f90 --- /dev/null +++ b/lib/crates/fabro-util/src/browser.rs @@ -0,0 +1,12 @@ +/// When this environment variable is set to any value, [`try_open`] returns +/// `Ok(())` without launching a browser. Test harnesses set it so spawned +/// `fabro` subprocesses do not pop real browser windows during CI or local +/// runs. +pub const SUPPRESS_ENV_VAR: &str = "FABRO_SUPPRESS_OPEN_BROWSER"; + +pub fn try_open(url: &str) -> std::io::Result<()> { + if std::env::var_os(SUPPRESS_ENV_VAR).is_some() { + return Ok(()); + } + open::that(url) +} diff --git a/lib/crates/fabro-util/src/lib.rs b/lib/crates/fabro-util/src/lib.rs index f584e3687..b77536640 100644 --- a/lib/crates/fabro-util/src/lib.rs +++ b/lib/crates/fabro-util/src/lib.rs @@ -1,4 +1,5 @@ pub mod backoff; +pub mod browser; pub mod check_report; pub mod dev_token; pub mod env;