Update ruby-examples.yml

harsha509 commented 3 weeks ago

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

netlify[bot] commented 3 weeks ago

Deploy request for selenium-dev pending review.

Visit the deploys page to approve it

Name	Link
Latest commit	d67b12bd47337837cd839d0da780e3f9462d7b25

codiumai-pr-agent-pro[bot] commented 3 weeks ago

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.

codiumai-pr-agent-pro[bot] commented 3 weeks ago

PR Code Suggestions ✨

Category	Suggestion	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]](https://github.com/SeleniumHQ/seleniumhq.github.io/pull/1777/files#diff-9c5c607062298096d440cd6b49fc726cc23abc0bbdc87e25e013d312c2ec824bR34-R37) ```diff +- 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
Possible issue	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]](https://github.com/SeleniumHQ/seleniumhq.github.io/pull/1777/files#diff-9c5c607062298096d440cd6b49fc726cc23abc0bbdc87e25e013d312c2ec824bR91-R94) ```diff -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]](https://github.com/SeleniumHQ/seleniumhq.github.io/pull/1777/files#diff-9c5c607062298096d440cd6b49fc726cc23abc0bbdc87e25e013d312c2ec824bR34-R37) ```diff -- 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]](https://github.com/SeleniumHQ/seleniumhq.github.io/pull/1777/files#diff-9c5c607062298096d440cd6b49fc726cc23abc0bbdc87e25e013d312c2ec824bR87-R94) ```diff - 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

SeleniumHQ / seleniumhq.github.io