[rust] honor --browser-version in Selenium Manager Electron driver resolution

rust/src/electron.rs

@@ -25,7 +25,8 @@ use crate::metadata::{
     create_driver_metadata, get_driver_version_from_metadata, get_metadata, write_metadata,
 };
 use crate::{
-    LATEST_RELEASE, Logger, OFFLINE_REQUEST_ERR_MSG, SeleniumManager, WINDOWS, create_http_client,
+    BETA, CANARY, DEV, ESR, LATEST_RELEASE, Logger, NIGHTLY, OFFLINE_REQUEST_ERR_MSG, STABLE,
+    SeleniumManager, WINDOWS, create_http_client,
 };
 use anyhow::Error;
 use reqwest::Client;
@@ -37,6 +38,46 @@ use std::sync::mpsc::{Receiver, Sender};
 pub const ELECTRON_NAME: &str = "electron";
 const DRIVER_URL: &str = "https://github.com/electron/electron/releases/";
 
+/// Returns the user's `--browser-version` as the resolved Electron driver
+/// version when - and only when - it is a *specific* version that we can use
+/// verbatim as a release tag (e.g. `36.2.1`, `v36.2.1`, `36.0.0-beta.1`).
+///
+/// Returns `None` for empty input, channel names (`stable`, `beta`, `dev`,
+/// `nightly`, `canary`, `esr`), and major-only inputs like `36` - those must
+/// keep using the `/releases/latest` redirect path because Electron has no
+/// `v36` tag and emitting one would build a 404 download URL.
+///
+/// A leading `v` or `V` is stripped (so `v36.2.1` doesn't double up against
+/// the `v` prefix added by `get_driver_url()`); we deliberately do *not* call
+/// `parse_version()` here because that strips prerelease suffixes such as
+/// `-beta.1`, which Electron release tags legitimately carry.
+fn normalize_pinned_version(browser_version: &str) -> Option<String> {
+    let trimmed = browser_version.trim();
+    if trimmed.is_empty() {
+        return None;
+    }
+    if trimmed.eq_ignore_ascii_case(STABLE)
+        || trimmed.eq_ignore_ascii_case(BETA)
+        || trimmed.eq_ignore_ascii_case(DEV)
+        || trimmed.eq_ignore_ascii_case(NIGHTLY)
+        || trimmed.eq_ignore_ascii_case(CANARY)
+        || trimmed.eq_ignore_ascii_case(ESR)
+    {
+        return None;
+    }
+    let stripped = trimmed
+        .strip_prefix('v')
+        .or_else(|| trimmed.strip_prefix('V'))
+        .unwrap_or(trimmed);
+    // Require a specific version (contains a dot); major-only inputs like
+    // `36` are not valid Electron release tags and must use the latest
+    // fallback.
+    if !stripped.contains('.') {
+        return None;
+    }
+    Some(stripped.to_string())
+}
+
 pub struct ElectronManager {
     pub browser_name: &'static str,
     pub driver_name: &'static str,
@@ -101,6 +142,27 @@ impl SeleniumManager for ElectronManager {
     }
 
     fn request_driver_version(&mut self) -> Result<String, Error> {
+        // Electron releases are tagged by Electron version, and the
+        // chromedriver asset shipped in each release matches that tag.
+        // When the user pins a *specific* browser version (e.g.
+        // `--browser-version 36.2.1`), that version is the driver version
+        // we want; resolving via `/releases/latest` would discard the
+        // user's request and return the latest tag instead.
+        //
+        // This short-circuit must run *before* the metadata cache lookup:
+        // the cache is keyed only on `major_browser_version`, so a cached
+        // entry for major `36` would otherwise override an explicit
+        // `36.2.1` pin and silently return a different patch version.
+        //
+        // Major-only inputs like `36` keep using the latest-redirect
+        // fallback path, because Electron has no `v36` tag - using it
+        // verbatim would build a 404 download URL.
+        let browser_version = self.get_browser_version().to_string();
+        let normalized_pin = normalize_pinned_version(&browser_version);
+        if let Some(pinned) = normalized_pin {
+            return Ok(pinned);
+        }
+
         let major_browser_version_binding = self.get_major_browser_version();
         let major_browser_version = major_browser_version_binding.as_str();
         let cache_path = self.get_cache_path()?;
@@ -120,7 +182,6 @@ impl SeleniumManager for ElectronManager {
             }
             _ => {
                 self.assert_online_or_err(OFFLINE_REQUEST_ERR_MSG)?;
-
                 let latest_url = format!(
                     "{}{}",
                     self.get_driver_mirror_url_or_default(DRIVER_URL),
@@ -129,7 +190,8 @@ impl SeleniumManager for ElectronManager {
                 let driver_version =
                     read_redirect_from_link(self.get_http_client(), latest_url, self.get_logger())?;
                 let driver_ttl = self.get_ttl();
-                if driver_ttl > 0 {
+                if driver_ttl > 0 && !major_browser_version.is_empty() && !driver_version.is_empty()
+                {
                     metadata.drivers.push(create_driver_metadata(
                         major_browser_version,
                         self.driver_name,
@@ -273,3 +335,66 @@ impl SeleniumManager for ElectronManager {
         None
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::normalize_pinned_version;
+
+    #[test]
+    fn specific_version_is_returned_verbatim() {
+        assert_eq!(
+            normalize_pinned_version("36.2.1"),
+            Some("36.2.1".to_string())
+        );
+    }
+
+    #[test]
+    fn leading_v_is_stripped() {
+        assert_eq!(
+            normalize_pinned_version("v36.2.1"),
+            Some("36.2.1".to_string())
+        );
+        assert_eq!(
+            normalize_pinned_version("V36.2.1"),
+            Some("36.2.1".to_string())
+        );
+    }
+
+    #[test]
+    fn prerelease_suffix_is_preserved() {
+        assert_eq!(
+            normalize_pinned_version("36.0.0-beta.1"),
+            Some("36.0.0-beta.1".to_string())
+        );
+        assert_eq!(
+            normalize_pinned_version("v36.0.0-beta.1"),
+            Some("36.0.0-beta.1".to_string())
+        );
+    }
+
+    #[test]
+    fn major_only_falls_back() {
+        // `36` has no Electron tag; must fall back to /releases/latest.
+        assert_eq!(normalize_pinned_version("36"), None);
+        assert_eq!(normalize_pinned_version("v36"), None);
+    }
+
+    #[test]
+    fn channel_names_fall_back() {
+        for channel in ["stable", "beta", "dev", "nightly", "canary", "esr"] {
+            assert_eq!(normalize_pinned_version(channel), None, "{}", channel);
+            assert_eq!(
+                normalize_pinned_version(&channel.to_uppercase()),
+                None,
+                "{}",
+                channel
+            );
+        }
+    }
+
+    #[test]
+    fn empty_or_whitespace_falls_back() {
+        assert_eq!(normalize_pinned_version(""), None);
+        assert_eq!(normalize_pinned_version("   "), None);
+    }
+}

rust/tests/electron_tests.rs

@@ -15,7 +15,7 @@
 // specific language governing permissions and limitations
 // under the License.
 
-use crate::common::get_selenium_manager;
+use crate::common::{get_driver_path, get_selenium_manager};
 
 use rstest::rstest;
 
@@ -37,3 +37,55 @@ fn electron_version_test(#[case] driver_version: String) {
         .assert();
     cmd_assert.success();
 }
+
+#[rstest]
+#[case("36.2.1")]
+#[case("v36.2.1")]
+fn electron_browser_version_test(#[case] browser_version: String) {
+    // Regression for issue #17549: when the user pins --browser-version, the
+    // resolved driver version must be derived from the requested version, not
+    // from the /releases/latest redirect. Asserting that the resolved driver
+    // path contains the requested version protects against silently falling
+    // back to the latest tag.
+    //
+    // The `v36.2.1` case additionally guards against a doubled `v` prefix:
+    // `get_driver_url()` already prepends `v` to the driver version, so the
+    // pinned version must be normalized (leading `v` stripped) before being
+    // returned.
+    let mut cmd = get_selenium_manager();
+    cmd.args([
+        "--browser",
+        "electron",
+        "--browser-version",
+        &browser_version,
+        "--output",
+        "json",
+    ]);
+    let driver_path = get_driver_path(&mut cmd);
+    let expected = browser_version.trim_start_matches(['v', 'V']);
+    assert!(
+        driver_path.contains(expected),
+        "expected resolved driver path to contain requested version {}, got: {}",
+        expected,
+        driver_path
+    );
+    assert!(
+        !driver_path.contains("vv"),
+        "resolved driver path must not contain doubled `v` prefix, got: {}",
+        driver_path
+    );
+}
+
+#[test]
+fn electron_major_only_browser_version_falls_back_test() {
+    // Regression: a major-only --browser-version like `36` is not a valid
+    // Electron release tag (no `v36` exists), so the manager must fall back
+    // to the /releases/latest redirect instead of constructing a 404 URL.
+    // We assert the command still succeeds; the resolved driver version is
+    // whatever `/releases/latest` currently points at, which we do not pin.
+    let mut cmd = get_selenium_manager();
+    let cmd_assert = cmd
+        .args(["--browser", "electron", "--browser-version", "36"])
+        .assert();
+    cmd_assert.success();
+}