From 553791ccf4da57c464be7a8a6dd354fc18a7f730 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Boni=20Garc=C3=ADa?= Date: Sun, 24 Sep 2023 22:33:57 +0200 Subject: [PATCH] [rust] Fix config setup in Selenium Manager (#12807) * [rust] Fix config setup in Selenium Manager * [rust] Reuse logic for escaping paths for config cache path * [rust] Revert canonicalice logic for cache path * [rust] Update checksum in Cargo.Bazel.lock --- rust/Cargo.Bazel.lock | 2 +- rust/src/config.rs | 3 ++ rust/src/lib.rs | 36 ++++++++++++++++-------- rust/src/main.rs | 14 +++++++--- rust/tests/config_tests.rs | 56 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 94 insertions(+), 17 deletions(-) create mode 100644 rust/tests/config_tests.rs diff --git a/rust/Cargo.Bazel.lock b/rust/Cargo.Bazel.lock index 359d5556f5db6..d15c161eb65e2 100644 --- a/rust/Cargo.Bazel.lock +++ b/rust/Cargo.Bazel.lock @@ -1,5 +1,5 @@ { - "checksum": "24231be79f3b3f23c29c0093148f7183cb564cd13ac0fe297c6e5efe9e6311a8", + "checksum": "437c56863604d3ef83e56701f4a5d8948553a0c963b7c970716fceabcc75ec10", "crates": { "addr2line 0.19.0": { "name": "addr2line", diff --git a/rust/src/config.rs b/rust/src/config.rs index a917347ad1b8e..b4d8918f2ab88 100644 --- a/rust/src/config.rs +++ b/rust/src/config.rs @@ -185,6 +185,9 @@ impl StringKey<'_> { // to be discovered in the first place and stored globally (on CACHE_PATH) return check_cache_path(result, default_value); } + if !result.is_empty() { + return result; + } } default_value } diff --git a/rust/src/lib.rs b/rust/src/lib.rs index bbb4c415cb011..8f0a665cc10df 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -935,18 +935,28 @@ pub trait SeleniumManager { } fn canonicalize_path(&self, path_buf: PathBuf) -> String { - let canon_path = path_buf_to_string(path_buf.as_path().canonicalize().unwrap_or(path_buf)); + let mut canon_path = path_buf_to_string( + path_buf + .as_path() + .canonicalize() + .unwrap_or(path_buf.clone()), + ); if WINDOWS.is(self.get_os()) { - canon_path.replace(UNC_PREFIX, "") - } else { - canon_path + canon_path = canon_path.replace(UNC_PREFIX, "") } + if !path_buf_to_string(path_buf.clone()).eq(&canon_path) { + self.get_logger().trace(format!( + "Path {} has been canonicalized to {}", + path_buf.display(), + canon_path + )); + } + canon_path } fn get_escaped_path(&self, string_path: String) -> String { - let original_path = string_path.clone(); - let mut escaped_path = string_path; - let path = Path::new(&original_path); + let mut escaped_path = string_path.clone(); + let path = Path::new(&string_path); if path.exists() { escaped_path = self.canonicalize_path(path.to_path_buf()); @@ -957,14 +967,16 @@ pub trait SeleniumManager { Command::new_single(format_one_arg(ESCAPE_COMMAND, escaped_path.as_str())); escaped_path = run_shell_command("bash", "-c", escape_command).unwrap_or_default(); if escaped_path.is_empty() { - escaped_path = original_path.clone(); + escaped_path = string_path.clone(); } } } - self.get_logger().trace(format!( - "Original path: {} - Escaped path: {}", - original_path, escaped_path - )); + if !string_path.eq(&escaped_path) { + self.get_logger().trace(format!( + "Path {} has been escaped to {}", + string_path, escaped_path + )); + } escaped_path } diff --git a/rust/src/main.rs b/rust/src/main.rs index e4fbab3a7fc1a..2fd4b2a837b14 100644 --- a/rust/src/main.rs +++ b/rust/src/main.rs @@ -15,7 +15,7 @@ // specific language governing permissions and limitations // under the License. -use std::path::PathBuf; +use std::path::Path; use std::process::exit; use clap::Parser; @@ -135,8 +135,14 @@ fn main() { let trace = cli.trace || BooleanKey("trace", false).get_value(); let log = Logger::create(&cli.output, debug, trace); let grid = cli.grid; - let browser_name: String = cli.browser.unwrap_or_default(); - let driver_name: String = cli.driver.unwrap_or_default(); + let mut browser_name: String = cli.browser.unwrap_or_default(); + let mut driver_name: String = cli.driver.unwrap_or_default(); + if browser_name.is_empty() { + browser_name = StringKey(vec!["browser"], "").get_value(); + } + if driver_name.is_empty() { + driver_name = StringKey(vec!["driver"], "").get_value(); + } let mut selenium_manager: Box = if !browser_name.is_empty() { get_manager_by_browser(browser_name).unwrap_or_else(|err| { @@ -224,7 +230,7 @@ fn main() { }); } -fn log_driver_and_browser_path(log: &Logger, driver_path: &PathBuf, browser_path: &str) { +fn log_driver_and_browser_path(log: &Logger, driver_path: &Path, browser_path: &str) { if driver_path.exists() { log.info(format!("{}{}", DRIVER_PATH, driver_path.display())); } else { diff --git a/rust/tests/config_tests.rs b/rust/tests/config_tests.rs new file mode 100644 index 0000000000000..93180314e4e47 --- /dev/null +++ b/rust/tests/config_tests.rs @@ -0,0 +1,56 @@ +// Licensed to the Software Freedom Conservancy (SFC) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The SFC licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use assert_cmd::Command; +use std::env; +use std::fs::File; +use std::io::{BufWriter, Write}; + +use crate::common::{assert_browser, assert_driver}; +use rstest::rstest; +use tempfile::Builder; + +mod common; + +#[rstest] +#[case("chrome")] +#[case("firefox")] +#[case("edge")] +fn config_test(#[case] browser_name: String) { + let tmp_dir = Builder::new().prefix("sm-config-test").tempdir().unwrap(); + let config_path = tmp_dir.path().join("se-config.toml"); + let config_file = File::create(config_path.as_path()).unwrap(); + let mut writer = BufWriter::new(config_file); + writer + .write_all(format!(r#"browser="{}""#, browser_name).as_bytes()) + .unwrap(); + writer.flush().unwrap(); + + let mut cmd = Command::new(env!("CARGO_BIN_EXE_selenium-manager")); + cmd.args([ + "--output", + "json", + "--cache-path", + tmp_dir.path().to_str().unwrap(), + ]) + .assert() + .success() + .code(0); + + assert_driver(&mut cmd); + assert_browser(&mut cmd); +}