fix: conflict config.toml in browser containers when node-docker volumes is shared

[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

fix: conflict config.toml in browser containers when node-docker volumes is shared

Motivation and Context

After harmonized mount points to /opt/selenium/config.toml (#2343). Once volumes config is shared to node browser containers, its config.toml could be overwritten by node-docker container config file.

In this case, mount your config.toml file to another name e.g /opt/selenium/docker.toml in node-docker container. And set the environment variable SE_NODE_DOCKER_CONFIG_FILENAME=docker.toml to specify that config file name for the startup script.

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

Bug fix, Enhancement, Documentation

---

Description

• Made the configuration file path dynamic in the NodeDocker startup script based on the SE_NODE_DOCKER_CONFIG_FILENAME environment variable.
• Added new Appium capabilities suppressKillServer and allowDelayAdb in Selenium tests setup.
• Updated Selenium version from 4.23.0 to 4.23.1 and added a gen_certs target in the Makefile.
• Adjusted Dockerfile to copy config.toml to /opt/selenium.
• Added documentation for sharing volumes config of Dynamic Grid container to node browser containers.
• Updated docker-compose file to reflect the dynamic configuration file path.

---

Changes walkthrough 📝

	Relevant files
Enhancement	start-selenium-grid-docker.sh Make configuration file path dynamic in startup script      NodeDocker/start-selenium-grid-docker.sh Modified the configuration file path to be dynamic based on the SE_NODE_DOCKER_CONFIG_FILENAME environment variable. +1/-1      __init__.py Add new Appium capabilities in Selenium tests setup            tests/SeleniumTests/init.py Added new Appium capabilities suppressKillServer and allowDelayAdb. +2/-0      Makefile Update Selenium version and add certificate generation target Makefile Updated Selenium version from 4.23.0 to 4.23.1. Added gen_certs target and included it in the base target. +6/-4      Dockerfile Adjust Dockerfile to copy config.toml to /opt/selenium      NodeDocker/Dockerfile Moved config.toml copy command to a new line and changed its destination directory. +2/-1      docker-compose-v3-test-node-docker.yaml Update docker-compose for dynamic config file path              tests/docker-compose-v3-test-node-docker.yaml Updated volume mount path for config.toml and added SE_NODE_DOCKER_CONFIG_FILENAME environment variable. +2/-1
Documentation	README.md Document sharing volumes config in Dynamic Grid                    README.md Added documentation for sharing volumes config of Dynamic Grid container to node browser containers. +31/-0

---

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

[qodo-merge-pro]

PR Reviewer Guide 🔍

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Key issues to review Configuration Flexibility The script now uses an environment variable SE_NODE_DOCKER_CONFIG_FILENAME to determine the configuration file path. Ensure that this variable is always set properly in environments where the script runs to avoid defaulting to config.toml, which might not exist. New Capabilities New capabilities suppressKillServer and allowDelayAdb have been added. Verify that these capabilities are supported in all target environments and do not introduce any side effects or undesired behavior.

[qodo-merge-pro]

PR Code Suggestions ✨

Category	Suggestion	Score
Possible issue	Add a check to ensure the SE_NODE_DOCKER_CONFIG_FILENAME environment variable is set Consider adding a check to ensure that the environment variable SE_NODE_DOCKER_CONFIG_FILENAME is not empty before using it in the --config option. This can prevent potential issues where the configuration file might not be specified, leading to unexpected behaviors. NodeDocker/start-selenium-grid-docker.sh [109] ---config /opt/selenium/${SE_NODE_DOCKER_CONFIG_FILENAME:-"config.toml"} +if [ -z "${SE_NODE_DOCKER_CONFIG_FILENAME}" ]; then + echo "Error: SE_NODE_DOCKER_CONFIG_FILENAME is not set." + exit 1 +fi +--config /opt/selenium/${SE_NODE_DOCKER_CONFIG_FILENAME} Apply this suggestion Suggestion importance[1-10]: 9 Why: This suggestion adds a crucial check to ensure that the SE_NODE_DOCKER_CONFIG_FILENAME environment variable is set, which can prevent potential issues where the configuration file might not be specified, leading to unexpected behaviors.	9
	Add a check to ensure version variables are synchronized To avoid potential issues with version mismatches or unexpected behavior in builds, consider adding a check to ensure that BASE_VERSION, BINDING_VERSION, and VERSION are synchronized, especially when they are expected to match. Makefile [5-9] BASE_VERSION := $(or $(BASE_VERSION),$(BASE_VERSION),4.23.1) BINDING_VERSION := $(or $(BINDING_VERSION),$(BINDING_VERSION),4.23.1) VERSION := $(or $(VERSION),$(VERSION),4.23.1) +if [ "$(BASE_VERSION)" != "$(BINDING_VERSION)" ] || [ "$(BASE_VERSION)" != "$(VERSION)" ]; then + echo "Error: Version mismatch detected." + exit 1 +fi Apply this suggestion Suggestion importance[1-10]: 8 Why: Ensuring that version variables are synchronized can prevent potential issues with version mismatches or unexpected behavior in builds, which is important for maintaining consistency.	8
Enhancement	Adjust volume mount paths to prevent conflicts Ensure that the volume mount paths are correctly set to avoid potential conflicts or overwrites, especially when dealing with configuration files that might be shared across different containers. tests/docker-compose-v3-test-node-docker.yaml [7] -- ./videos/config.toml:/opt/selenium/docker.toml +- ./config/docker.toml:/opt/selenium/docker.toml # Adjusted path to separate config files from video files Apply this suggestion Suggestion importance[1-10]: 7 Why: Adjusting the volume mount paths to separate config files from video files can help avoid potential conflicts or overwrites, improving the reliability of the configuration.	7
Best practice	Validate and comment on the intentional settings of boolean capabilities It's recommended to validate the boolean values for capabilities like suppressKillServer and allowDelayAdb to ensure they are intentionally set as expected, especially when their default values might cause unintended behavior in test environments. tests/SeleniumTests/init.py [145-146] -options.set_capability('appium:suppressKillServer', True) -options.set_capability('appium:allowDelayAdb', False) +options.set_capability('appium:suppressKillServer', True) # Ensure this is the intended setting +options.set_capability('appium:allowDelayAdb', False) # Verify this should not be True Apply this suggestion Suggestion importance[1-10]: 5 Why: Adding comments to validate boolean values is a good practice for clarity, but it does not significantly improve the functionality or prevent major issues.	5