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

[build][doc]: upgrade browser with remain on same the Selenium version #2217

Merged
merged 1 commit into from
Apr 19, 2024

Conversation

VietND96
Copy link
Member

@VietND96 VietND96 commented Apr 19, 2024

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

Motivation and Context

Guideline for the question How do I upgrade the chrome version of a “selenium/node-chrome”, currently I am using the 4.16.1 verison, i would like to remain on the same selenium version but just upgrade the chromedriver and browser to the latest?

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.

Type

enhancement, documentation


Description

  • Added new Makefile targets to facilitate upgrading browser versions (Chrome, Firefox, Edge) in Docker images without changing the Selenium version.
  • Updated the README to include detailed instructions on how to use these new Makefile targets to upgrade browser and driver versions.
  • Each browser upgrade target in the Makefile builds the respective Docker image and verifies the versions of the Selenium server, browser, and driver.

Changes walkthrough

Relevant files
Enhancement
Makefile
Add Docker image upgrade targets for Chrome, Firefox, and Edge

Makefile

  • Added targets for upgrading Chrome, Firefox, and Edge versions in
    Docker images.
  • Each target builds the Docker image and checks the versions of
    Selenium server, browser, and driver.
  • +21/-0   
    Documentation
    README.md
    Update documentation for upgrading browser versions in Docker images

    README.md

  • Updated documentation to include instructions on how to upgrade
    browser versions while keeping the same Selenium version.
  • Provided detailed commands for upgrading Chrome, Firefox, and Edge in
    Docker images.
  • +25/-0   

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

    Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
    Copy link

    PR Description updated to latest commit (497434e)

    Copy link

    PR Review

    ⏱️ Estimated effort to review [1-5]

    2, because the PR involves updates to the Makefile and README.md to facilitate upgrading browser versions while keeping the same Selenium version. The changes are straightforward and well-documented, making the review process relatively simple.

    🧪 Relevant tests

    No

    🔍 Possible issues

    Possible Issue: The PR does not mention updating any tests to cover the new upgrade paths. It's important to ensure that these upgrades do not introduce regressions or compatibility issues.

    🔒 Security concerns

    No


    ✨ Review tool usage guide:

    Overview:
    The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

    The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

    • When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:
    /review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...
    
    [pr_reviewer]
    some_config1=...
    some_config2=...
    

    See the review usage page for a comprehensive guide on using this tool.

    Copy link

    PR Code Suggestions

    CategorySuggestions                                                                                                                                                       
    Best practice
    Add a check to ensure Docker builds are successful before running containers.

    Consider adding a check for successful image builds before attempting to run the
    containers. This ensures that the subsequent commands only execute if the build was
    successful.

    Makefile [161]

    -cd ./NodeChrome && docker build $(BUILD_ARGS) --build-arg NAMESPACE=$(NAMESPACE) --build-arg VERSION=$(VERSION) --build-arg AUTHORS=$(AUTHORS) -t $(NAME)/node-chrome:$(TAG_VERSION) .
    +cd ./NodeChrome && docker build $(BUILD_ARGS) --build-arg NAMESPACE=$(NAMESPACE) --build-arg VERSION=$(VERSION) --build-arg AUTHORS=$(AUTHORS) -t $(NAME)/node-chrome:$(TAG_VERSION) . && \
    +docker run --rm $(NAME)/standalone-chrome:$(TAG_VERSION) /opt/selenium/selenium-server.jar info --version
     
    Combine Docker run commands to ensure each executes only after the previous succeeds.

    Combine the docker run commands into a single line using && to ensure that each command
    only runs if the previous one was successful.

    Makefile [163-165]

    -docker run --rm $(NAME)/standalone-chrome:$(TAG_VERSION) /opt/selenium/selenium-server.jar info --version
    -docker run --rm $(NAME)/standalone-chrome:$(TAG_VERSION) google-chrome --version
    +docker run --rm $(NAME)/standalone-chrome:$(TAG_VERSION) /opt/selenium/selenium-server.jar info --version && \
    +docker run --rm $(NAME)/standalone-chrome:$(TAG_VERSION) google-chrome --version && \
     docker run --rm $(NAME)/standalone-chrome:$(TAG_VERSION) chromedriver --version
     
    Add error handling to Docker commands to manage build failures.

    Add error handling for Docker commands to manage failures gracefully.

    Makefile [161]

    -cd ./NodeChrome && docker build $(BUILD_ARGS) --build-arg NAMESPACE=$(NAMESPACE) --build-arg VERSION=$(VERSION) --build-arg AUTHORS=$(AUTHORS) -t $(NAME)/node-chrome:$(TAG_VERSION) .
    +cd ./NodeChrome && docker build $(BUILD_ARGS) --build-arg NAMESPACE=$(NAMESPACE) --build-arg VERSION=$(VERSION) --build-arg AUTHORS=$(AUTHORS) -t $(NAME)/node-chrome:$(TAG_VERSION) || { echo "Docker build failed"; exit 1; }
     
    Maintainability
    Use a variable for Docker image tags to reduce redundancy.

    Use a variable for the repeated Docker image tag to reduce redundancy and potential errors
    in future modifications.

    Makefile [162]

    -cd ./Standalone && docker build $(BUILD_ARGS) $(FROM_IMAGE_ARGS) --build-arg BASE=node-chrome -t $(NAME)/standalone-chrome:$(TAG_VERSION) .
    +CHROME_IMAGE_TAG=$(NAME)/standalone-chrome:$(TAG_VERSION)
    +cd ./Standalone && docker build $(BUILD_ARGS) $(FROM_IMAGE_ARGS) --build-arg BASE=node-chrome -t $CHROME_IMAGE_TAG .
     
    Extract common Docker build arguments into a variable for simplicity and maintainability.

    Extract common Docker build arguments into a variable to simplify the Docker build
    commands and improve maintainability.

    Makefile [161]

    -cd ./NodeChrome && docker build $(BUILD_ARGS) --build-arg NAMESPACE=$(NAMESPACE) --build-arg VERSION=$(VERSION) --build-arg AUTHORS=$(AUTHORS) -t $(NAME)/node-chrome:$(TAG_VERSION) .
    +COMMON_BUILD_ARGS="--build-arg NAMESPACE=$(NAMESPACE) --build-arg VERSION=$(VERSION) --build-arg AUTHORS=$(AUTHORS)"
    +cd ./NodeChrome && docker build $(BUILD_ARGS) $COMMON_BUILD_ARGS -t $(NAME)/node-chrome:$(TAG_VERSION) .
     

    ✨ Improve tool usage guide:

    Overview:
    The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

    • When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:
    /improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...
    
    [pr_code_suggestions]
    some_config1=...
    some_config2=...
    

    See the improve usage page for a comprehensive guide on using this tool.

    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.

    🎉

    @VietND96 VietND96 merged commit 4c572af into SeleniumHQ:trunk Apr 19, 2024
    11 checks passed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants