Update java-examples.yml

harsha509 commented 1 week ago

User description

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

Enhanced the logic for retrieving the latest Selenium snapshot in the Java examples workflow by selecting the snapshot based on the last modified date.
Improved the output message to clearly indicate the latest Selenium snapshot version being used.

Changes walkthrough 📝

Relevant files

Enhancement

java-examples.yml

Enhance snapshot retrieval logic in Java examples workflow

.github/workflows/java-examples.yml

Updated the method to fetch the latest Selenium snapshot.

Added logic to select the snapshot based on the last modified date.

Improved output message for clarity.

+4/-4

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

netlify[bot] commented 1 week ago

Deploy request for selenium-dev pending review.

Visit the deploys page to approve it

Name	Link
Latest commit	6184804c4e1d439d45a9e670258e8e8b32383b8a

codiumai-pr-agent-pro[bot] commented 1 week ago

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review

Potential Error Handling
The new code doesn't include error handling for the case when the XML content retrieval or parsing fails. This could lead to undefined behavior if the curl command or xq parsing fails. Windows Compatibility
The Windows PowerShell script uses similar logic to the Unix script, but it's not clear if the `max_by(.lastModified)` function will work the same way in PowerShell. This might lead to inconsistent behavior between Unix and Windows environments.

codiumai-pr-agent-pro[bot] commented 1 week ago

PR Code Suggestions ✨

Category	Suggestion	Score
Error handling	Add error handling for network requests and data extraction ___ Consider adding error handling for the `curl` command to ensure the script doesn't fail silently if the URL is unreachable or returns an unexpected response. [.github/workflows/java-examples.yml [69-70]](https://github.com/SeleniumHQ/seleniumhq.github.io/pull/1919/files#diff-091d3d4615e90bd4f823cfa98958771393c175fdc81fe1b250698bf7a9e4bb45R69-R70) ```diff -xml_content=$(curl -sf https://oss.sonatype.org/service/local/repositories/snapshots/content/org/seleniumhq/selenium/selenium-java/) +if ! xml_content=$(curl -sf https://oss.sonatype.org/service/local/repositories/snapshots/content/org/seleniumhq/selenium/selenium-java/); then + echo "Failed to fetch XML content" + exit 1 +fi latest_snapshot=$(echo $xml_content \| xq -r '.content.data."content-item"' \| max_by(.lastModified) \| .text) +if [ -z "$latest_snapshot" ]; then + echo "Failed to extract latest snapshot version" + exit 1 +fi ``` - [ ] Apply this suggestion Suggestion importance[1-10]: 9 Why: The suggestion to add error handling for the `curl` command and data extraction is crucial for ensuring the script does not fail silently, which improves robustness and reliability.	9
Error handling	Validate the retrieved version before using it in subsequent commands ___ Consider adding a check to ensure that `$latest_snapshot` is not empty before using it in the Maven command, to prevent potential issues if the snapshot version couldn't be retrieved. [.github/workflows/java-examples.yml [84-87]](https://github.com/SeleniumHQ/seleniumhq.github.io/pull/1919/files#diff-091d3d4615e90bd4f823cfa98958771393c175fdc81fe1b250698bf7a9e4bb45R84-R87) ```diff $latest_snapshot = $xml_content.Content \| xq -r '.content.data.\"content-item\"' \| max_by(.lastModified) \| .text Write-Output "Latest Selenium Snapshot: $latest_snapshot" +if ([string]::IsNullOrEmpty($latest_snapshot)) { + Write-Error "Failed to retrieve latest Selenium snapshot version" + exit 1 +} cd examples/java mvn -B -U test "-Dselenium.version=$latest_snapshot" ``` - [ ] Apply this suggestion Suggestion importance[1-10]: 9 Why: Adding a check to ensure `$latest_snapshot` is not empty before using it in the Maven command is important for preventing errors and ensuring the script behaves as expected.	9
Performance	Use built-in PowerShell JSON parsing capabilities instead of external tools ___ In the Windows PowerShell section, consider using `ConvertFrom-Json` instead of `xq` for parsing JSON, as it's a built-in PowerShell cmdlet and doesn't require additional installation. [.github/workflows/java-examples.yml [83-84]](https://github.com/SeleniumHQ/seleniumhq.github.io/pull/1919/files#diff-091d3d4615e90bd4f823cfa98958771393c175fdc81fe1b250698bf7a9e4bb45R83-R84) ```diff $xml_content = Invoke-WebRequest -Uri "https://oss.sonatype.org/service/local/repositories/snapshots/content/org/seleniumhq/selenium/selenium-java/" -$latest_snapshot = $xml_content.Content \| xq -r '.content.data.\"content-item\"' \| max_by(.lastModified) \| .text +$json_content = $xml_content.Content \| ConvertFrom-Json +$latest_snapshot = ($json_content.content.data.'content-item' \| Sort-Object -Property lastModified -Descending \| Select-Object -First 1).text ``` - [ ] Apply this suggestion Suggestion importance[1-10]: 8 Why: Using `ConvertFrom-Json` is a good suggestion as it leverages built-in PowerShell capabilities, reducing dependencies and potentially improving performance.	8

SeleniumHQ / seleniumhq.github.io