Skip to content
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

Feature run selenium tests grid on kubernetes via helm chart #2027

Merged

Conversation

amardeep2006
Copy link
Contributor

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

This is regarding #1975
I have enhanced the GitHub workflow to run existing selenium test cases against the selenium grid deployed in kind kubernetes cluster.

Motivation and Context

It will increase helm chart testability. It makes use of existing test cases. This is how it works :

  1. Spins up kind kubernetes cluster.
  2. Installs nginx-ingress on kubernetes cluster
  3. Deploys the helm chart to k8s
  4. Triggers selenium based test cases
  5. Did small refactoring in existing test cases as python code had hardcoded selenium grid port. Now it picks from OS (defaults to 4444)

I have intentionally deployed and tested the Chrome, Edge and Firefox pods one by one because of GitHub runner limitations. The KIND cluster was facing insufficient CPU if we deploy all pods in one go.

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.

Makefile Outdated Show resolved Hide resolved
Copy link
Member

@diemol diemol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this! Why do we need to duplicate the tests file?

@amardeep2006
Copy link
Contributor Author

Thanks for this! Why do we need to duplicate the tests file?

I may have overlooked DRY principle. Let me refactor the test.py to make it kubernetes aware and not spinning up docker containers. I will push the new commit when ready.

@amardeep2006
Copy link
Contributor Author

@diemol I have made changes suggested to reuse existing test.py

Copy link
Member

@diemol diemol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, @amardeep2006!

@diemol diemol merged commit e56a1fc into SeleniumHQ:trunk Nov 27, 2023
8 checks passed
@VietND96 VietND96 linked an issue Nov 30, 2023 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Helm Charts maintainer needed
3 participants