[java][sm] Configure Selenium Manager environment from System Properties

[kool79]

User description

Fixes #13857

Allow to configure Selenium Manager from java bindings with help of java System Properties

Description

Configure a Selenium Manager with a system properties from Java

When create process for Selenium Manager, pass the System Properties from java, which start with SE_ prefix, to the Selenium Manager process. With this approach, non-empty system properties, which are defined in java, have a higher priority over the environment variables (for Selenium Manager process only)

Motivation and Context

See #13857

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [x] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• [x] I have read the contributing document.
• [x] My change requires a change to the documentation.
• [ ] I have updated the documentation accordingly.
• [ ] I have added tests to cover my changes.
• [ ] All new and existing tests passed.

---

Type

Enhancement

---

Description

• This PR introduces the ability to configure the Selenium Manager using Java system properties that start with the SE_ prefix.
• System properties are now prioritized over environment variables for configuring the Selenium Manager, providing more flexibility and control from within Java applications.
• The process builder for the Selenium Manager has been updated to include these system properties in its environment, ensuring that configurations are respected during runtime.

---

Changes walkthrough

Relevant files

Enhancement

SeleniumManager.java Enhance Selenium Manager Configuration via System Properties java/src/org/openqa/selenium/manager/SeleniumManager.java Added support for configuring Selenium Manager using Java system properties prefixed with SE_. Enhanced the process builder to include system properties in the environment of the Selenium Manager process. Modified cache path determination logic to prioritize system properties over environment variables. +23/-7

---

✨ PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[CLAassistant]

All committers have signed the CLA.

[codiumai-pr-agent-pro[bot]]

PR Description updated to latest commit (https://github.com/SeleniumHQ/selenium/commit/25964eb4c1e503cc31ee275014a5402b1731f05a)

• [ ] Copy walkthrough table to "Files Changed" Tab

[codiumai-pr-agent-pro[bot]]

PR Review

⏱️ Estimated effort to review [1-5]	3, because the PR involves changes to the environment configuration logic which can have wide-ranging effects. Understanding and verifying the new logic for prioritizing system properties over environment variables, and ensuring it doesn't introduce regressions or unexpected behavior, requires a thorough review.
🧪 Relevant tests	No
🔍 Possible issues	Possible Bug: The new system property handling logic might introduce concurrency issues if properties are changed by other parts of the application during the iteration. The comment about reading property with 'default' value due to concurrency might not be sufficient to prevent issues.
	Possible Bug: The change in cache path logic might cause issues if the system property or environment variable is not set, as it now defaults to an empty string which could lead to incorrect path computations.
🔒 Security concerns	No

---

✨ Review tool usage guide:

---

Overview: The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

• When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:

/review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...

• With a configuration file, use the following template:

[pr_reviewer]
some_config1=...
some_config2=...

See the review usage page for a comprehensive guide on using this tool.

[codiumai-pr-agent-pro[bot]]

PR Code Suggestions

Category	Suggestions
Enhancement	Improve the fetching of environment-specific properties by using environment variables directly. Replace the use of System.getProperties() with System.getenv() for consistency and to avoid potential issues with properties not being set as environment variables. This change ensures that the environment-specific properties are correctly fetched from the environment variables, which is typically the expected source in deployment environments. java/src/org/openqa/selenium/manager/SeleniumManager.java [126-133] -Properties properties = System.getProperties(); -for (String name : properties.stringPropertyNames()) { +Map<String, String> env = System.getenv(); +for (String name : env.keySet()) { if (name.startsWith(SE_ENV_PREFIX)) { - String value = properties.getProperty(name, ""); - if (!value.isEmpty()) + String value = env.get(name); + if (value != null && !value.isEmpty()) processBuilder.environment(name, value); } }
	Enhance thread safety by using concurrent collections for environment variables. To ensure thread safety and avoid potential race conditions with mutable static fields, consider using ConcurrentHashMap for environment variables in processBuilder. java/src/org/openqa/selenium/manager/SeleniumManager.java [132] -processBuilder.environment(name, value); +processBuilder.environment().putIfAbsent(name, value);
Bug	Prevent potential NullPointerException by ensuring cachePath is not null before modification. Add a null check for cachePath before attempting to replace the HOME placeholder to prevent potential NullPointerException. java/src/org/openqa/selenium/manager/SeleniumManager.java [265] -cachePath = cachePath.replace(HOME, System.getProperty("user.home")); +if (cachePath != null) { + cachePath = cachePath.replace(HOME, System.getProperty("user.home")); +}
Maintainability	Improve command construction readability and maintainability in process builder. Use String.join to construct the command for processBuilder to enhance readability and maintainability. java/src/org/openqa/selenium/manager/SeleniumManager.java [136] -processBuilder.command(binary.toAbsolutePath().toString(), arguments).start(); +processBuilder.command(String.join(" ", binary.toAbsolutePath().toString(), String.join(" ", arguments))).start();
Best practice	Use a more specific exception for clarity in error handling related to environment configuration. Consider using a more specific exception than IOException for errors related to environment variable configuration to provide clearer error handling. java/src/org/openqa/selenium/manager/SeleniumManager.java [256] -private Path getBinaryInCache(String binaryName) throws IOException { +private Path getBinaryInCache(String binaryName) throws ConfigurationException {

---

✨ Improve tool usage guide:

---

Overview: The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

• When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:

/improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...

• With a configuration file, use the following template:

[pr_code_suggestions]
some_config1=...
some_config2=...

See the improve usage page for a comprehensive guide on using this tool.

[diemol]

@kool79 do you think you can help us and update the docs for Selenium Manager, please?

[titusfortner]

@diemol do we need to do this in the other bindings?

[diemol]

No. This was necessary because there is no straightforward way to create environment variables in Java.