-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[feat][test]: add env vars to generate config for Relay in node-base #2254
Conversation
PR Description updated to latest commit (95e3b54)
|
PR Review 🔍
|
PR Code Suggestions ✨
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for doing this, @VietND96.
Offering a way to configure Relay is a good idea, but the PR adds too much complexity to the project. Let me explain:
- We already have too many environment variables and need to document them all at some point. We recommend to use
config.toml
files to configure the Grid, so I prefer to offer a way to the user to mount it and use it for Relay. It is easier to read a file instead of several environment variables. - I know Relay is a common way to integrate Appium. However, the tests in this PR are hard to maintain, and the Android image is know to work slowly when nested virtualization is not present. If we want to test this, I prefer to start a Standalone container, and have it as the receiver of the forwarded commands through Relay.
@diemol, I see |
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. |
1300ae2
to
ceb38b3
Compare
Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
@diemol, few changes have been done
|
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
orselenium/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:
Types of changes
Checklist
PR Type
enhancement, tests
Description
Changes walkthrough 📝
4 files
__init__.py
Add Environment Variables and Conditional Delay for Node Relay
tests/SeleniumTests/init.py
TEST_NODE_RELAY
is not 'false'.Makefile
Introduce Make Targets for Node Relay Testing
Makefile
generate_config
Enhance Node Configuration Script with Relay Options
NodeBase/generate_config
Dockerfile.android
Create Dockerfile for Android Testing Environment
tests/Dockerfile.android
environment.
5 files
docker-test.yml
Update GitHub Actions for Docker Tests with Node Relay
.github/workflows/docker-test.yml
steps.
helm-chart-test.yml
Remove Scheduled Cron Jobs from Helm Chart Tests
.github/workflows/helm-chart-test.yml
nightly.yaml
Update Nightly Workflow to Conditionally Include Tests
.github/workflows/nightly.yaml
event triggers.
docker-compose-v3-relay.yml
Add Docker Compose Configuration for Node Relay
docker-compose-v3-relay.yml
node relay.
docker-compose-v3-test-node-relay.yml
Setup Docker Compose for Node Relay Testing
tests/docker-compose-v3-test-node-relay.yml
1 files
README.md
Document Node Configuration Relay Commands
README.md