[java][grid]: Set test name to video file name in dynamic grid

[VietND96]

User description

Thanks for contributing to Selenium! A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines. Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

[java][grid]: Set test name to video file name in dynamic grid

Motivation and Context

This is based on the support of Metadata in tests. When running test with dynamic grid, by adding metadata to your tests, the video file name will extract the value of capability se:name and then set to output the video file name.

For example, in Python binding:

from selenium.webdriver.chrome.options import Options as ChromeOptions
from selenium import webdriver

options = ChromeOptions()
options.set_capability('se:recordVideo', True)
options.set_capability('se:screenResolution', '1920x1080')
options.set_capability('se:name', 'test_visit_basic_auth_secured_page (ChromeTests)')
driver = webdriver.Remote(options=options, command_executor="http://localhost:4444")
driver.get("https://selenium.dev")
driver.quit()

After test executed, under /assets you can see the video file name under /<sessionId>/test_visit_basic_auth_secured_page_ChromeTests.mp4

File name will be trimmed to 255 characters to avoid long file names. Moreover, space character will be replaced by _ and only characters alphabets, numbers, - (hyphen), _ (underscore) are retained in the file name.

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.
• [ ] My change requires a change to the documentation.
• [ ] I have updated the documentation accordingly.
• [ ] I have added tests to cover my changes.
• [x] All new and existing tests passed.

---

Type

enhancement

---

Description

• Refactored the handling of environment variables in DockerSessionFactory for better clarity and separation of concerns.
• Introduced functionality to set the video file name based on the se:name capability, enhancing traceability of test artifacts.
• Implemented sanitization and truncation for video file names to ensure compatibility and prevent excessively long file names.
• Updated the JDK version in the IDE configuration to JDK 17, aligning with updated language features and support.

---

Changes walkthrough

	Relevant files
Enhancement	DockerSessionFactory.java Refactor Env Vars Handling and Set Video File Name from Test Name java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java Refactored environment variable setting into separate methods for clarity. Added capability to set video file name based on the se:name capability in session request. Video file name sanitization and truncation logic added. +41/-16
Configuration changes	misc.xml Update Project JDK Version in IDE Configuration                    .idea/misc.xml Updated project JDK version from 11 to 17 in IDE configuration. +2/-2

---

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

[qodo-code-review[bot]]

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

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

[qodo-code-review[bot]]

PR Review

⏱️ Estimated effort to review [1-5]	3, because the PR involves changes in the handling of environment variables and capabilities, which are critical for the correct functioning of the Selenium Grid. The logic for setting video file names based on test names introduces new methods and modifies existing ones, requiring careful review to ensure compatibility and correctness.
🧪 Relevant tests	No
🔍 Possible issues	Possible Bug: The method getTestName uses regex to sanitize the test name and limits its length to 251 characters. This might inadvertently cut off important information from the test name, especially if the test name contains significant data towards the end.
	Data Loss: The replacement of spaces with underscores and removal of all characters except alphanumerics, hyphens, and underscores in test names might not be compatible with all systems or might lead to loss of important distinguishing information.
🔒 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.

[qodo-code-review[bot]]

PR Code Suggestions

Category	Suggestions
Enhancement	Simplify the method of setting environment variables by removing unnecessary map creation. Refactor the method setEnvVarsToContainer to directly accept the environment variables from the system instead of creating a new map and then filtering it. This will reduce the overhead of creating an unnecessary map and streamline the process of setting environment variables. java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java [313-318] -Map<String, String> seEnvVars = System.getenv(); -seEnvVars.entrySet().stream() - .filter( - entry -> - entry.getKey().startsWith("SE_") || entry.getKey().equalsIgnoreCase("LANGUAGE")) +System.getenv().entrySet().stream() + .filter(entry -> entry.getKey().startsWith("SE_") || entry.getKey().equalsIgnoreCase("LANGUAGE")) .forEach(entry -> envVars.put(entry.getKey(), entry.getValue()));
	Add specific exception handling for better error management and debugging. Consider using a more specific exception handling for the TimeZone and Dimension retrieval in the setCapsToEnvVars method. This will help in debugging and maintaining the code by providing more detailed error information if the capabilities do not contain the expected types or values. java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java [330-331] -Optional<TimeZone> timeZone = ofNullable(getTimeZone(sessionRequestCapabilities)); -timeZone.ifPresent(zone -> envVars.put("TZ", zone.getID())); +try { + TimeZone timeZone = getTimeZone(sessionRequestCapabilities).orElseThrow(() -> new IllegalArgumentException("Time zone not specified in session capabilities")); + envVars.put("TZ", timeZone.getID()); +} catch (IllegalArgumentException e) { + // Handle the case where time zone is not provided or invalid + // Log or throw further as needed +}
	Use regex and utility method for string operations to clean and truncate the test name. Instead of manually replacing characters and checking the length in getTestName, consider using a regex pattern to clean the string and a utility method to truncate it. This approach is cleaner and reduces the complexity within the method. java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java [385-389] -name = name.replaceAll(" ", "_").replaceAll("[^a-zA-Z0-9_-]", ""); -if (name.length() > 251) { - name = name.substring(0, 251); -} +name = name.replaceAll("[^\\w-]", "_"); +name = StringUtils.abbreviate(name, 251);
Bug	Add null check before calling toString() to prevent NullPointerException. To ensure the robustness of the getTestName method, add a null check for the capability retrieval before calling toString() on it. This prevents potential NullPointerException if the capability is not set. java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java [382-384] Optional<Object> testName = ofNullable(sessionRequestCapabilities.getCapability("se:name")); -if (testName.isPresent()) { +if (testName.isPresent() && testName.get() != null) { String name = testName.get().toString();
Maintainability	Refactor repeated code into a common method to improve maintainability. Refactor the repeated code for setting environment variables in getBrowserContainerEnvVars and getVideoContainerEnvVars by creating a common method that both can call. This will reduce code duplication and improve maintainability. java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java [371-374] -setEnvVarsToContainer(envVars); -setCapsToEnvVars(sessionRequestCapabilities, envVars); +setCommonEnvVars(sessionRequestCapabilities, envVars);

---

✨ 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]

I cannot update the fork with the latest from trunk.

[VietND96]

@diemol, I synced this, can you review this first?