ci: Fix for command without spec#38353
Conversation
WalkthroughThe pull request modifies two GitHub Actions workflow files to improve the handling of test specifications. In Changes
Possibly related PRs
Suggested Labels
Suggested Reviewers
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
|
![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/build-client-server-count.yml (1)
51-58: LGTM with minor formatting fix.The implementation for handling the
specs_to_runargument is clean and consistent with other argument handling in the file.Remove the trailing space on line 51.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 51-51: trailing spaces
(trailing-spaces)
.github/workflows/ci-test-limited-with-count.yml (1)
144-195: LGTM with suggested improvements.The implementation provides robust error handling and clear logging for spec retrieval. Consider the following improvements:
- Quote variables to prevent word splitting:
-if [[ -z "$specs_to_run" || "$specs_to_run" == "no_data" ]]; then +if [[ -z "${specs_to_run}" || "${specs_to_run}" == "no_data" ]]; then
Remove trailing spaces on lines 151 and 195.
Consider using an array to store specs instead of string concatenation:
-specs_to_run="$specs_to_run,$line" +specs_array+=("$line") +specs_to_run=$(IFS=,; echo "${specs_array[*]}")🧰 Tools
🪛 actionlint (1.7.4)
146-146: shellcheck reported issue in this script: SC2086:info:48:38: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 yamllint (1.35.1)
[error] 151-151: trailing spaces
(trailing-spaces)
[error] 195-195: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/build-client-server-count.yml(2 hunks).github/workflows/ci-test-limited-with-count.yml(3 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/build-client-server-count.yml
[error] 51-51: trailing spaces
(trailing-spaces)
.github/workflows/ci-test-limited-with-count.yml
[error] 151-151: trailing spaces
(trailing-spaces)
[error] 195-195: trailing spaces
(trailing-spaces)
🪛 actionlint (1.7.4)
.github/workflows/ci-test-limited-with-count.yml
146-146: shellcheck reported issue in this script: SC2086:info:48:38: Double quote to prevent globbing and word splitting
(shellcheck)
🔇 Additional comments (2)
.github/workflows/build-client-server-count.yml (1)
118-118: LGTM!
The PR comment body has been simplified to show only essential information.
.github/workflows/ci-test-limited-with-count.yml (1)
31-31: LGTM!
The specs_to_run parameter is consistently defined for both workflow triggers with appropriate defaults and descriptions.
Also applies to: 57-58
sagar-qa007
changed the title
github-actions
bot
added
the
skip-changelog
## Description Fix for command without spec. Fixes # https://app.zenhub.com/workspaces/stability-pod-6690c4814e31602e25cab7fd/issues/gh/appsmithorg/appsmith/38352 ## Automation /ok-to-test tags="@tag.Sanity" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/12482462821> > Commit: add048b > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=12482462821&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Sanity` > Spec: > <hr>Tue, 24 Dec 2024 14:11:18 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced flexibility in specifying test files with a new input parameter `specs_to_run`. - Default value for `specs_to_run` is now set to `'no_data'` when not provided. - **Bug Fixes** - Improved error handling for cases where no specifications are provided, including fallback to a specified file. - **Documentation** - Updated comments for clarity in workflow processes. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 participants
Description
Fix for command without spec.
Fixes # https://app.zenhub.com/workspaces/stability-pod-6690c4814e31602e25cab7fd/issues/gh/appsmithorg/appsmith/38352
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/12482462821
Commit: add048b
Cypress dashboard.
Tags:
@tag.SanitySpec:
Tue, 24 Dec 2024 14:11:18 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
specs_to_run.specs_to_runis now set to'no_data'when not provided.Bug Fixes
Documentation