ci: Checking no_data with contains to avoid error #38592
Conversation
WalkthroughThe pull request focuses on enhancing the GitHub Actions workflow file for CI testing, specifically improving the handling of Changes
Suggested labels
Suggested reviewers
Possibly related PRs
Poem
🪧 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
|
/ci-test-limit-count run_count=1 update_snapshot=true specs_to_run=cypress/e2e/Regression/ClientSide/Anvil/Widgets/AnvilButtonWidgetSnapshot_spec.ts |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/12740793582. |
![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 (2)
.github/workflows/ci-test-limited-with-count.yml (2)
181-184: Consider adding pattern validation for test specs.The code skips comments but doesn't validate if the remaining lines match the expected Cypress spec file pattern.
Add pattern validation:
if [[ $line =~ ^#|^\/\/ || -z $line ]]; then echo "[DEBUG] Skipped line: '$line'" # Indicate skipped lines continue +elif [[ ! $line =~ \.cy\.(js|ts|jsx|tsx)$ ]]; then + echo "[WARN] Invalid spec file pattern: '$line'" + continue fi
202-204: Remove trailing whitespace.There are trailing spaces at the end of line 202.
-echo "[DEBUG] Setting specs_to_run to GitHub environment variable: $specs_to_run" -echo "specs_to_run=$specs_to_run" >> $GITHUB_ENV - +echo "[DEBUG] Setting specs_to_run to GitHub environment variable: $specs_to_run" +echo "specs_to_run=$specs_to_run" >> $GITHUB_ENV
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci-test-limited-with-count.yml(2 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/ci-test-limited-with-count.yml
[error] 150-150: trailing spaces
(trailing-spaces)
[error] 177-177: trailing spaces
(trailing-spaces)
🔇 Additional comments (2)
.github/workflows/ci-test-limited-with-count.yml (2)
147-161: Enhanced debug logging and input source determination looks good!The implementation properly checks both
inputs.specs_to_runandgithub.event.inputs.specs_to_run, providing clear logging for better troubleshooting.🧰 Tools
🪛 yamllint (1.35.1)
[error] 150-150: trailing spaces
(trailing-spaces)
166-187: Verify the file existence check before processing.The code attempts to read
limited-tests.txtwithout explicitly checking if the file exists first. Whilelsandcatcommands are used, they don't prevent the subsequent file read operation.Add an explicit file check before processing:
limited_tests_file="${{ github.workspace }}/app/client/cypress/limited-tests.txt" +if [[ ! -f "$limited_tests_file" ]]; then + echo "[ERROR] Fallback file $limited_tests_file not found!" >&2 + exit 1 +fi ls -l ${{ github.workspace }}/app/client/cypress/limited-tests.txtLet's verify the current behavior:
✅ Verification successful
Current implementation is functionally safe
The file exists in the repository and the current implementation includes implicit checks through
lsandcatcommands. While adding an explicit check would improve error handling, the current code will fail visibly if the file is missing or unreadable.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the limited-tests.txt file exists and is readable if [[ -f "${{ github.workspace }}/app/client/cypress/limited-tests.txt" ]]; then echo "File exists and is readable" ls -l "${{ github.workspace }}/app/client/cypress/limited-tests.txt" else echo "File does not exist or is not readable" fiLength of output: 149
Script:
#!/bin/bash # Find the limited-tests.txt file echo "Searching for limited-tests.txt..." fd "limited-tests.txt" --type f # If found, try to read its contents if fd "limited-tests.txt" --type f -x cat {}; then echo "File content shown above" else echo "File not found or not readable" fiLength of output: 213
🧰 Tools
🪛 yamllint (1.35.1)
[error] 177-177: trailing spaces
(trailing-spaces)
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12740793582. |
|
/ci-test-limit-count run_count=1 runId=12740793582 |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/12741028237. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12741028237. |
Description
Found that existing text file was not reading when no_data passed. Fixed the problem by adding that condition.
Fixes # https://app.zenhub.com/workspaces/qa-63316faf86bb2e170ed2e46b/issues/gh/appsmithorg/appsmith/38591
Automation
/ok-to-test tags="@tag.IDE"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12740786426
Commit: 7c4e548
Cypress dashboard.
Tags:
@tag.IDESpec:
Mon, 13 Jan 2025 05:09:41 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit