[java][grid]: video file name set independently in dynamic grid via se:videoName

[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

Motivation and Context

This is an improvement of https://github.com/SeleniumHQ/selenium/pull/13907

In dynamic grid, whenever capability se:name (which shows a test name instead of the session ID in the Grid UI) is set via binding, it is also used to set the output video record file name.

To solve a requirement is

People are setting the cap se:name. However, they don't want this name to be set to a video record file. Or they want an option to control the video record file name independently from the test name shown in the Grid UI.

Capability se:videoName will be used to set video record file name independently. If the capability se:videoName is not set, the se:name is still being used to set for the video record (without regression broken in part of #13907).

For example:

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)')
options.set_capability('se:videoName', 'video')
driver = webdriver.Remote(options=options, command_executor="http://localhost:4444")
driver.get("https://selenium.dev")
driver.quit()

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

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.

---

PR Type

enhancement

---

Description

• Added support for setting video file names independently in the dynamic grid using the se:videoName capability.
• If se:videoName is not set, the existing se:name capability will be used as a fallback to name the video file.
• Refactored the method to retrieve capability names for video file naming to improve code maintainability.

---

Changes walkthrough 📝

	Relevant files
Enhancement	DockerSessionFactory.java Support independent video file naming via `se:videoName` capability. java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java Added logic to set video file name independently using se:videoName capability. Fallback to se:name capability if se:videoName is not provided. Refactored method to generalize capability name retrieval for video file naming. +12/-4

---

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

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

PR Reviewer Guide 🔍

⏱️ Estimated effort to review [1-5]	2
🧪 Relevant tests	No
🔒 Security concerns	No
⚡ Key issues to review	Possible Bug: The method getVideoFileName is used to fetch both se:videoName and se:name capabilities. However, the method name suggests it is specifically for getting video file names. Consider renaming the method to reflect its generalized use, such as getCapabilityAsString.
	Code Duplication: The logic for sanitizing the name string is duplicated in the getVideoFileName method. Consider extracting this logic into a separate method to improve code maintainability and reduce duplication.

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

PR Code Suggestions ✨

Category	Suggestion	Score
Enhancement	Combine the Optional checks for videoName and testName into a single statement ___ **Combine the two Optional checks for videoName and testName into a single statement to simplify the code and reduce redundancy.** [java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java [376-383]](https://github.com/SeleniumHQ/selenium/pull/14148/files#diff-a05d31304c9c75c40d9d11ef388a2c669f1f8c35e943ed8dcbc32e1da7694c5eR376-R383) ```diff -Optional videoName = - ofNullable(getVideoFileName(sessionRequestCapabilities, "se:videoName")); -if (videoName.isPresent()) { - envVars.put("SE_VIDEO_FILE_NAME", String.format("%s.mp4", videoName.get())); -} else { - Optional testName = - ofNullable(getVideoFileName(sessionRequestCapabilities, "se:name")); - testName.ifPresent(name -> envVars.put("SE_VIDEO_FILE_NAME", String.format("%s.mp4", name))); -} +Optional videoName = ofNullable(getVideoFileName(sessionRequestCapabilities, "se:videoName")) + .or(() -> ofNullable(getVideoFileName(sessionRequestCapabilities, "se:name"))); +videoName.ifPresent(name -> envVars.put("SE_VIDEO_FILE_NAME", String.format("%s.mp4", name))); ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 8 Why: This suggestion correctly identifies a way to simplify the code by combining two `Optional` checks into one, reducing redundancy and improving readability.	8
Possible bug	Add a null check for sessionRequestCapabilities before accessing its capabilities ___ **Add a null check for sessionRequestCapabilities before calling getCapability to prevent potential NullPointerException.** [java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java [389-390]](https://github.com/SeleniumHQ/selenium/pull/14148/files#diff-a05d31304c9c75c40d9d11ef388a2c669f1f8c35e943ed8dcbc32e1da7694c5eR389-R390) ```diff +if (sessionRequestCapabilities == null) { + return null; +} Optional	7
Maintainability	Use a constant for the video file extension to improve maintainability ___ **Use a constant for the video file extension to avoid hardcoding the ".mp4" string multiple times, which improves maintainability.** [java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java [379-383]](https://github.com/SeleniumHQ/selenium/pull/14148/files#diff-a05d31304c9c75c40d9d11ef388a2c669f1f8c35e943ed8dcbc32e1da7694c5eR379-R383) ```diff -envVars.put("SE_VIDEO_FILE_NAME", String.format("%s.mp4", videoName.get())); -testName.ifPresent(name -> envVars.put("SE_VIDEO_FILE_NAME", String.format("%s.mp4", name))); +final String VIDEO_FILE_EXTENSION = ".mp4"; +envVars.put("SE_VIDEO_FILE_NAME", String.format("%s" + VIDEO_FILE_EXTENSION, videoName.get())); +testName.ifPresent(name -> envVars.put("SE_VIDEO_FILE_NAME", String.format("%s" + VIDEO_FILE_EXTENSION, name))); ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 6 Why: Using a constant for repeated string literals like file extensions can improve maintainability and make future changes easier. This suggestion is valid and applicable to the new code in the PR.	6