Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Update ruby-examples.yml #1777

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

Merged
harsha509 merged 1 commit into SeleniumHQ:trunk from harsha509:trunk
Jun 23, 2024
Merged

Update ruby-examples.yml #1777

harsha509 merged 1 commit into SeleniumHQ:trunk from harsha509:trunk
Jun 23, 2024

Conversation

@harsha509
Copy link
Member

@harsha509 harsha509 commented Jun 23, 2024 •
edited by qodo-merge-pro bot
Loading

User description

Add step to install SafarTechPreview

Thanks for contributing to the Selenium site and documentation!
A PR well described will help maintainers to review and merge it quickly

Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, and help reviewers by making them as simple and short as possible.

Description

Motivation and Context

Types of changes

  • Change to the site (I have double-checked the Netlify deployment, and my changes look good)
  • Code example added (and I also added the example to all translated languages)
  • Improved translation
  • Added new translation (and I also added a notice to each document missing translation)

Checklist

  • I have read the contributing document.
  • I have used hugo to render the site/docs locally and I am sure it works.

PR Type

enhancement, configuration changes

Description

  • Added a step to install Safari Technology Preview on macOS in the GitHub Actions workflow.
  • Included a check to ensure safaridriver exists and exit if it is not found.

Changes walkthrough 📝

Relevant files
Enhancement
ruby-examples.yml
Add Safari Technology Preview installation and configuration steps

.github/workflows/ruby-examples.yml

  • Added step to install Safari Technology Preview on macOS.
  • Added check for safaridriver existence and exit if not found.
    • +12/-0

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

     

    Add step to install SafarTechPreview
    Copy link

    netlify bot commented Jun 23, 2024

    👷 Deploy request for selenium-dev pending review.

    Visit the deploys page to approve it

    Name Link
    🔨 Latest commit d67b12b

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review [1-5] 2
    🧪 Relevant tests No
    🔒 Security concerns No
    ⚡ Key issues to review Possible Bug:
    The PR adds a check for safaridriver in the macOS-latest environment, but it does not handle the scenario where the installation of Safari Technology Preview might fail. Consider adding error handling for the installation step.

    Copy link
    Contributor

    PR Code Suggestions ✨

    CategorySuggestion Score
    Possible issue
    Add a step to install Homebrew before installing Safari Technology Preview to ensure the brew command is available

    To ensure the brew command is available, add a step to install Homebrew before attempting
    to install Safari Technology Preview. This prevents potential failures on systems where
    Homebrew is not pre-installed.

    .github/workflows/ruby-examples.yml [34-37] 

    +- name: Install Homebrew
+ if: matrix.os == 'macos-latest'
+ run: |
+ /bin/bash -c "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)"
 - name: Install Safari Technology Preview
 if: matrix.os == 'macos-latest'
 run: |
 brew install --cask safari-technology-preview
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: This is a crucial suggestion as it ensures that the necessary package manager is available before attempting to install Safari Technology Preview, preventing potential failures.

    		8
    Use a more specific condition to check if safaridriver is executable

    Use a more specific condition to check for the existence of safaridriver by verifying the
    exact path to the executable.

    .github/workflows/ruby-examples.yml [91-94] 

    -if [[ ! -f "/Applications/Safari Technology Preview.app/Contents/MacOS/safaridriver" ]]; then
- echo "safaridriver not found. Exiting."
+if [[ ! -x "/Applications/Safari Technology Preview.app/Contents/MacOS/safaridriver" ]]; then
+ echo "safaridriver not found or not executable. Exiting."
 exit 1
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    Why: This suggestion enhances the reliability of the check by ensuring that safaridriver is not only present but also executable, which is important for the intended operation.

    		6
    Enhancement
    Combine the installation and configuration of Safari Technology Preview into a single step

    Combine the installation and configuration of Safari Technology Preview into a single step
    to streamline the workflow and reduce redundancy.

    .github/workflows/ruby-examples.yml [34-37] 

    -- name: Install Safari Technology Preview
+- name: Install and Configure Safari Technology Preview
 if: matrix.os == 'macos-latest'
 run: |
 brew install --cask safari-technology-preview
-- name: Install and Configure Safari and WebDriver
- if: matrix.os == 'macos-latest'
- run: |
 # Check if safaridriver exists
 if [[ ! -f "/Applications/Safari Technology Preview.app/Contents/MacOS/safaridriver" ]]; then
 echo "safaridriver not found. Exiting."
 exit 1
 fi
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: This suggestion improves the workflow by reducing redundancy and streamlining the process, which is beneficial for maintainability and clarity.

    		7
    Best practice
    Add continue-on-error: true to the step checking for safaridriver to prevent workflow failure

    Add a continue-on-error: true property to the step that checks for safaridriver to prevent
    the entire workflow from failing if Safari Technology Preview is not installed.

    .github/workflows/ruby-examples.yml [87-94] 

     - name: Install and Configure Safari and WebDriver
 if: matrix.os == 'macos-latest'
+ continue-on-error: true
 run: |
 # Check if safaridriver exists
 if [[ ! -f "/Applications/Safari Technology Preview.app/Contents/MacOS/safaridriver" ]]; then
 echo "safaridriver not found. Exiting."
 exit 1
 fi
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    Why: Adding continue-on-error: true is a good practice to prevent the entire workflow from failing due to a non-critical step, enhancing robustness.

    		6

    @harsha509 harsha509 merged commit 34cdad7 into SeleniumHQ:trunk Jun 23, 2024
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    No reviews

    Assignees

    No one assigned

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    1 participant

    AltStyle によって変換されたページ (->オリジナル) /