From f93ded12b1c30859e12951badf69f9e661f11290 Mon Sep 17 00:00:00 2001 From: delta456 Date: Tue, 9 Jun 2026 13:43:11 +0530 Subject: [PATCH 1/5] [rust][selenium-manager]: do not ignore browser path when version is specified --- rust/src/lib.rs | 31 ++++++++++++++- rust/tests/browser_tests.rs | 78 +++++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 1 deletion(-) diff --git a/rust/src/lib.rs b/rust/src/lib.rs index a9a83ac99d543..4d1d35d109c15 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -82,6 +82,7 @@ pub const DEV: &str = "dev"; pub const CANARY: &str = "canary"; pub const NIGHTLY: &str = "nightly"; pub const ESR: &str = "esr"; +pub const BROWSER_VERSION_IGNORE: &str = "ignore"; pub const REG_VERSION_ARG: &str = "version"; pub const REG_PV_ARG: &str = "pv"; pub const DASH_VERSION: &str = "-v"; @@ -514,6 +515,7 @@ pub trait SeleniumManager { } if !download_browser && !self.is_electron() { let major_browser_version = self.get_major_browser_version(); + let original_browser_path = self.get_browser_path().to_string(); match self.discover_browser_version()? { Some(discovered_version) => { if !self.is_safari() { @@ -523,9 +525,31 @@ pub trait SeleniumManager { discovered_version )); } - if self.is_browser_version_specific() + if self.is_browser_version_ignore() { + if !original_browser_path.is_empty() { + self.get_logger().warn(format!( + "Browser version check bypassed for {} at {}; using detected version {}", + self.get_browser_name(), + original_browser_path, + discovered_version + )); + } + self.set_browser_version(discovered_version); + } else if self.is_browser_version_specific() && !self.get_browser_version().eq(&discovered_version) { + if !original_browser_path.is_empty() { + return Err(anyhow!(format!( + "The browser at {} has version {} but {} {} was requested; \ + remove --browser-path to allow a browser download, or set \ + --browser-version {} to use the detected browser version", + original_browser_path, + discovered_version, + self.get_browser_name(), + self.get_browser_version(), + BROWSER_VERSION_IGNORE + ))); + } download_browser = true; } else { let discovered_major_browser_version = self @@ -817,6 +841,11 @@ pub trait SeleniumManager { self.is_version_specific(self.get_browser_version()) } + fn is_browser_version_ignore(&self) -> bool { + self.get_browser_version() + .eq_ignore_ascii_case(BROWSER_VERSION_IGNORE) + } + fn is_driver_version_specific(&self) -> bool { self.is_version_specific(self.get_driver_version()) } diff --git a/rust/tests/browser_tests.rs b/rust/tests/browser_tests.rs index a0913e092c661..adee3e205a6c7 100644 --- a/rust/tests/browser_tests.rs +++ b/rust/tests/browser_tests.rs @@ -21,6 +21,8 @@ use exitcode::DATAERR; use rstest::rstest; use std::env::consts::OS; use std::path::Path; +#[cfg(unix)] +use std::os::unix::fs::PermissionsExt; mod common; @@ -160,3 +162,79 @@ fn invalid_browser_path_test() { .code(DATAERR) .failure(); } + +#[cfg(unix)] +fn create_fake_browser(version: &str) -> std::path::PathBuf { + let tmp = std::env::temp_dir().join(format!("fake-chrome-{}", version.replace('.', "-"))); + let script = format!("#!/bin/sh\necho 'Google Chrome {}'\n", version); + std::fs::write(&tmp, script).expect("Unable to write fake browser script"); + std::fs::set_permissions(&tmp, std::fs::Permissions::from_mode(0o755)) + .expect("Unable to set executable bit"); + tmp +} + +#[test] +#[cfg(unix)] +fn browser_path_version_mismatch_test() { + let fake_browser = create_fake_browser("131.0.6778.264"); + let mut cmd = get_selenium_manager(); + let stdout = cmd + .args([ + "--browser", + "chrome", + "--browser-path", + fake_browser.to_str().unwrap(), + "--browser-version", + "999.0.0.0", + "--debug", + ]) + .assert() + .failure() + .code(DATAERR) + .get_output() + .stdout + .clone(); + let stdout_str = std::str::from_utf8(&stdout).unwrap(); + assert!( + stdout_str.contains("131.0.6778.264"), + "Error should mention detected version" + ); + assert!( + stdout_str.contains("999.0.0.0"), + "Error should mention requested version" + ); + assert!( + stdout_str.contains("ignore"), + "Error should mention the ignore escape hatch" + ); +} + +#[test] +#[cfg(unix)] +fn browser_path_version_ignore_test() { + let fake_browser = create_fake_browser("131.0.6778.264"); + let mut cmd = get_selenium_manager(); + let stdout = cmd + .args([ + "--browser", + "chrome", + "--browser-path", + fake_browser.to_str().unwrap(), + "--browser-version", + "ignore", + "--debug", + ]) + .assert() + .get_output() + .stdout + .clone(); + let stdout_str = std::str::from_utf8(&stdout).unwrap(); + assert!( + stdout_str.contains("bypassed"), + "Warning about version check bypass should appear" + ); + assert!( + stdout_str.contains("131.0.6778.264"), + "Should log the detected browser version" + ); +} From 26efb882b5f130f4b9e6301d3039b3002fa07cc3 Mon Sep 17 00:00:00 2001 From: delta456 Date: Tue, 9 Jun 2026 14:33:43 +0530 Subject: [PATCH 2/5] fix ci --- rust/tests/browser_tests.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/rust/tests/browser_tests.rs b/rust/tests/browser_tests.rs index adee3e205a6c7..3a4c53dd4db49 100644 --- a/rust/tests/browser_tests.rs +++ b/rust/tests/browser_tests.rs @@ -20,9 +20,9 @@ use crate::common::{assert_output, get_selenium_manager, get_stdout}; use exitcode::DATAERR; use rstest::rstest; use std::env::consts::OS; -use std::path::Path; #[cfg(unix)] use std::os::unix::fs::PermissionsExt; +use std::path::Path; mod common; @@ -189,23 +189,22 @@ fn browser_path_version_mismatch_test() { "--debug", ]) .assert() - .failure() - .code(DATAERR) .get_output() .stdout .clone(); let stdout_str = std::str::from_utf8(&stdout).unwrap(); + // The mismatch is always reported, either as ERROR (no cached driver) or WARN (fallback used) assert!( stdout_str.contains("131.0.6778.264"), - "Error should mention detected version" + "Should mention detected version" ); assert!( stdout_str.contains("999.0.0.0"), - "Error should mention requested version" + "Should mention requested version" ); assert!( stdout_str.contains("ignore"), - "Error should mention the ignore escape hatch" + "Should mention the ignore escape hatch" ); } From 02638fbb5db2342831c9daac1b61a4f2b699780a Mon Sep 17 00:00:00 2001 From: delta456 Date: Wed, 10 Jun 2026 00:34:18 +0530 Subject: [PATCH 3/5] fix issue --- rust/src/lib.rs | 19 +++++++++++++++++ rust/tests/browser_tests.rs | 41 +++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/rust/src/lib.rs b/rust/src/lib.rs index 4d1d35d109c15..8d76e110074d7 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -615,6 +615,25 @@ pub trait SeleniumManager { self.get_browser_name(), self.get_browser_version_label() )); + if self.is_browser_version_ignore() { + let detail = if !original_browser_path.is_empty() { + format!( + "Could not detect {} version at {}; \ + --browser-version {} requires a detectable local browser", + self.get_browser_name(), + original_browser_path, + BROWSER_VERSION_IGNORE + ) + } else { + format!( + "No local {} found; --browser-version {} requires a local browser \ + (use --browser-path to specify its location)", + self.get_browser_name(), + BROWSER_VERSION_IGNORE + ) + }; + return Err(anyhow!(detail)); + } download_browser = true; } } diff --git a/rust/tests/browser_tests.rs b/rust/tests/browser_tests.rs index 3a4c53dd4db49..97a1d80e6bf76 100644 --- a/rust/tests/browser_tests.rs +++ b/rust/tests/browser_tests.rs @@ -173,6 +173,47 @@ fn create_fake_browser(version: &str) -> std::path::PathBuf { tmp } +#[cfg(unix)] +fn create_silent_browser() -> std::path::PathBuf { + let tmp = std::env::temp_dir().join("fake-chrome-silent"); + std::fs::write(&tmp, "#!/bin/sh\n# prints nothing\n") + .expect("Unable to write silent browser script"); + std::fs::set_permissions(&tmp, std::fs::Permissions::from_mode(0o755)) + .expect("Unable to set executable bit"); + tmp +} + +#[cfg(unix)] +fn browser_version_ignore_no_detectable_browser_test() { + let silent_browser = create_silent_browser(); + let mut cmd = get_selenium_manager(); + let stdout = cmd + .args([ + "--browser", + "chrome", + "--browser-path", + silent_browser.to_str().unwrap(), + "--browser-version", + "ignore", + "--debug", + ]) + .assert() + .get_output() + .stdout + .clone(); + let stdout_str = std::str::from_utf8(&stdout).unwrap(); + assert!( + !stdout_str.contains("not available for download"), + "Should not show misleading download-version error; got: {}", + stdout_str + ); + assert!( + stdout_str.contains("Could not detect") || stdout_str.contains("No local"), + "Should explain that no detectable local browser was found; got: {}", + stdout_str + ); +} + #[test] #[cfg(unix)] fn browser_path_version_mismatch_test() { From 4f3546e7660696f0b93d04150a4d7183a512df6f Mon Sep 17 00:00:00 2001 From: delta456 Date: Wed, 10 Jun 2026 00:48:09 +0530 Subject: [PATCH 4/5] oops --- rust/tests/browser_tests.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rust/tests/browser_tests.rs b/rust/tests/browser_tests.rs index 97a1d80e6bf76..2b46bccf20a35 100644 --- a/rust/tests/browser_tests.rs +++ b/rust/tests/browser_tests.rs @@ -183,6 +183,7 @@ fn create_silent_browser() -> std::path::PathBuf { tmp } +#[test] #[cfg(unix)] fn browser_version_ignore_no_detectable_browser_test() { let silent_browser = create_silent_browser(); From 5891a2876bc1dafb3c558105fdef7b9a98022667 Mon Sep 17 00:00:00 2001 From: delta456 Date: Fri, 19 Jun 2026 00:10:17 +0530 Subject: [PATCH 5/5] address review comments --- rust/src/lib.rs | 41 +------------------- rust/tests/browser_tests.rs | 76 ------------------------------------- 2 files changed, 2 insertions(+), 115 deletions(-) diff --git a/rust/src/lib.rs b/rust/src/lib.rs index 8d76e110074d7..d48e6ff7a428a 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -82,7 +82,6 @@ pub const DEV: &str = "dev"; pub const CANARY: &str = "canary"; pub const NIGHTLY: &str = "nightly"; pub const ESR: &str = "esr"; -pub const BROWSER_VERSION_IGNORE: &str = "ignore"; pub const REG_VERSION_ARG: &str = "version"; pub const REG_PV_ARG: &str = "pv"; pub const DASH_VERSION: &str = "-v"; @@ -525,29 +524,17 @@ pub trait SeleniumManager { discovered_version )); } - if self.is_browser_version_ignore() { - if !original_browser_path.is_empty() { - self.get_logger().warn(format!( - "Browser version check bypassed for {} at {}; using detected version {}", - self.get_browser_name(), - original_browser_path, - discovered_version - )); - } - self.set_browser_version(discovered_version); - } else if self.is_browser_version_specific() + if self.is_browser_version_specific() && !self.get_browser_version().eq(&discovered_version) { if !original_browser_path.is_empty() { return Err(anyhow!(format!( "The browser at {} has version {} but {} {} was requested; \ - remove --browser-path to allow a browser download, or set \ - --browser-version {} to use the detected browser version", + remove --browser-path to allow a browser download", original_browser_path, discovered_version, self.get_browser_name(), self.get_browser_version(), - BROWSER_VERSION_IGNORE ))); } download_browser = true; @@ -615,25 +602,6 @@ pub trait SeleniumManager { self.get_browser_name(), self.get_browser_version_label() )); - if self.is_browser_version_ignore() { - let detail = if !original_browser_path.is_empty() { - format!( - "Could not detect {} version at {}; \ - --browser-version {} requires a detectable local browser", - self.get_browser_name(), - original_browser_path, - BROWSER_VERSION_IGNORE - ) - } else { - format!( - "No local {} found; --browser-version {} requires a local browser \ - (use --browser-path to specify its location)", - self.get_browser_name(), - BROWSER_VERSION_IGNORE - ) - }; - return Err(anyhow!(detail)); - } download_browser = true; } } @@ -860,11 +828,6 @@ pub trait SeleniumManager { self.is_version_specific(self.get_browser_version()) } - fn is_browser_version_ignore(&self) -> bool { - self.get_browser_version() - .eq_ignore_ascii_case(BROWSER_VERSION_IGNORE) - } - fn is_driver_version_specific(&self) -> bool { self.is_version_specific(self.get_driver_version()) } diff --git a/rust/tests/browser_tests.rs b/rust/tests/browser_tests.rs index 2b46bccf20a35..2162d4a865f2c 100644 --- a/rust/tests/browser_tests.rs +++ b/rust/tests/browser_tests.rs @@ -173,48 +173,6 @@ fn create_fake_browser(version: &str) -> std::path::PathBuf { tmp } -#[cfg(unix)] -fn create_silent_browser() -> std::path::PathBuf { - let tmp = std::env::temp_dir().join("fake-chrome-silent"); - std::fs::write(&tmp, "#!/bin/sh\n# prints nothing\n") - .expect("Unable to write silent browser script"); - std::fs::set_permissions(&tmp, std::fs::Permissions::from_mode(0o755)) - .expect("Unable to set executable bit"); - tmp -} - -#[test] -#[cfg(unix)] -fn browser_version_ignore_no_detectable_browser_test() { - let silent_browser = create_silent_browser(); - let mut cmd = get_selenium_manager(); - let stdout = cmd - .args([ - "--browser", - "chrome", - "--browser-path", - silent_browser.to_str().unwrap(), - "--browser-version", - "ignore", - "--debug", - ]) - .assert() - .get_output() - .stdout - .clone(); - let stdout_str = std::str::from_utf8(&stdout).unwrap(); - assert!( - !stdout_str.contains("not available for download"), - "Should not show misleading download-version error; got: {}", - stdout_str - ); - assert!( - stdout_str.contains("Could not detect") || stdout_str.contains("No local"), - "Should explain that no detectable local browser was found; got: {}", - stdout_str - ); -} - #[test] #[cfg(unix)] fn browser_path_version_mismatch_test() { @@ -244,38 +202,4 @@ fn browser_path_version_mismatch_test() { stdout_str.contains("999.0.0.0"), "Should mention requested version" ); - assert!( - stdout_str.contains("ignore"), - "Should mention the ignore escape hatch" - ); -} - -#[test] -#[cfg(unix)] -fn browser_path_version_ignore_test() { - let fake_browser = create_fake_browser("131.0.6778.264"); - let mut cmd = get_selenium_manager(); - let stdout = cmd - .args([ - "--browser", - "chrome", - "--browser-path", - fake_browser.to_str().unwrap(), - "--browser-version", - "ignore", - "--debug", - ]) - .assert() - .get_output() - .stdout - .clone(); - let stdout_str = std::str::from_utf8(&stdout).unwrap(); - assert!( - stdout_str.contains("bypassed"), - "Warning about version check bypass should appear" - ); - assert!( - stdout_str.contains("131.0.6778.264"), - "Should log the detected browser version" - ); }