ci: updated cores #38420
ci: updated cores #38420
Conversation
WalkthroughThe pull request modifies the GitHub Actions workflow configuration for Appsmith's CI Test on hosted instances. Key changes include updating the runner to Changes
Suggested labels
Suggested reviewers
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (3)
🧰 Additional context used📓 Path-based instructions (3)app/client/cypress/e2e/GSheet/WidgetBinding_AllAccess_Spec.ts (1)Pattern
app/client/cypress/e2e/GSheet/GsheetMisc_Spec.ts (1)Pattern
app/client/cypress/e2e/GSheet/ReadOnly_Access_Spec.ts (1)Pattern
🔇 Additional comments (3)
🪧 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
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: 0
🧹 Nitpick comments (1)
.github/workflows/ci-test-hosted.yml (1)
Line range hint
127-134: Consider using a version variable for Chrome installationHard-coding the Chrome version might lead to maintenance overhead. Consider using a variable or environment parameter for easier version management.
- 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 install -y ./google-chrome-stable_129.0.6668.100-1_amd64.deb + CHROME_VERSION="129.0.6668.100" + wget -q "https://dl.google.com/linux/chrome/deb/pool/main/g/google-chrome-stable/google-chrome-stable_${CHROME_VERSION}-1_amd64.deb" + sudo apt-get install -y "./google-chrome-stable_${CHROME_VERSION}-1_amd64.deb"
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci-test-hosted.yml(1 hunks)
🔇 Additional comments (3)
.github/workflows/ci-test-hosted.yml (3)
Line range hint 11-19: LGTM: Well-structured input parameter addition
The new pr parameter is properly configured with appropriate type validation and default values.
24-24: LGTM: Enhanced runner configuration
The upgrade to 8-cores aligns with the PR objective to address memory-related crashes in Google Sheets test cases.
Line range hint 308-311: Consider obfuscating internal URLs in Slack notifications
The Slack messages contain direct links to the internal Cypress dashboard. Consider using a URL shortener or removing sensitive parts of the URL path for security.
Also applies to: 317-320
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/ci-test-hosted.yml (1)
Line range hint
308-321: Consider enhancing error reporting with failure categories.While the current implementation effectively reports test failures, consider categorizing failures (e.g., memory issues, timeouts) to help track the impact of the resource updates.
- name: Generate slack message continue-on-error: true if: always() id: slack_notification run: | if [[ "${{ steps.test_status.outputs.run_result }}" == "failedtest" ]]; then - echo "slack_message=There are test failures in the run. Cypress Dashboard: <https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=${{ github.run_id }}&attempt=${{ github.run_attempt }}&selectiontype=test&testsstatus=failed&specsstatus=fail|Click here!>" >> $GITHUB_OUTPUT + # Extract failure category from test results + FAILURE_TYPE=$(jq -r '.stats.failures[] | .category' ~/results/*/results.json | sort | uniq -c | tr '\n' ',' | sed 's/,$//') + echo "slack_message=Test failures detected ($FAILURE_TYPE). Cypress Dashboard: <https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=${{ github.run_id }}&attempt=${{ github.run_attempt }}&selectiontype=test&testsstatus=failed&specsstatus=fail|Click here!>" >> $GITHUB_OUTPUT echo "slack_color=#FF0000" >> $GITHUB_OUTPUT echo "slack_icon=:parachute:" >> $GITHUB_OUTPUT
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci-test-hosted.yml(2 hunks)
🔇 Additional comments (2)
.github/workflows/ci-test-hosted.yml (2)
Line range hint 132-140: LGTM: Chrome installation properly configured.
The explicit Chrome version specification ensures consistent test environment across runs.
254-254: LGTM: Headless mode optimizes resource usage.
Running Cypress tests in headless mode reduces memory consumption, which aligns with the optimization goals.
| @@ -21,7 +21,7 @@ on: | |||
|
|
|||
| jobs: | |||
| ci-test: | |||
| runs-on: ubuntu-latest | |||
| runs-on: ubuntu-latest-8-cores | |||
There was a problem hiding this comment.
💡 Codebase verification
Warning: ubuntu-latest-8-cores is not a standard GitHub-hosted runner
The runner configuration ubuntu-latest-8-cores appears to be a custom runner label that's used in several workflows:
- ci-test-hosted.yml
- server-build.yml
- client-lint.yml
- client-unit-tests.yml
GitHub's standard hosted runners are ubuntu-latest, windows-latest, and macos-latest. Unless your organization has set up self-hosted runners with this label, this change will cause workflow failures.
🔗 Analysis chain
LGTM: Runner upgrade addresses memory issues.
The upgrade to 8 cores aligns with the goal of resolving Google Sheets test crashes due to high memory usage.
Let's verify the runner's availability:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify if the runner is available in the GitHub Actions environment
gh api \
-H "Accept: application/vnd.github+json" \
-H "X-GitHub-Api-Version: 2022-11-28" \
/repos/$GITHUB_REPOSITORY/actions/runners | jq '.runners[] | select(.name | contains("ubuntu-latest-8-cores"))'
Length of output: 264
Script:
#!/bin/bash
# Let's check the GitHub Actions workflow files for runner configurations
rg -U "runs-on:.*ubuntu.*" .github/workflows/
Length of output: 4235
Script:
#!/bin/bash
# Let's also check if there are any other workflow files using similar runner configurations
fd -e yml -e yaml . .github/workflows/ --exec grep -l "runs-on.*8-cores" {}
Length of output: 225
Script:
#!/bin/bash
# Check GitHub Actions documentation references or comments about runner specs
rg -i "runner|cores|compute" .github/workflows/
Length of output: 1924
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/ci-test-hosted.yml (1)
185-190: Optimize resource monitoring implementationThe current implementation could be improved for better efficiency:
- Consider using
vmstatorpidstatinstead oftopfor lower overhead- Increase the monitoring interval to reduce system load
- nohup bash -c 'while true; do echo "==== CPU & RAM Usage ===="; top -b -n 1 | head -n 10; echo "==== Disk Usage ===="; df -h; sleep 10; done' > resource_usage.log & + nohup bash -c 'while true; do echo "==== CPU & RAM Usage ===="; vmstat 1 1; echo "==== Process Stats ===="; pidstat 1 1; echo "==== Disk Usage ===="; df -h; sleep 30; done' > resource_usage.log &🧰 Tools
🪛 yamllint (1.35.1)
[error] 189-189: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci-test-hosted.yml(3 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/ci-test-hosted.yml
[error] 189-189: trailing spaces
(trailing-spaces)
[error] 265-265: trailing spaces
(trailing-spaces)
[error] 270-270: trailing spaces
(trailing-spaces)
🪛 actionlint (1.7.4)
.github/workflows/ci-test-hosted.yml
262-262: shellcheck reported issue in this script: SC2046:warning:1:6: Quote this to prevent word splitting
(shellcheck)
267-267: the runner of "actions/upload-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (1)
.github/workflows/ci-test-hosted.yml (1)
24-24: Verify runner availability before merging
The ubuntu-latest-8-cores runner appears to be a custom configuration. Ensure this runner is properly configured in your GitHub organization's self-hosted runners to prevent workflow failures.
.github/workflows/ci-test-hosted.yml
Outdated
| - name: Upload Resource Usage Log | ||
| uses: actions/upload-artifact@v3 | ||
| with: | ||
| name: resource-usage-log | ||
| path: resource_usage.log | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update upload-artifact action to v4
For consistency with other upload-artifact actions in this workflow, update to v4.
- uses: actions/upload-artifact@v3
+ uses: actions/upload-artifact@v4📝 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: Upload Resource Usage Log | |
| uses: actions/upload-artifact@v3 | |
| with: | |
| name: resource-usage-log | |
| path: resource_usage.log | |
| - name: Upload Resource Usage Log | |
| uses: actions/upload-artifact@v4 | |
| with: | |
| name: resource-usage-log | |
| path: resource_usage.log | |
🧰 Tools
🪛 actionlint (1.7.4)
267-267: the runner of "actions/upload-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 yamllint (1.35.1)
[error] 270-270: trailing spaces
(trailing-spaces)
.github/workflows/ci-test-hosted.yml
Outdated
| - name: Stop Monitoring | ||
| run: | | ||
| kill $(cat resource_monitor_pid) || true | ||
| echo "Resource monitoring stopped." |
There was a problem hiding this comment.
Fix shell script quoting in cleanup command
The kill command should use proper quoting to handle PIDs with spaces safely.
- kill $(cat resource_monitor_pid) || true
+ kill "$(cat resource_monitor_pid)" || 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: Stop Monitoring | |
| run: | | |
| kill $(cat resource_monitor_pid) || true | |
| echo "Resource monitoring stopped." | |
| - name: Stop Monitoring | |
| run: | | |
| kill "$(cat resource_monitor_pid)" || true | |
| echo "Resource monitoring stopped." |
🧰 Tools
🪛 actionlint (1.7.4)
262-262: shellcheck reported issue in this script: SC2046:warning:1:6: Quote this to prevent word splitting
(shellcheck)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/ci-test-hosted.yml (1)
185-190: Enhance resource monitoring implementationConsider structuring the output in JSON format for better parsing and analysis.
- nohup bash -c 'while true; do echo "==== CPU & RAM Usage ===="; top -b -n 1 | head -n 10; echo "==== Disk Usage ===="; df -h; sleep 10; done' > resource_usage.log & + nohup bash -c ' + while true; do + echo "{\"timestamp\":\"$(date -Iseconds)\",\"cpu_ram\":\"$(top -bn1 | head -n 10 | jq -Rsc)\",\"disk\":\"$(df -h | jq -Rsc)\"}" + sleep 10 + done + ' > resource_usage.log 2>&1 &🧰 Tools
🪛 yamllint (1.35.1)
[error] 189-189: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci-test-hosted.yml(3 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/ci-test-hosted.yml
[error] 189-189: trailing spaces
(trailing-spaces)
[error] 266-266: trailing spaces
(trailing-spaces)
[error] 268-268: trailing spaces
(trailing-spaces)
[error] 272-272: trailing spaces
(trailing-spaces)
🪛 actionlint (1.7.4)
.github/workflows/ci-test-hosted.yml
263-263: shellcheck reported issue in this script: SC2046:warning:1:6: Quote this to prevent word splitting
(shellcheck)
269-269: the runner of "actions/upload-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (3)
.github/workflows/ci-test-hosted.yml (3)
261-264:
Fix shell script quoting for safety
The kill command should use proper quoting to handle PIDs safely.
- kill $(cat resource_monitor_pid) || true
+ kill "$(cat resource_monitor_pid)" || trueLikely invalid or redundant comment.
🧰 Tools
🪛 actionlint (1.7.4)
263-263: shellcheck reported issue in this script: SC2046:warning:1:6: Quote this to prevent word splitting
(shellcheck)
267-273: 🛠️ Refactor suggestion
Update upload-artifact action to v4
For consistency with other upload-artifact actions in this workflow, update to v4.
- uses: actions/upload-artifact@v3
+ uses: actions/upload-artifact@v4Likely invalid or redundant comment.
🧰 Tools
🪛 actionlint (1.7.4)
269-269: the runner of "actions/upload-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 yamllint (1.35.1)
[error] 268-268: trailing spaces
(trailing-spaces)
[error] 272-272: trailing spaces
(trailing-spaces)
24-24: Verify custom runner configuration
The ubuntu-latest-8-cores appears to be a custom runner. Ensure it's properly configured in your GitHub organization's self-hosted runners.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/ci-test-hosted.yml (1)
262-274:⚠️ Potential issueFix shell script quoting and update upload-artifact action.
- The
killcommand needs proper quoting- The upload-artifact action should be updated to v4
Apply this diff to fix the issues:
- kill $(cat resource_monitor_pid) || true + kill "$(cat resource_monitor_pid)" || true - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4🧰 Tools
🪛 actionlint (1.7.4)
264-264: shellcheck reported issue in this script: SC2046:warning:1:6: Quote this to prevent word splitting
(shellcheck)
270-270: the runner of "actions/upload-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 yamllint (1.35.1)
[error] 267-267: trailing spaces
(trailing-spaces)
[error] 269-269: trailing spaces
(trailing-spaces)
[error] 273-273: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci-test-hosted.yml(3 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/ci-test-hosted.yml
[error] 189-189: trailing spaces
(trailing-spaces)
[error] 267-267: trailing spaces
(trailing-spaces)
[error] 269-269: trailing spaces
(trailing-spaces)
[error] 273-273: trailing spaces
(trailing-spaces)
🪛 actionlint (1.7.4)
.github/workflows/ci-test-hosted.yml
264-264: shellcheck reported issue in this script: SC2046:warning:1:6: Quote this to prevent word splitting
(shellcheck)
270-270: the runner of "actions/upload-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (2)
.github/workflows/ci-test-hosted.yml (2)
24-24: Verify the availability of the custom runner.
The runner ubuntu-latest-8-cores appears to be a custom configuration. While this aligns with the PR objective to address memory issues, ensure it's properly configured in your GitHub organization.
Line range hint 192-254: Enable Cypress debug mode with caution.
The addition of DEBUG: 'cypress:*' will generate verbose logs. While this helps with debugging, it might impact performance and generate large log files.
Run this script to check the impact on log size:
✅ Verification successful
Let me analyze the Cypress configuration and workflow files to better understand the debug logging setup. Since the GitHub API access is not available, we'll look at the local configuration to verify the debug mode settings and their potential impact.
Debug mode is already configured appropriately in the CI pipeline
The DEBUG: 'cypress:*' setting is already being used selectively in the CI setup script (app/client/cypress/setup-test-ci.sh) and the project uses proper reporters (cypress-mochawesome-reporter, cypress-multi-reporters) to handle logs efficiently. The debug configuration in the workflow file follows Cypress's recommended practices for CI environments.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check recent workflow runs for log size comparison
gh api -X GET /repos/$GITHUB_REPOSITORY/actions/runs \
--jq '.workflow_runs[] | select(.name=="Appsmith CI Test Workflow For Hosted Instance") | {id: .id, status: .status, conclusion: .conclusion}'
Length of output: 293
Script:
#!/bin/bash
# Let's check the Cypress configuration and test files to understand the debug logging impact
fd -e json -e js cypress.config
rg -l "DEBUG" --type yaml .github/workflows/
Length of output: 208
Script:
#!/bin/bash
# Check if there are any existing debug configurations or patterns
rg "DEBUG.*cypress" -A 2 -B 2
Length of output: 492
Script:
#!/bin/bash
# Look for any Cypress reporter configurations that might be affected
rg "reporter" --type json
rg "reporter" -g "cypress.config*"
Length of output: 297
🧰 Tools
🪛 yamllint (1.35.1)
[error] 189-189: trailing spaces
(trailing-spaces)
| - name: Monitor Resource Usage | ||
| run: | | ||
| echo "Monitoring system metrics during Cypress tests..." | ||
| nohup bash -c 'while true; do echo "==== CPU & RAM Usage ===="; top -b -n 1 | head -n 10; echo "==== Disk Usage ===="; df -h; sleep 10; done' > resource_usage.log & | ||
| echo $! > resource_monitor_pid | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Resource monitoring implementation needs improvement.
The current implementation has several issues:
- The monitoring interval (10s) might be too frequent and could impact test performance
- The output format makes it difficult to parse and analyze the data
- There's no error handling for the background process
Apply this diff to improve the implementation:
- - name: Monitor Resource Usage
- run: |
- echo "Monitoring system metrics during Cypress tests..."
- nohup bash -c 'while true; do echo "==== CPU & RAM Usage ===="; top -b -n 1 | head -n 10; echo "==== Disk Usage ===="; df -h; sleep 10; done' > resource_usage.log &
- echo $! > resource_monitor_pid
+ - name: Monitor Resource Usage
+ run: |
+ echo "Monitoring system metrics during Cypress tests..."
+ nohup bash -c '
+ while true; do
+ timestamp=$(date -u +"%Y-%m-%dT%H:%M:%SZ")
+ echo "{"
+ echo " \"timestamp\": \"$timestamp\","
+ echo " \"cpu_ram\": $(top -bn1 | head -n 10 | jq -R -s .),"
+ echo " \"disk\": $(df -h | jq -R -s .)"
+ echo "},"
+ sleep 30
+ done
+ ' > resource_usage.log 2>&1 &
+ echo $! > resource_monitor_pid📝 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: Monitor Resource Usage | |
| run: | | |
| echo "Monitoring system metrics during Cypress tests..." | |
| nohup bash -c 'while true; do echo "==== CPU & RAM Usage ===="; top -b -n 1 | head -n 10; echo "==== Disk Usage ===="; df -h; sleep 10; done' > resource_usage.log & | |
| echo $! > resource_monitor_pid | |
| - name: Monitor Resource Usage | |
| run: | | |
| echo "Monitoring system metrics during Cypress tests..." | |
| nohup bash -c ' | |
| while true; do | |
| timestamp=$(date -u +"%Y-%m-%dT%H:%M:%SZ") | |
| echo "{" | |
| echo " \"timestamp\": \"$timestamp\"," | |
| echo " \"cpu_ram\": $(top -bn1 | head -n 10 | jq -R -s .)," | |
| echo " \"disk\": $(df -h | jq -R -s .)" | |
| echo "}," | |
| sleep 30 | |
| done | |
| ' > resource_usage.log 2>&1 & | |
| echo $! > resource_monitor_pid |
🧰 Tools
🪛 yamllint (1.35.1)
[error] 189-189: trailing spaces
(trailing-spaces)
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
app/client/cypress/support/Pages/GSheetHelper.ts (1)
Line range hint
88-89: Avoid usingagHelper.Sleepcalls.
This code callsthis.agHelper.Sleep(500), which is strongly discouraged according to the Cypress guidelines for stable e2e tests. It is preferable to ensure the required element or resource is ready without introducing arbitrary wait times.- this.agHelper.Sleep(500); + // Wait for an explicit element to appear or rely on assertion-based checks
🧹 Nitpick comments (1)
app/client/cypress/support/Pages/GSheetHelper.ts (1)
93-98: Clean up commented-out code.
The oldEnterValueapproach is now commented out. If you no longer intend to use it, removing these lines will help keep the code concise and maintainable.- // this.agHelper.EnterValue(rowData, { - // propFieldName: "", - // directInput: false, - // inputFieldName: inputField, - // });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/client/cypress/support/Pages/GSheetHelper.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
app/client/cypress/support/Pages/GSheetHelper.ts (1)
Pattern app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
🔇 Additional comments (1)
app/client/cypress/support/Pages/GSheetHelper.ts (1)
99-100: Confirm usage of data- attributes and stable locators.*
ReplacingEnterValuewithPasteis valid, but confirm thatthis.locator._inputFieldByName(inputField)leverages data-* attributes (as recommended by the coding guidelines) for robust and stable selector usage in Cypress.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/client/cypress/e2e/GSheet/WidgetBinding_SelectedAccess_Spec.ts (1)
Line range hint
75-75: Avoid usage of agHelper.Sleep(500)
Per the Cypress coding guidelines, using sleep calls is discouraged. Consider alternative solutions such as waiting on specific events or network calls, or using built-in Cypress commands with proper timeouts.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/client/cypress/e2e/GSheet/WidgetBinding_SelectedAccess_Spec.ts(1 hunks)app/client/cypress/support/Pages/GSheetHelper.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/client/cypress/support/Pages/GSheetHelper.ts
🧰 Additional context used
📓 Path-based instructions (1)
app/client/cypress/e2e/GSheet/WidgetBinding_SelectedAccess_Spec.ts (1)
Pattern app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
🔇 Additional comments (1)
app/client/cypress/e2e/GSheet/WidgetBinding_SelectedAccess_Spec.ts (1)
44-44: Use caution with partial data insertion
Reducing the inserted rows from indexes 2 to 10 might improve performance or test reliability, but ensure that downstream functionalities expecting the full dataset still work properly.
Description
Identified multiple crashes in GSheet test cases, primarily caused by high memory usage. To address this, we are improving performance by updating the base image to a higher configuration.
Fixes # https://app.zenhub.com/workspaces/stability-pod-6690c4814e31602e25cab7fd/issues/gh/appsmithorg/appsmith/38421
Automation
/ok-to-test tags="@tag.Datasource"
🔍 Cypress test results
Caution
🔴 🔴 🔴 Some tests have failed.
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12583570995
Commit: 9b0dd98
Cypress dashboard.
Tags: @tag.Datasource
Spec:
The following are new failures, please fix them before merging the PR:
- cypress/e2e/Regression/ServerSide/ApiTests/API_MultiPart_Spec.ts
List of identified flaky tests.Thu, 02 Jan 2025 14:52:26 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Summary by CodeRabbit