[feat][test]: add env vars to generate config for Relay in node-base

[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

[feat][test]: add env vars to generate config for Relay in node-base

Motivation and Context

---

Node configuration relay commands

Relaying commands to a service endpoint that supports WebDriver. It is useful to connect an external service that supports WebDriver to Selenium Grid. An example of such service could be a cloud provider or an Appium server. In this way, Grid can enable more coverage to platforms and versions not present locally.

The following is an en example of connecting an Appium server to Grid.

docker-compose-v3-relay.yml

If you want to relay commands only, selenium/node-base is suitable and lightweight for this purpose. In case you want to configure node with both browsers and relay commands, selenium/node-chrome or selenium/node-docker (with TOML configuration options) can be used.

To run a sample test with the relayed node, you can clone the project and try below command:

make test_node_relay

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

---

PR Type

enhancement, tests

---

Description

• Introduced environment variables and conditional logic for node relay testing in Python tests.
• Updated GitHub Actions workflows by adding node relay tests and removing scheduled cron jobs.
• Added new Makefile targets and Docker Compose configurations to support node relay testing.
• Enhanced node configuration scripts to include relay options and updated documentation accordingly.
• Created a new Dockerfile for an Android testing environment.

---

Changes walkthrough 📝

	Relevant files
Enhancement	4 files __init__.py Add Environment Variables and Conditional Delay for Node Relay tests/SeleniumTests/init.py Added environment variables for node relay and Android platform API. Introduced a conditional delay when TEST_NODE_RELAY is not 'false'. +13/-0    Makefile Introduce Make Targets for Node Relay Testing                        Makefile Added new make targets for testing node relay configurations. +25/-0    generate_config Enhance Node Configuration Script with Relay Options          NodeBase/generate_config Added relay configuration blocks to the node configuration script. Adjusted node stereotype and driver configuration settings. +49/-12  Dockerfile.android Create Dockerfile for Android Testing Environment                tests/Dockerfile.android Added a new Dockerfile for building an Android-based testing environment. +13/-0
Configuration changes	5 files docker-test.yml Update GitHub Actions for Docker Tests with Node Relay      .github/workflows/docker-test.yml Removed scheduled cron jobs. Added a new test strategy for node relay with corresponding setup steps. +8/-2      helm-chart-test.yml Remove Scheduled Cron Jobs from Helm Chart Tests                  .github/workflows/helm-chart-test.yml Removed scheduled cron jobs. +0/-2      nightly.yaml Update Nightly Workflow to Conditionally Include Tests      .github/workflows/nightly.yaml Added conditional execution for docker and helm chart tests based on event triggers. +9/-0      docker-compose-v3-relay.yml Add Docker Compose Configuration for Node Relay                    docker-compose-v3-relay.yml Created a new Docker Compose file for setting up environments with node relay. +66/-0    docker-compose-v3-test-node-relay.yml Setup Docker Compose for Node Relay Testing                            tests/docker-compose-v3-test-node-relay.yml Introduced a Docker Compose configuration for node relay testing. +94/-0
Documentation	1 files README.md Document Node Configuration Relay Commands                              README.md Added documentation for node configuration relay commands. +19/-0

---

💡 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/docker-selenium/commit/95e3b54b7e15e929c1cf35ce1ec0a749c84e0164)

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

[qodo-code-review[bot]]

PR Review 🔍

⏱️ Estimated effort to review [1-5]	4, because the PR involves multiple complex changes across various files including Dockerfiles, Makefile, GitHub Actions workflows, and Python test scripts. The introduction of new environment variables and conditional logic increases the complexity, requiring careful review to ensure compatibility and correctness.
🧪 Relevant tests	Yes
⚡ Possible issues	Hardcoded Values: The use of hardcoded values (e.g., URLs, platform names) in the Dockerfiles and test scripts might limit flexibility and reusability.
	Conditional Logic in Tests: The introduction of conditional logic based on environment variables in test scripts could lead to scenarios where certain code paths are not tested under different configurations.
	Delay in Tests: The hardcoded sleep of 120 seconds in the Python test script could introduce unnecessary delays in test execution, potentially impacting the overall test suite performance.
🔒 Security concerns	No

[qodo-code-review[bot]]

PR Code Suggestions ✨

Category	Suggestions
Enhancement	Convert string comparison to boolean for consistency and reliability. Convert the string comparison for TEST_NODE_RELAY to a boolean check to ensure consistency with other environment variable checks in the code. tests/SeleniumTests/init.py [30] -if TEST_NODE_RELAY != 'false': +if TEST_NODE_RELAY.lower() == 'true':
	Make the sleep delay configurable via an environment variable for better flexibility. Replace the hard-coded sleep time with a configurable environment variable to allow more flexible test setups. tests/SeleniumTests/init.py [31] -time.sleep(120) +time.sleep(int(os.environ.get('RELAY_ACTIVATION_DELAY', 120)))
Bug	Improve the reliability of platform string comparison by making it case-insensitive. Use a more specific condition to check for the Android platform to avoid potential issues with case sensitivity or unexpected platform strings. tests/SeleniumTests/init.py [134] -if TEST_NODE_RELAY == 'Android': +if TEST_NODE_RELAY.lower() == 'android':
Possible issue	Add error handling for a potentially unset environment variable to prevent runtime errors. Add error handling for the environment variable TEST_ANDROID_PLATFORM_API to ensure it is not None before setting capabilities, which could cause runtime errors. tests/SeleniumTests/init.py [136] -options.set_capability('appium:platformVersion', TEST_ANDROID_PLATFORM_API) +if TEST_ANDROID_PLATFORM_API: + options.set_capability('appium:platformVersion', TEST_ANDROID_PLATFORM_API) +else: + raise ValueError("TEST_ANDROID_PLATFORM_API environment variable is not set")
	Add validation for setting the 'platformName' capability to prevent setting invalid values. Ensure that the options.set_capability for 'platformName' is set to a valid value when TEST_NODE_RELAY is not 'Android'. tests/SeleniumTests/init.py [141] -options.set_capability('platformName', 'Linux') +if TEST_NODE_RELAY.lower() == 'linux': + options.set_capability('platformName', 'Linux') +else: + raise ValueError("Unsupported TEST_NODE_RELAY value")

[VietND96]

@diemol, I see --config can pass multiple toml files. Can we separate another generate_relay_config and isolate related env vars there? Of course, still many env vars, but it is readable. Because there were few env vars with default values get/set. A toml config file can be defined and mounted to the container, however it needs to rewrite all without reusable existing. Separate out another toml file for relay I think it would help in case we want to mount specific relay configs within reuse existing default node configs.

[diemol]

It is a good idea to have it as a different file. But I still dislike the environment variables approach; for example, setting the stereotypes via environment variables makes it very hard to read and maintain.

[VietND96]

@diemol, few changes have been done

• I don't add the env var for setting complex stereotypes to balance both approaches. Env vars are used for basic config to be generated. For complex stereotypes, let the user disable generate config mode and input a TOML config file.
• Add a test with start a Standalone container, and have it as the receiver of the forwarded commands through Relay.
• I would like to keep a test for the node relay to Appium (for reason like a reference test to verify & confirm something works quickly) . If the test is not stable or hard to maintain later, simplify to remove Android from the for loop in Makefile :)