[rust] Fix for non empty driver path in webdriver manager

[iampopovich]

User description

🔗 Related Issues

fixed #16215

💥 What does this PR do?

please notice that bug discovery was assisted by github copilot

This pull request improves how the application handles and logs information when running in offline mode, especially regarding driver and browser paths. It also adds a new test to ensure the JSON output in offline mode includes the expected fields.

Offline mode handling and logging:

• Modified the log_driver_and_browser_path function in rust/src/main.rs to accept an is_offline argument and improved its logic to avoid logging driver unavailability errors when in offline mode. Now, it only logs an error if not offline and the driver path is missing.
• Updated calls to log_driver_and_browser_path in main to pass the new is_offline flag, ensuring consistent behavior based on connectivity status. [1] [2]

Testing improvements:

• Added a new test offline_json_output_includes_browser_path_test in rust/tests/offline_tests.rs to verify that when running in offline mode with JSON output, the result includes a browser_path field and appropriate log messages.

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

• Bug fix (backwards compatible)
• New feature (non-breaking change which adds functionality and tests!)

---

PR Type

Bug fix

---

Description

• Fix offline mode driver path handling and logging

• Add offline parameter to log_driver_and_browser_path function

• Prevent driver unavailability errors in offline mode

• Add JSON output test for offline mode validation

---

Diagram Walkthrough

flowchart LR
  A["Offline Mode Check"] --> B["log_driver_and_browser_path"]
  B --> C["Driver Path Validation"]
  C --> D["Conditional Error Logging"]
  E["JSON Output Test"] --> F["Offline Mode Validation"]

Loading

File Walkthrough

	Relevant files
Bug fix	main.rs Enhanced offline mode driver path handling                              rust/src/main.rs Add is_offline parameter to log_driver_and_browser_path function Update function calls to pass offline status Modify driver path validation logic for offline mode Add offline mode handling in error scenarios +13/-2
Tests	offline_tests.rs Add offline JSON output validation test                                    rust/tests/offline_tests.rs Add new test offline_json_output_includes_browser_path_test Validate JSON output structure in offline mode Check for browser_path field and offline mode logs +41/-0

---

[qodo-merge-pro]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
⚪	No custom compliance provided Follow the guide to enable custom compliance check.
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Use Option<&Path> for driver path Refactor log_driver_and_browser_path to accept Option<&Path> instead of &Path. This makes the absence of a driver path explicit, avoiding the use of an empty path as a special value. Examples: rust/src/main.rs [301-321] fn log_driver_and_browser_path( log: &Logger, driver_path: &Path, browser_path: &str, receiver: &Receiver<String>, is_offline: bool, ) { if let Ok(err) = receiver.try_recv() { log.warn(err); } ... (clipped 11 lines) rust/src/main.rs [282-288] log_driver_and_browser_path( log, &Path::new(""), &selenium_manager.get_browser_path_or_latest_from_cache(), selenium_manager.get_receiver(), selenium_manager.is_offline(), ); Solution Walkthrough: Before: // in main() if selenium_manager.is_offline() { log.warn(&err); log_driver_and_browser_path( log, &Path::new(""), // Using an empty path to signal no driver &selenium_manager.get_browser_path_or_latest_from_cache(), selenium_manager.get_receiver(), selenium_manager.is_offline(), ); flush_and_exit(OK, log, Some(err)); } fn log_driver_and_browser_path(log: &Logger, driver_path: &Path, ..., is_offline: bool) { if !driver_path.as_os_str().is_empty() && driver_path.exists() { log.info(format!("{}{}", DRIVER_PATH, driver_path.display())); } else if !is_offline { log.error(format!("Driver unavailable: {}", driver_path.display())); flush_and_exit(UNAVAILABLE, log, None); } } After: // in main() if selenium_manager.is_offline() { log.warn(&err); log_driver_and_browser_path( log, None, // Explicitly passing None &selenium_manager.get_browser_path_or_latest_from_cache(), selenium_manager.get_receiver(), selenium_manager.is_offline(), ); flush_and_exit(OK, log, Some(err)); } fn log_driver_and_browser_path(log: &Logger, driver_path: Option<&Path>, ..., is_offline: bool) { match driver_path { Some(path) if path.exists() => { log.info(format!("{}{}", DRIVER_PATH, path.display())); } _ if is_offline => { /* No driver path is acceptable in offline mode */ } Some(path) => { /* path does not exist and not offline */ log.error(format!("Driver unavailable: {}", path.display())); flush_and_exit(UNAVAILABLE, log, None); } None => { /* No path provided and not offline */ log.error("Driver unavailable"); flush_and_exit(UNAVAILABLE, log, None); } } } Suggestion importance[1-10]: 7 __ Why: This is a strong suggestion that improves code design by replacing a "magic value" (&Path::new("")) with the idiomatic Rust Option type, making the code safer and more explicit.	Medium
General	Assert driver path is absent Enhance the offline_json_output_includes_browser_path_test by adding an assertion to confirm that the driver_path field is null in the JSON output when running in offline mode. rust/tests/offline_tests.rs [63-68] assert!( json["result"]["browser_path"].is_string(), "browser_path should be a string" ); +assert!( + json["result"]["driver_path"].is_null(), + "driver_path should be null" +); assert!(json["logs"].is_array(), "Logs should be an array"); Apply / Chat Suggestion importance[1-10]: 7 __ Why: This suggestion improves the new test by adding a crucial assertion to verify that driver_path is null in the JSON output for the offline failure case, making the test more comprehensive and aligned with the expected behavior.	Medium
	Improve driver path validation logic Refactor the driver path validation to explicitly check for an empty path before checking for its existence. This provides a more informative error message if the path is empty when not in offline mode. rust/src/main.rs [312-317] -if !driver_path.as_os_str().is_empty() && driver_path.exists() { +if driver_path.as_os_str().is_empty() { + if !is_offline { + log.error("Driver path is empty"); + flush_and_exit(UNAVAILABLE, log, None); + } +} else if driver_path.exists() { log.info(format!("{}{}", DRIVER_PATH, driver_path.display())); } else if !is_offline { log.error(format!("Driver unavailable: {}", driver_path.display())); flush_and_exit(UNAVAILABLE, log, None); } Apply / Chat Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies a potential for an uninformative error message and proposes a change that makes the log_driver_and_browser_path function more robust by handling an empty driver_path explicitly, improving error reporting.	Low
Learned best practice	Assert on structured JSON logs Avoid stringifying the whole JSON to search substrings; iterate and assert on actual log fields to make the test robust and precise. rust/tests/offline_tests.rs [69-73] -let logs_str = json["logs"].to_string(); +let logs = json["logs"].as_array().expect("Logs should be an array"); assert!( - logs_str.contains("offline mode"), + logs.iter().any(|entry| entry.to_string().contains("offline mode")), "Should mention offline mode in logs" ); Apply / Chat Suggestion importance[1-10]: 5 __ Why: Relevant best practice - Validate inputs and states early with precise checks to prevent logic errors; prefer structured inspection over stringified JSON for assertions.	Low
Update

[iampopovich]

This conflict was caused by PR #16368. At first glance, it seems to be about formatting changes in the main.rs file. I'll investigate and fix it later today.

[iampopovich]

resolved in 2eeadab