[java] Use Environment variable to set driver location

[pujagani]

User description

🔗 Related Issues

Related to #14045

💥 What does this PR do?

Implements using environment variable for driver location in addition to system properties.

🔧 Implementation Notes

Environment variables are global in nature so that is given priority over system property if set.

💡 Additional Considerations

🔄 Types of changes

• Cleanup (formatting, renaming)
• Bug fix (backwards compatible)
• New feature (non-breaking change which adds functionality and tests!)
• Breaking change (fix or feature that would cause existing functionality to change)

---

PR Type

Enhancement, Tests

---

Description

• Add support for environment variables to specify driver locations

• Prioritize environment variable over system property for driver path

• Update and expand unit tests for new driver location logic

• Add new test dependencies for environment variable mocking

---

Changes walkthrough 📝

	Relevant files
Enhancement	7 files ChromeDriverService.java Add environment variable support for ChromeDriver location +6/-0      EdgeDriverService.java Add environment variable support for EdgeDriver location  +6/-0      GeckoDriverService.java Add environment variable support for GeckoDriver location +6/-0      InternetExplorerDriverService.java Add environment variable support for IEDriver location      +6/-0      DriverFinder.java Update driver path resolution to check environment variable first +13/-10  DriverService.java Add environment variable getter to DriverService base class +4/-0      SafariDriverService.java Add environment variable support for SafariDriver location +6/-0
Tests	1 files DriverFinderTest.java Add and update tests for environment variable driver location +68/-0
Dependencies	3 files MODULE.bazel Add system-stubs dependencies for environment variable testing +2/-0      maven_install.json Update Maven dependencies for system-stubs and related packages +45/-6    BUILD.bazel Add system-stubs dependencies to test build                            +2/-0

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ❌ 1234 - Not compliant Non-compliant requirements: • Fix issue where JavaScript in link's href is not triggered on click() in Firefox 42.0 with Selenium 2.48.0/2.48.2 • Ensure click() properly executes JavaScript in href attributes • Maintain backward compatibility with Selenium 2.47.1 behavior 5678 - Not compliant Non-compliant requirements: • Fix "Error: ConnectFailure (Connection refused)" when instantiating ChromeDriver • Address issue where only the first ChromeDriver instance works without errors • Ensure subsequent ChromeDriver instantiations don't produce connection errors

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Environment Variable Handling The implementation checks for environment variables before system properties, which is a change in priority that could affect existing configurations. Verify this prioritization is intentional and documented. result = new Result(System.getenv(service.getDriverEnvironmentVariable())); if (result.getDriverPath() == null) { result = new Result(System.getProperty(service.getDriverProperty())); if (result.getDriverPath() == null) { List<String> arguments = toArguments(); result = seleniumManager.getBinaryPaths(arguments); Require.state(options.getBrowserName(), Path.of(result.getBrowserPath())) .isExecutable(); } else { LOG.fine( String.format( "Skipping Selenium Manager, path to %s found in system property: %s", driverName, result.getDriverPath())); } } Test Coverage While there are tests for environment variable priority over system properties, consider adding a test for when both environment variable and executable path are set to verify the correct precedence. @Test void environmentVariableTakePriorityOverSystemProperty() throws IOException { environment.set("ENVIRONMENT_VARIABLE_DRIVER_PATH", driverFile.toString()); when(service.getExecutable()).thenReturn(null); when(service.getDriverProperty()).thenReturn("property.ignores.selenium.manager"); when(service.getDriverEnvironmentVariable()).thenReturn("ENVIRONMENT_VARIABLE_DRIVER_PATH"); System.setProperty("property.ignores.selenium.manager", "path"); Capabilities capabilities = new ImmutableCapabilities("browserName", "chrome"); DriverFinder finder = new DriverFinder(service, capabilities); assertThat(finder.getDriverPath()).isEqualTo(driverFile.toString()); assertThat(finder.getBrowserPath()).isNull(); verify(service, times(1)).getExecutable(); verify(service, times(1)).getDriverName(); verify(service, times(1)).getDriverEnvironmentVariable(); }

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Add missing log message Add a log message when using the environment variable path similar to the one for system property. This would maintain consistency in logging and help with debugging. java/src/org/openqa/selenium/remote/service/DriverFinder.java [99-113] result = new Result(System.getenv(service.getDriverEnvironmentVariable())); if (result.getDriverPath() == null) { result = new Result(System.getProperty(service.getDriverProperty())); if (result.getDriverPath() == null) { List<String> arguments = toArguments(); result = seleniumManager.getBinaryPaths(arguments); Require.state(options.getBrowserName(), Path.of(result.getBrowserPath())) .isExecutable(); } else { LOG.fine( String.format( "Skipping Selenium Manager, path to %s found in system property: %s", driverName, result.getDriverPath())); } +} else { + LOG.fine( + String.format( + "Skipping Selenium Manager, path to %s found in environment variable: %s", + driverName, result.getDriverPath())); } Apply this suggestion Suggestion importance[1-10]: 7 __ Why: The suggestion adds a missing log message when a driver path is found via environment variable, maintaining consistency with the existing logging for system properties. This improves debugging capabilities and code consistency.	Medium
Learned best practice	Add null checks before accessing environment variables or system properties to prevent NullPointerExceptions Add null checks for the environment variable and property names before attempting to retrieve their values to prevent potential NullPointerExceptions. java/src/org/openqa/selenium/remote/service/DriverFinder.java [99-101] -result = new Result(System.getenv(service.getDriverEnvironmentVariable())); +String envVar = service.getDriverEnvironmentVariable(); +result = envVar != null ? new Result(System.getenv(envVar)) : new Result(null); if (result.getDriverPath() == null) { - result = new Result(System.getProperty(service.getDriverProperty())); + String property = service.getDriverProperty(); + result = property != null ? new Result(System.getProperty(property)) : new Result(null); Apply this suggestion Suggestion importance[1-10]: 6	Low
Update

[pujagani]

StressTest is failing in CI but passes locally.

[titusfortner]

This looks good; nice work figuring out how to stub the environment variables for the tests.