[py] Properly verify Selenium Manager exists

[cgoldberg]

User description

💥 What does this PR do?

This PR makes a slight change to the logic for finding the Selenium Manager binary, so it doesn't give a false positive if there is a directory named selenium-manager where the executable should be if you build/install from source or a local sdist package. This also adds a check to verify the Selenium Manager binary exists if its location is specified in an environment variable, so it's clear why it is failing.

It also converts some old string formatting to use f-strings and adds information to the docstring about where we look for the binary.

🔄 Types of changes

• Cleanup (formatting, renaming)
• Bug fix (backwards compatible)

---

PR Type

Bug fix, Tests

---

Description

• Fix false positive when directory named selenium-manager exists instead of executable

• Change compiled_path.exists() to compiled_path.is_file() for proper verification

• Convert string formatting to f-strings for consistency

• Enhance docstring with binary search order and error conditions

---

Diagram Walkthrough

flowchart LR
  A["Binary Detection Logic"] -->|Check env variable| B["SE_MANAGER_PATH"]
  A -->|Check compiled path| C["compiled_path.is_file"]
  C -->|Old: exists| D["False Positive<br/>on directory"]
  C -->|New: is_file| E["Correct verification<br/>of executable"]
  A -->|Check wheel package| F["Platform-specific binary"]
  A -->|Fail| G["WebDriverException"]

Loading

File Walkthrough

Relevant files

Bug fix

selenium_manager.py Fix binary detection and modernize string formatting          py/selenium/webdriver/common/selenium_manager.py Changed compiled_path.exists() to compiled_path.is_file() to properly verify executable instead of accepting directories Converted three logger calls from old string formatting (%s) to f-strings Enhanced docstring with numbered list of binary search locations and additional error condition documentation Added blank line after license header for PEP 8 compliance +14/-6

---

[qodo-code-review]

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
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Action Logging: The new code paths that determine and validate the Selenium Manager binary location do not add or update any audit logging for critical actions, which may be acceptable for library code but cannot be verified from this diff alone. Referred Code if (env_path := os.getenv("SE_MANAGER_PATH")) is not None: logger.debug(f"Selenium Manager set by env SE_MANAGER_PATH to: {env_path}") path = Path(env_path) elif compiled_path.is_file(): path = compiled_path else: allowed = { ("darwin", "any"): "macos/selenium-manager", ("win32", "any"): "windows/selenium-manager.exe", ("cygwin", "any"): "windows/selenium-manager.exe", ("linux", "x86_64"): "linux/selenium-manager", ("freebsd", "x86_64"): "linux/selenium-manager", ("openbsd", "x86_64"): "linux/selenium-manager", } arch = platform.machine() if sys.platform in ("linux", "freebsd", "openbsd") else "any" if sys.platform in ["freebsd", "openbsd"]: logger.warning(f"Selenium Manager binary may not be compatible with {sys.platform}; verify settings") location = allowed.get((sys.platform, arch)) if location is None: ... (clipped 10 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Error Detail: Raised WebDriverException messages include platform and path details which are likely acceptable for internal exceptions but could be sensitive if surfaced to end users; visibility cannot be determined from this diff. Referred Code location = allowed.get((sys.platform, arch)) if location is None: raise WebDriverException(f"Unsupported platform/architecture combination: {sys.platform}/{arch}") path = Path(__file__).parent.joinpath(location) if path is None or not path.is_file(): raise WebDriverException(f"Unable to obtain working Selenium Manager binary; {path}") Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

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

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Learned best practice	✅ Validate environment variable path Suggestion Impact: The commit changed the code to create a Path from SE_MANAGER_PATH and only use it if it is a file; otherwise it sets path to None, effectively validating the environment variable. code diff: if (env_path := os.getenv("SE_MANAGER_PATH")) is not None: logger.debug(f"Selenium Manager set by env SE_MANAGER_PATH to: {env_path}") - path = Path(env_path) + path_candidate = Path(env_path) + path = path_candidate if path_candidate.is_file() else None Validate that SE_MANAGER_PATH points to an existing executable file and ignore or error out if it does not. py/selenium/webdriver/common/selenium_manager.py [81-83] if (env_path := os.getenv("SE_MANAGER_PATH")) is not None: logger.debug(f"Selenium Manager set by env SE_MANAGER_PATH to: {env_path}") - path = Path(env_path) + env_path = env_path.strip() + candidate = Path(env_path) + if candidate.is_file(): + path = candidate + else: + logger.warning(f"SE_MANAGER_PATH does not point to a file: {env_path}") [Suggestion processed] Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Validate and sanitize external inputs such as environment variables before use to avoid invalid paths or injection issues.	Low
Update

[navin772]

LGTM!