Docker: set/unset stereotype via env var SE_NODE_BROWSER_VERSION and SE_BROWSER_BINARY_LOCATION

[VietND96]

User description

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

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

Description

By default, Node stereotype sets browserVersion in short format e.g 131.0.
In a case, if want to unset browserVersion (leave it as empty). Setting env var SE_NODE_BROWSER_VERSION=

Similarly, for SE_BROWSER_BINARY_LOCATION to unset or set another location in the container for browser binary or based on official image, you rebuild your own, then can set via this env var.

Motivation and Context

Types of changes

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

Checklist

• 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.
• All new and existing tests passed.

---

PR Type

Enhancement

---

Description

• Added ability to control browser version through SE_NODE_BROWSER_VERSION environment variable
• Added support for custom browser binary location via SE_BROWSER_BINARY_LOCATION environment variable
• Enhanced configuration generation scripts to handle empty/unset browser version
• Updated test configurations to demonstrate browser version configuration
• Fixed variable name inconsistency for browser binary location
• Added proper file existence checks in configuration scripts

---

Changes walkthrough 📝

	Relevant files
Configuration changes	Dockerfile Add browser version environment variable configuration      NodeBase/Dockerfile Added new environment variable SE_NODE_BROWSER_VERSION with default value "stable" +1/-0
Enhancement	generate_config Update browser configuration generation logic                        NodeBase/generate_config Modified browser version check to consider SE_NODE_BROWSER_VERSION environment variable Updated browser binary location handling with SE_BROWSER_BINARY_LOCATION variable Fixed variable name typo from SE__BROWSER_BINARY_LOCATION to SE_BROWSER_BINARY_LOCATION +4/-4      generate_config Enhance standalone configuration generation                            Standalone/generate_config Added file existence checks for browser configuration files Implemented conditional browser version and binary location settings Updated stereotype generation with new environment variables +10/-4
Tests	docker-compose-v3-test-parallel.yml Add browser version configuration to test services              tests/docker-compose-v3-test-parallel.yml Added SE_NODE_BROWSER_VERSION environment variable to multiple services +3/-0      docker-compose-v3-test-standalone.yml Add browser version configuration to standalone test          tests/docker-compose-v3-test-standalone.yml Added SE_NODE_BROWSER_VERSION environment variable to standalone service +1/-0

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Logic Flow The condition for setting SE_NODE_BROWSER_VERSION checks for both file existence and "stable" value, but doesn't handle the case when SE_NODE_BROWSER_VERSION is empty string. This might lead to unexpected behavior when trying to unset the browser version. Code Smell The else branch for SE_NODE_STEREOTYPE assignment is redundant as it just assigns the variable to itself. This could be simplified or removed. Code Duplication The stereotype generation logic is duplicated between NodeBase/generate_config and Standalone/generate_config. Consider extracting this into a shared script to maintain consistency.

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible issue	Add validation for empty browser version to prevent invalid JSON generation in the stereotype configuration Add input validation to ensure SE_NODE_BROWSER_VERSION is not empty when used in the stereotype generation, as an empty value could lead to invalid JSON. NodeBase/generate_config [67] -SE_NODE_STEREOTYPE="{\"browserName\": \"${SE_NODE_BROWSER_NAME}\", \"browserVersion\": \"${SE_NODE_BROWSER_VERSION}\", \"platformName\": \"Linux\", ${SE_BROWSER_BINARY_LOCATION}, \"se:containerName\": \"${SE_NODE_CONTAINER_NAME}\"}" +if [[ -n "${SE_NODE_BROWSER_VERSION}" ]]; then + SE_NODE_STEREOTYPE="{\"browserName\": \"${SE_NODE_BROWSER_NAME}\", \"browserVersion\": \"${SE_NODE_BROWSER_VERSION}\", \"platformName\": \"Linux\", ${SE_BROWSER_BINARY_LOCATION}, \"se:containerName\": \"${SE_NODE_CONTAINER_NAME}\"}" +else + SE_NODE_STEREOTYPE="{\"browserName\": \"${SE_NODE_BROWSER_NAME}\", \"platformName\": \"Linux\", ${SE_BROWSER_BINARY_LOCATION}, \"se:containerName\": \"${SE_NODE_CONTAINER_NAME}\"}" +fi Apply this suggestion Suggestion importance[1-10]: 8 Why: The suggestion addresses a critical issue where an empty browser version could lead to malformed JSON in the stereotype configuration, which could cause runtime failures. The proposed validation ensures robust JSON generation.	8
	Handle empty binary location to prevent JSON syntax errors in the configuration Validate SE_BROWSER_BINARY_LOCATION before using it in the JSON string to prevent syntax errors if the variable is empty. NodeBase/generate_config [67] -SE_NODE_STEREOTYPE="{\"browserName\": \"${SE_NODE_BROWSER_NAME}\", \"browserVersion\": \"${SE_NODE_BROWSER_VERSION}\", \"platformName\": \"Linux\", ${SE_BROWSER_BINARY_LOCATION}, \"se:containerName\": \"${SE_NODE_CONTAINER_NAME}\"}" +binary_location="" +if [[ -n "${SE_BROWSER_BINARY_LOCATION}" ]]; then + binary_location="${SE_BROWSER_BINARY_LOCATION}," +fi +SE_NODE_STEREOTYPE="{\"browserName\": \"${SE_NODE_BROWSER_NAME}\", \"browserVersion\": \"${SE_NODE_BROWSER_VERSION}\", \"platformName\": \"Linux\", ${binary_location} \"se:containerName\": \"${SE_NODE_CONTAINER_NAME}\"}" Apply this suggestion Suggestion importance[1-10]: 8 Why: This suggestion fixes a potential JSON syntax error that could occur when SE_BROWSER_BINARY_LOCATION is empty, as it would leave a dangling comma in the JSON string. The solution properly handles empty values to maintain valid JSON structure.	8