chore: Fix chrome version to 129#38183
Conversation
WalkthroughThe pull request focuses on updating multiple GitHub Actions workflow files for Appsmith's CI testing infrastructure. The changes primarily involve refining workflow configurations, adding new input parameters, updating Chrome installation methods, and enhancing test execution flexibility. Key modifications include introducing Changes
Sequence DiagramsequenceDiagram
participant Workflow as GitHub Workflow
participant Chrome as Chrome Setup
participant Tests as Test Runner
participant Cache as Artifact Cache
Workflow->>Chrome: Install Chrome v129.0.6668.100
Chrome-->>Workflow: Chrome installed
Workflow->>Cache: Restore previous artifacts
Cache-->>Workflow: Artifacts retrieved
Workflow->>Tests: Execute tests
Tests-->>Workflow: Test results
Workflow->>Cache: Store test results
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
bot
added
the
skip-changelog
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.github/workflows/ci-test-limited-with-count.yml (1)
Line range hint
13-15: Good addition of run_count parameterThe implementation of run_count parameter with cypress-repeat-pro enhances test reliability by allowing multiple test runs. This is particularly useful for identifying flaky tests.
Consider adding documentation about recommended run_count values for different test scenarios.
🧰 Tools
🪛 actionlint (1.7.4)
236-236: shellcheck reported issue in this script: SC2086:info:1:40: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/ci-test-custom-script.yml (1)
Line range hint
7-11: Consider parameterizing Chrome versionSince you've added the
ted_tagparameter for flexibility, consider making the Chrome version configurable as well. This would allow easier version updates and testing across different Chrome versions.workflow_call: inputs: + chrome_version: + description: "Chrome version to use for testing" + required: false + type: string + default: "129" pr: description: "This is the PR number in case the workflow is being called in a pull request" required: false type: number # Later in the file - uses: browser-actions/setup-chrome@latest with: - chrome-version: 129 + chrome-version: ${{ inputs.chrome_version }}Also applies to: 259-259
🧰 Tools
🪛 actionlint (1.7.4)
260-260: shellcheck reported issue in this script: SC2086:info:1:40: Double quote to prevent globbing and word splitting
(shellcheck)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/ci-test-custom-script.yml(1 hunks).github/workflows/ci-test-hosted.yml(1 hunks).github/workflows/ci-test-limited-with-count.yml(1 hunks).github/workflows/ci-test-limited.yml(1 hunks)
🔇 Additional comments (3)
.github/workflows/ci-test-limited.yml (1)
226-226: Chrome version update
The Chrome version update is consistent with other workflow files.
.github/workflows/ci-test-limited-with-count.yml (1)
235-235: Chrome version update
The Chrome version update is consistent with other workflow files.
.github/workflows/ci-test-custom-script.yml (1)
Line range hint 19-23: LGTM! Well-structured input parameter for TestEventDriver
The addition of the ted_tag parameter with a default value of 'latest' provides good flexibility for testing with different versions while maintaining backward compatibility.
🧰 Tools
🪛 actionlint (1.7.4)
260-260: shellcheck reported issue in this script: SC2086:info:1:40: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/ci-test-hosted.yml
Outdated
| - uses: browser-actions/setup-chrome@latest | ||
| with: | ||
| chrome-version: stable | ||
| chrome-version: 129 |
There was a problem hiding this comment.
💡 Codebase verification
Chrome version 129 is too far ahead of current stable releases
The project uses Cypress 13.13.0 for testing, and Chrome 129 is not yet released (current stable is 120). Using unreleased browser versions in CI can lead to instability.
- Recommend using Chrome version 120 or lower to match current stable releases
- Update
.github/workflows/ci-test-hosted.ymlline 142
🔗 Analysis chain
Verify Chrome version compatibility with test suite
The change to Chrome version 129 should be validated against the test suite requirements and browser compatibility matrix.
Consider implementing browser version compatibility checks in the test suite to prevent issues with future version updates.
Let me check the specific browser version requirements in these files.
Let me check the Cypress configuration to understand browser testing requirements.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check Chrome version compatibility with test suite
# Look for any browser version specific code or configurations
rg -g '*.{ts,js,json}' -l 'chrome.*version|browser.*version' app/client/
Length of output: 261
Script:
#!/bin/bash
# Check the content of files for Chrome version references
rg -g '*.{ts,js,json}' 'chrome.*version|browser.*version' app/client/test/factories/MockPluginsState.ts
rg -g '*.{ts,js,json}' 'chrome.*version|browser.*version' app/client/cypress/fixtures/CommunityIssuesExport.json
# Check package.json for any browser dependencies
rg -g 'package.json' '"(cypress|playwright|puppeteer|selenium)"' .
# Check for any browser configuration in Cypress
fd -g 'cypress.config.*' .
Length of output: 66731
Script:
#!/bin/bash
# Check Cypress configuration and browser settings
fd -t f -e json -e js cypress.config
fd -t f package.json -x cat {} \; | grep -A 5 -B 5 '"cypress":'
Length of output: 520
| - uses: browser-actions/setup-chrome@latest | ||
| with: | ||
| chrome-version: stable | ||
| chrome-version: 129 |
There was a problem hiding this comment.
💡 Codebase verification
Chrome 129 is compatible but consider using version range
Chrome 129 is a stable version and compatible with Cypress 13.13.0, but fixing to a specific version might be restrictive. Consider using a version range (e.g., >= 129.0.0) to get security updates while maintaining stability.
- Update line 259 to use
chrome-version: '>=129.0.0'for better maintainability and security
🔗 Analysis chain
Verify Chrome version 129 compatibility
Fixing the Chrome version can help with test stability, but version 129 needs verification:
- Ensure it's compatible with your Cypress version
- Consider potential security implications of using a fixed version
Let me analyze the Cypress configuration to ensure compatibility with Chrome 129. The previous results confirm Chrome 129 is a stable version, but we need to verify Cypress compatibility.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if Chrome 129 is a stable version and get its release date
curl -s https://versionhistory.googleapis.com/v1/chrome/platforms/linux/channels/stable/versions | jq -r '.versions[] | select(.version | startswith("129"))'
# Check Cypress compatibility matrix
curl -s https://raw.githubusercontent.com/cypress-io/cypress-documentation/develop/content/guides/references/browser-launch-api.md | grep -A 5 "Chrome version support"
Length of output: 763
Script:
#!/bin/bash
# Let's check the Cypress configuration and version in the repository
fd -e json package.json --exec cat {} | jq -r 'select(.dependencies.cypress or .devDependencies.cypress) | .dependencies.cypress, .devDependencies.cypress'
# Check if there are any Cypress configuration files
fd -e json -e js cypress.config
Length of output: 206
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/ci-test-custom-script.yml(1 hunks).github/workflows/ci-test-hosted.yml(1 hunks).github/workflows/ci-test-limited-with-count.yml(1 hunks).github/workflows/ci-test-limited.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/ci-test-hosted.yml
- .github/workflows/ci-test-custom-script.yml
| - uses: browser-actions/setup-chrome@latest | ||
| with: | ||
| chrome-version: stable | ||
| chrome-version: 130 |
There was a problem hiding this comment.
Chrome version (130) doesn't match PR objective (129)
The PR objective states that Chrome version should be fixed to 129, but this line sets it to 130.
Apply this change to align with PR objective:
- chrome-version: 130
+ chrome-version: 129📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| chrome-version: 130 | |
| chrome-version: 129 |
| - uses: browser-actions/setup-chrome@latest | ||
| with: | ||
| chrome-version: stable | ||
| chrome-version: 130 |
There was a problem hiding this comment.
Chrome version (130) doesn't match PR objective (129)
The PR objective states that Chrome version should be fixed to 129, but this line sets it to 130.
Apply this change to align with PR objective:
- chrome-version: 130
+ chrome-version: 129📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| chrome-version: 130 | |
| chrome-version: 129 |
sagar-qa007
added
ok-to-test
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.github/workflows/ci-test-hosted.yml (1)
147-148: Quote the Chrome path to prevent word splittingThe Chrome path should be quoted to prevent potential issues with spaces or special characters.
- echo "BROWSER_PATH=${{ steps.setup-chrome.outputs.chrome-path }}" >> $GITHUB_ENV - ${{ steps.setup-chrome.outputs.chrome-path }} --version + echo "BROWSER_PATH=\"${{ steps.setup-chrome.outputs.chrome-path }}\"" >> $GITHUB_ENV + "${{ steps.setup-chrome.outputs.chrome-path }}" --version.github/workflows/ci-test-limited.yml (1)
223-223: Remove trailing whitespaceThere is a trailing whitespace on this line that should be removed.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 223-223: trailing spaces
(trailing-spaces)
.github/workflows/ci-test-custom-script.yml (1)
264-265: Add quotes around Chrome path variableThe Chrome path should be quoted to prevent potential issues with paths containing spaces.
- echo "BROWSER_PATH=${{ steps.setup-chrome.outputs.chrome-path }}" >> $GITHUB_ENV - ${{ steps.setup-chrome.outputs.chrome-path }} --version + echo "BROWSER_PATH=\"${{ steps.setup-chrome.outputs.chrome-path }}\"" >> $GITHUB_ENV + "${{ steps.setup-chrome.outputs.chrome-path }}" --version
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/ci-test-custom-script.yml(1 hunks).github/workflows/ci-test-hosted.yml(1 hunks).github/workflows/ci-test-limited-with-count.yml(1 hunks).github/workflows/ci-test-limited.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/ci-test-custom-script.yml
263-263: shellcheck reported issue in this script: SC2086:info:1:70: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/ci-test-limited.yml
230-230: shellcheck reported issue in this script: SC2086:info:1:70: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/ci-test-hosted.yml
146-146: shellcheck reported issue in this script: SC2086:info:1:70: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/ci-test-limited-with-count.yml
239-239: shellcheck reported issue in this script: SC2086:info:1:70: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 yamllint (1.35.1)
.github/workflows/ci-test-limited.yml
[error] 223-223: trailing spaces
(trailing-spaces)
🔇 Additional comments (6)
.github/workflows/ci-test-hosted.yml (1)
141-145: Chrome setup configuration looks good
The Chrome version is correctly set to 129 as specified in the PR objective, with proper configuration for chromedriver and dependencies.
.github/workflows/ci-test-limited.yml (2)
225-229: Chrome setup configuration looks good
The Chrome version is correctly set to 129 as specified in the PR objective, with proper configuration for chromedriver and dependencies.
231-232: Quote the Chrome path to prevent word splitting
The Chrome path should be quoted to prevent potential issues with spaces or special characters.
.github/workflows/ci-test-limited-with-count.yml (2)
234-238: Chrome setup configuration looks good
The Chrome version is correctly set to 129 as specified in the PR objective, with proper configuration for chromedriver and dependencies.
240-241: Quote the Chrome path to prevent word splitting
The Chrome path should be quoted to prevent potential issues with spaces or special characters.
.github/workflows/ci-test-custom-script.yml (1)
260-262: Chrome 129 is compatible but consider using version range
Chrome 129 is a stable version and compatible with Cypress 13.13.0, but fixing to a specific version might be restrictive. Consider using a version range (e.g., >= 129.0.0) to get security updates while maintaining stability.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci-test-custom-script.yml(2 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/ci-test-custom-script.yml
263-263: shellcheck reported issue in this script: SC2086:info:1:70: Double quote to prevent globbing and word splitting
(shellcheck)
🔇 Additional comments (2)
.github/workflows/ci-test-custom-script.yml (2)
261-262: LGTM! Good practice setting explicit install flags
Explicitly setting install-chromedriver and install-dependencies to true improves workflow reproducibility.
Line range hint 373-377: LGTM! Proper Cypress test configuration
The Cypress test configuration is well-structured with appropriate browser selection and environment settings.
| id: setup-chrome | ||
| with: | ||
| chrome-version: stable | ||
| chrome-version: 129 | ||
| install-chromedriver: true | ||
| install-dependencies: true | ||
| - run: | | ||
| echo "BROWSER_PATH=$(which chrome)" >> $GITHUB_ENV | ||
| echo "BROWSER_PATH=${{ steps.setup-chrome.outputs.chrome-path }}" >> $GITHUB_ENV | ||
| ${{ steps.setup-chrome.outputs.chrome-path }} --version |
There was a problem hiding this comment.
💡 Codebase verification
Chrome version 129 is not yet released and may cause issues
The current stable Chrome version is 120. Using an unreleased version (129) could cause compatibility issues with Cypress and affect test reliability. Please use Chrome version 120 instead.
id: setup-chrome
with:
- chrome-version: 129
+ chrome-version: 120
install-chromedriver: true
install-dependencies: true🔗 Analysis chain
Chrome version 129 is not yet released and may cause issues
The current stable Chrome version is 120. Using an unreleased version (129) could cause compatibility issues with Cypress and affect snapshot testing reliability.
Apply this diff to fix the Chrome version:
with:
- chrome-version: 129
+ chrome-version: 120
install-chromedriver: true
install-dependencies: true🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify Chrome version compatibility with Cypress
fd -e json package.json --exec jq -r '.dependencies.cypress, .devDependencies.cypress'
# Check Cypress configuration for browser settings
fd -e json -e js cypress.config --exec cat {}
Length of output: 291
Script:
#!/bin/bash
# Check for any browser-related configurations in package.json files
fd -e json package.json --exec grep -l "browser\|chrome" {} | xargs cat
# Search for browser configuration in any test setup files
rg -g "!node_modules" -g "!dist" -l "browser.+config|chrome.+config"
# Look for any Playwright or Selenium configurations
fd -e config.js -e config.ts -e config.json --exec grep -l "browser\|chrome" {}
# Check GitHub workflow files for browser-related jobs
fd workflow -e yml -e yaml --exec grep -A 5 -B 5 "chrome-version:" {}
Length of output: 15487
Script:
#!/bin/bash
# Check Cypress configuration files for browser settings
cat app/client/cypress.config.ts app/client/cypress_ci.config.ts app/client/cypress_ci_custom.config.ts
# Look for Chrome version references in workflow files
fd -e yml -e yaml . --exec cat {} | grep -A 10 -B 10 "chrome-version:"
Length of output: 4963
Script:
#!/bin/bash
# Search for Chrome version in GitHub workflow files directly
find .github/workflows -type f -name "*.yml" -exec cat {} \;
# Check for any Chrome-related environment variables in workflow files
find .github/workflows -type f -exec grep -l "chrome" {} \;
Length of output: 67658
Script:
#!/bin/bash
# Search for Chrome version in ci-test-custom-script.yml
cat .github/workflows/ci-test-custom-script.yml
# Check for any Cypress browser configuration files
fd -e json -e js -e ts -g "!node_modules" -g "!dist" --exec grep -l "browser.*config\|chrome.*config" {}
Length of output: 21044
🧰 Tools
🪛 actionlint (1.7.4)
263-263: shellcheck reported issue in this script: SC2086:info:1:70: Double quote to prevent globbing and word splitting
(shellcheck)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci-test-custom-script.yml(2 hunks)
🔇 Additional comments (1)
.github/workflows/ci-test-custom-script.yml (1)
260-266:
Remove commented code and use stable Chrome version
The commented Chrome setup block specifies version 129 which:
- Conflicts with the container specification using Chrome 130
- Is not yet released (current stable is 120)
Either:
- Remove the commented code if not needed, or
- Uncomment and fix the version:
- # - uses: browser-actions/setup-chrome@latest
- # id: setup-chrome
- # with:
- # chrome-version: 129
- # install-chromedriver: true
- # install-dependencies: trueor
- uses: browser-actions/setup-chrome@latest
id: setup-chrome
with:
- chrome-version: 129
+ chrome-version: 120
install-chromedriver: true
install-dependencies: trueLikely invalid or redundant comment.
| container: | ||
| image: cypress/browsers:node-20.18.0-chrome-130.0.6723.69-1-ff-131.0.3-edge-130.0.2849.52-1 | ||
| options: --user 1001 |
There was a problem hiding this comment.
Version mismatch with PR objective
The container specification uses Chrome 130, but the PR objective is to fix the Chrome version to 129. This inconsistency could lead to test reliability issues.
container:
- image: cypress/browsers:node-20.18.0-chrome-130.0.6723.69-1-ff-131.0.3-edge-130.0.2849.52-1
+ image: cypress/browsers:node-20.18.0-chrome-120.0.6099.129-1-ff-120.0-edge-120.0.2210.61-1
options: --user 1001📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| container: | |
| image: cypress/browsers:node-20.18.0-chrome-130.0.6723.69-1-ff-131.0.3-edge-130.0.2849.52-1 | |
| options: --user 1001 | |
| container: | |
| image: cypress/browsers:node-20.18.0-chrome-120.0.6099.129-1-ff-120.0-edge-120.0.2210.61-1 | |
| options: --user 1001 |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/ci-test-custom-script.yml (1)
373-373:⚠️ Potential issueUpdate Chrome path to match version 120
The Chrome path needs to be updated to match the stable version.
- browser: /opt/hostedtoolcache/setup-chrome/chromium/129.0.6668.100/x64/chrome + browser: /opt/hostedtoolcache/setup-chrome/chromium/120.0.6099.129/x64/chrome
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/ci-test-custom-script.yml(3 hunks).github/workflows/ci-test-limited.yml(2 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/ci-test-limited.yml
230-230: shellcheck reported issue in this script: SC2086:info:1:70: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 yamllint (1.35.1)
.github/workflows/ci-test-limited.yml
[error] 223-223: trailing spaces
(trailing-spaces)
.github/workflows/ci-test-custom-script.yml
[error] 73-73: trailing spaces
(trailing-spaces)
🔇 Additional comments (2)
.github/workflows/ci-test-limited.yml (2)
227-229:
Chrome version 129 is not yet released
Using an unreleased Chrome version (129) could cause test reliability issues. The current stable version is 120.
Apply this change:
- chrome-version: 129
+ chrome-version: 120
install-chromedriver: true
install-dependencies: trueLikely invalid or redundant comment.
343-343:
Update Chrome path to match version 120
The Chrome path needs to be updated to match the suggested Chrome version.
- browser: /opt/hostedtoolcache/setup-chrome/chromium/129.0.6668.100/x64/chrome
+ browser: /opt/hostedtoolcache/setup-chrome/chromium/120.0.6099.129/x64/chromeLikely invalid or redundant comment.
| # - uses: browser-actions/setup-chrome@latest | ||
| # id: setup-chrome | ||
| # with: | ||
| # chrome-version: 129 | ||
| # install-chromedriver: true | ||
| # install-dependencies: true | ||
|
|
There was a problem hiding this comment.
Uncomment Chrome setup to ensure browser availability
The Chrome setup is commented out but the workflow still expects Chrome to be available. This could cause test failures.
Remove the comment markers to enable Chrome setup:
- # - uses: browser-actions/setup-chrome@latest
- # id: setup-chrome
- # with:
- # chrome-version: 129
- # install-chromedriver: true
- # install-dependencies: true
+ - uses: browser-actions/setup-chrome@latest
+ id: setup-chrome
+ with:
+ chrome-version: 120
+ install-chromedriver: true
+ install-dependencies: true📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # - uses: browser-actions/setup-chrome@latest | |
| # id: setup-chrome | |
| # with: | |
| # chrome-version: 129 | |
| # install-chromedriver: true | |
| # install-dependencies: true | |
| - uses: browser-actions/setup-chrome@latest | |
| id: setup-chrome | |
| with: | |
| chrome-version: 120 | |
| install-chromedriver: true | |
| install-dependencies: true | |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12368919764. |
|
/ci-test-limit-count run_count=1 runId=12368615036 |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12369036510. |
|
/ci-test-limit-count run_count=1 |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12369211360. |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci-test-custom-script.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/ci-test-custom-script.yml
268-268: shellcheck reported issue in this script: SC2086:info:5:47: Double quote to prevent globbing and word splitting
(shellcheck)
🔇 Additional comments (1)
.github/workflows/ci-test-custom-script.yml (1)
257-266:
Uncomment the browser-actions/setup-chrome configuration
The Chrome setup configuration is commented out but should be used instead of manual installation. The GitHub Action provides a more reliable and secure way to manage Chrome versions.
Apply this diff to restore the proper Chrome setup:
-- # - uses: browser-actions/setup-chrome@latest
-- # id: setup-chrome
-- # with:
-- # chrome-version: 129
-- # install-chromedriver: true
-- # install-dependencies: true
-- # - run: |
-- # echo "BROWSER_PATH=${{ steps.setup-chrome.outputs.chrome-path }}" >> $GITHUB_ENV
-- # ${{ steps.setup-chrome.outputs.chrome-path }} --version
+- uses: browser-actions/setup-chrome@latest
+ id: setup-chrome
+ with:
+ chrome-version: "120.0.6099.129"
+ install-chromedriver: true
+ install-dependencies: true
+- run: |
+ echo "BROWSER_PATH=${{ steps.setup-chrome.outputs.chrome-path }}" >> $GITHUB_ENV
+ ${{ steps.setup-chrome.outputs.chrome-path }} --versionLikely invalid or redundant comment.
|
/ci-test-limit |
|
/ci-test-limit-count run_count=1 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/ci-test-hosted.yml (1)
140-147: LGTM! Consider adding error handlingThe Chrome installation steps are well-structured and align with the PR objective. However, consider adding error handling for the installation steps.
- name: Install Google Chrome 129.0.6668.100 run: | + set -e # Exit on error sudo apt-get remove google-chrome-stable wget -q https://dl.google.com/linux/chrome/deb/pool/main/g/google-chrome-stable/google-chrome-stable_129.0.6668.100-1_amd64.deb sudo apt-get update sudo apt-get install -y ./google-chrome-stable_129.0.6668.100-1_amd64.deb echo "BROWSER_PATH=$(which google-chrome)" >> $GITHUB_ENV + if ! google-chrome --version | grep -q "129.0.6668.100"; then + echo "Chrome version mismatch" + exit 1 + fi google-chrome --version🧰 Tools
🪛 actionlint (1.7.4)
141-141: shellcheck reported issue in this script: SC2086:info:5:47: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/ci-test-limited.yml (1)
223-223: Remove trailing whitespaceRemove the trailing whitespace on line 223.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 223-223: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/ci-test-custom-script.yml(1 hunks).github/workflows/ci-test-hosted.yml(2 hunks).github/workflows/ci-test-limited-with-count.yml(2 hunks).github/workflows/ci-test-limited.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/ci-test-custom-script.yml
258-258: shellcheck reported issue in this script: SC2086:info:5:47: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/ci-test-hosted.yml
141-141: shellcheck reported issue in this script: SC2086:info:5:47: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/ci-test-limited-with-count.yml
234-234: shellcheck reported issue in this script: SC2086:info:5:47: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/ci-test-limited.yml
225-225: shellcheck reported issue in this script: SC2086:info:5:47: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 yamllint (1.35.1)
.github/workflows/ci-test-limited.yml
[error] 223-223: trailing spaces
(trailing-spaces)
🔇 Additional comments (4)
.github/workflows/ci-test-hosted.yml (1)
249-249: LGTM! Browser configuration is correct
The Cypress test configuration correctly uses the BROWSER_PATH environment variable set during Chrome installation.
.github/workflows/ci-test-limited.yml (1)
224-231: Consider adding error handling
The same error handling improvements suggested for ci-test-hosted.yml should be applied here.
🧰 Tools
🪛 actionlint (1.7.4)
225-225: shellcheck reported issue in this script: SC2086:info:5:47: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/ci-test-limited-with-count.yml (2)
233-240: Consider adding error handling
The same error handling improvements suggested for ci-test-hosted.yml should be applied here.
🧰 Tools
🪛 actionlint (1.7.4)
234-234: shellcheck reported issue in this script: SC2086:info:5:47: Double quote to prevent globbing and word splitting
(shellcheck)
354-355: LGTM! Cypress-repeat-pro configuration is correct
The cypress-repeat-pro configuration correctly uses the BROWSER_PATH environment variable for browser specification.
| - name: Install Google Chrome 129.0.6668.100 | ||
| run: | | ||
| sudo apt-get remove google-chrome-stable | ||
| wget -q https://dl.google.com/linux/chrome/deb/pool/main/g/google-chrome-stable/google-chrome-stable_129.0.6668.100-1_amd64.deb | ||
| sudo apt-get update | ||
| sudo apt-get install -y ./google-chrome-stable_129.0.6668.100-1_amd64.deb | ||
| echo "BROWSER_PATH=$(which google-chrome)" >> $GITHUB_ENV | ||
| google-chrome --version |
There was a problem hiding this comment.
Critical: Remove manual Chrome installation and use browser-actions/setup-chrome
The current Chrome installation has several critical issues:
- Chrome version 129.0.6668.100 is not yet released (current stable is 120)
- Direct downloads without verification pose security risks
- The installation URL will fail as this version doesn't exist
Replace the manual installation with the browser-actions/setup-chrome action:
- - name: Install Google Chrome 129.0.6668.100
- run: |
- sudo apt-get remove google-chrome-stable
- wget -q https://dl.google.com/linux/chrome/deb/pool/main/g/google-chrome-stable/google-chrome-stable_129.0.6668.100-1_amd64.deb
- sudo apt-get update
- sudo apt-get install -y ./google-chrome-stable_129.0.6668.100-1_amd64.deb
- echo "BROWSER_PATH=$(which google-chrome)" >> $GITHUB_ENV
- google-chrome --version
+ - uses: browser-actions/setup-chrome@latest
+ id: setup-chrome
+ with:
+ chrome-version: '120'
+ install-chromedriver: true
+ install-dependencies: true📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Install Google Chrome 129.0.6668.100 | |
| run: | | |
| sudo apt-get remove google-chrome-stable | |
| wget -q https://dl.google.com/linux/chrome/deb/pool/main/g/google-chrome-stable/google-chrome-stable_129.0.6668.100-1_amd64.deb | |
| sudo apt-get update | |
| sudo apt-get install -y ./google-chrome-stable_129.0.6668.100-1_amd64.deb | |
| echo "BROWSER_PATH=$(which google-chrome)" >> $GITHUB_ENV | |
| google-chrome --version | |
| - uses: browser-actions/setup-chrome@latest | |
| id: setup-chrome | |
| with: | |
| chrome-version: '120' | |
| install-chromedriver: true | |
| install-dependencies: true |
🧰 Tools
🪛 actionlint (1.7.4)
258-258: shellcheck reported issue in this script: SC2086:info:5:47: Double quote to prevent globbing and word splitting
(shellcheck)
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12371594512. |
2 participants
Description
Fix chrome version for snapshot testing.
Fixes #
Issue Numberor
Fixes
Issue URLWarning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.Sanity"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12371605353
Commit: cc4ac10
Cypress dashboard.
Tags:
@tag.SanitySpec:
Tue, 17 Dec 2024 11:35:17 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
ted_tag,run_count,previous-workflow-run-id) to enhance workflow flexibility.Bug Fixes
Improvements
Chores