chore: Update dispatch option #37066
Conversation
WalkthroughThe changes introduce a Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
🪧 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: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- .github/workflows/server-build.yml (1 hunks)
🔇 Additional comments (2)
.github/workflows/server-build.yml (2)
26-45: Implementation aligns with PR objectives.The addition of
workflow_dispatchsuccessfully implements the requirement for manual server test execution while maintaining compatibility with existing automation. The input parameters mirror theworkflow_callconfiguration, ensuring consistent behavior across different trigger methods.
26-45: Verify workflow integration with existing conditions.The workflow uses these inputs in multiple conditions. For example:
if: inputs.skip-tests != 'true' if: inputs.is-pg-build == 'true'These string comparisons will need updates if you switch to boolean types.
Let's verify all usage of these parameters:
✅ Verification successful
The previous command might have failed due to escaping issues. Let's try a simpler approach to find all workflow files and examine their content for these input parameters.
Let's try a different approach by examining each yaml file separately and looking for these specific input usages. This should help us identify where and how these inputs are being used in conditions.
Let's focus our search specifically on GitHub workflow files since the original review comment was about
.github/workflows/server-build.yml. This should help us find where these input parameters are actually being used in conditions.
Let's examine the entire content of the server-build.yml file directly to see how these input parameters are being used in the workflow. This will help us verify if there are any string comparisons using these inputs that would need updates.
No changes needed for string comparisons in workflow conditions
After examining the server-build.yml workflow file, I found that the input parameters are correctly used in conditions. The string comparisons are intentionally using string values 'true'/'false' because:
- The input parameters are explicitly defined as type: string with default: "false"
- The conditions in the workflow are correctly comparing with string literals:
if: inputs.skip-tests != 'true'if: inputs.is-pg-build == 'true'This is the standard way to handle boolean flags in GitHub Actions workflows, as workflow inputs of type string with 'true'/'false' values are a common pattern.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Find all conditions using these input parameters rg "inputs\.(skip-tests|is-pg-build)" -A 2 -B 2Length of output: 49
Script:
#!/bin/bash # Let's try a different approach to find these workflow inputs usage # Search in all yaml/yml files fd -e yml -e yaml -x grep -l "inputs" {} \; | xargs cat | grep -A 2 -B 2 "inputs\.\(skip-tests\|is-pg-build\)"Length of output: 113
Script:
#!/bin/bash # Let's try one more time with simpler grep approach fd -e yml -e yaml | while read -r file; do echo "=== $file ===" grep -A 2 -B 2 "inputs\.skip-tests\|inputs\.is-pg-build" "$file" || true doneLength of output: 8947
Script:
#!/bin/bash # Let's try searching in GitHub workflow files specifically fd -e yml -e yaml -p '.github/workflows' | while read -r file; do echo "=== $file ===" grep -A 2 -B 2 "inputs\." "$file" || true doneLength of output: 55
Script:
#!/bin/bash # Try a direct approach with the specific file if [ -f ".github/workflows/server-build.yml" ]; then echo "=== Content of server-build.yml ===" cat ".github/workflows/server-build.yml" fiLength of output: 16995
| workflow_dispatch: | ||
| inputs: | ||
| pr: | ||
| description: "PR number for the workflow" | ||
| required: false | ||
| type: number | ||
| skip-tests: | ||
| description: "Skip tests flag" | ||
| required: false | ||
| type: string | ||
| default: "false" | ||
| branch: | ||
| description: "Branch for the build" | ||
| required: false | ||
| type: string | ||
| is-pg-build: | ||
| description: "Flag for PG build" | ||
| required: false | ||
| type: string | ||
| default: "false" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using boolean type for flag parameters.
The skip-tests and is-pg-build parameters are boolean flags but defined as string type with "false" default. Using the native boolean type would be more appropriate and consistent with GitHub Actions best practices.
workflow_dispatch:
inputs:
pr:
description: "PR number for the workflow"
required: false
type: number
skip-tests:
description: "Skip tests flag"
required: false
- type: string
- default: "false"
+ type: boolean
+ default: false
branch:
description: "Branch for the build"
required: false
type: string
is-pg-build:
description: "Flag for PG build"
required: false
- type: string
- default: "false"
+ type: boolean
+ default: false📝 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.
| workflow_dispatch: | |
| inputs: | |
| pr: | |
| description: "PR number for the workflow" | |
| required: false | |
| type: number | |
| skip-tests: | |
| description: "Skip tests flag" | |
| required: false | |
| type: string | |
| default: "false" | |
| branch: | |
| description: "Branch for the build" | |
| required: false | |
| type: string | |
| is-pg-build: | |
| description: "Flag for PG build" | |
| required: false | |
| type: string | |
| default: "false" | |
| workflow_dispatch: | |
| inputs: | |
| pr: | |
| description: "PR number for the workflow" | |
| required: false | |
| type: number | |
| skip-tests: | |
| description: "Skip tests flag" | |
| required: false | |
| type: boolean | |
| default: false | |
| branch: | |
| description: "Branch for the build" | |
| required: false | |
| type: string | |
| is-pg-build: | |
| description: "Flag for PG build" | |
| required: false | |
| type: boolean | |
| default: false |
Description
We’ve often faced challenges in running server-specific tests efficiently for individual PRs. To streamline this process and improve testing speed, I’ve implemented the workflow dispatch feature, enabling quick and on-demand execution of server tests.
Fixes #
37060Automation
/ok-to-test tags="@tag.IDE"
🔍 Cypress test results
Warning
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11497895741
Commit: 48c3cd9
Cypress dashboard.
Tags: @tag.IDE
Spec:
It seems like no tests ran 😔. We are not able to recognize it, please check workflow here.
Fri, 25 Oct 2024 05:13:19 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?