-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ci: Added ci option for snapshot #38261
Conversation
WalkthroughThe pull request modifies GitHub workflow files to enhance CI/CD processes, specifically focusing on adding flexibility to test execution and snapshot management. The changes introduce new input parameters Changes
Assessment against linked issues
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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/build-client-server-count.yml (1)
43-57
: Consider using explicit string comparison for bash conditions.When checking empty strings in bash, it's safer to use explicit string comparison.
- if [[ -z "$checkArg" ]]; then + if [[ "$checkArg" == "" ]]; then.github/workflows/ci-test-limited-with-count.yml (1)
22-31
: Fix YAML indentation and trailing spaces.The YAML indentation for input parameters should be 8 spaces, and trailing spaces should be removed.
update_snapshot: - description: 'Give option to update snapshot (true/false)' + description: 'Give option to update snapshot (true/false)' - required: false + required: false - type: boolean + type: boolean - default: false + default: false specs_to_run: description: 'Cypress spec file(s) to run' required: false - type: string + type: stringAlso applies to: 48-56
🧰 Tools
🪛 yamllint (1.35.1)
[warning] 23-23: wrong indentation: expected 8 but found 10
(indentation)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/build-client-server-count.yml
(5 hunks).github/workflows/ci-test-limited-with-count.yml
(5 hunks).github/workflows/ci-test-limited.yml
(1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/ci-test-limited-with-count.yml
[warning] 23-23: wrong indentation: expected 8 but found 10
(indentation)
[warning] 49-49: wrong indentation: expected 8 but found 10
(indentation)
[error] 56-56: trailing spaces
(trailing-spaces)
[error] 133-133: trailing spaces
(trailing-spaces)
[error] 140-140: trailing spaces
(trailing-spaces)
[error] 144-144: trailing spaces
(trailing-spaces)
[error] 501-501: trailing spaces
(trailing-spaces)
🔇 Additional comments (7)
.github/workflows/ci-test-limited.yml (1)
421-421
: LGTM! Good use of workspace variable.
Using ${{ github.workspace }}
instead of relative path ensures consistency across different GitHub Actions runners.
.github/workflows/build-client-server-count.yml (3)
20-21
: LGTM! Good addition of new outputs.
The new outputs update_snapshot
and specs_to_run
enhance the workflow's flexibility for snapshot testing.
117-117
: LGTM! Enhanced PR comment with spec information.
Adding spec information to the PR comment improves visibility of test execution.
169-170
: LGTM! Consistent parameter passing.
The new parameters are correctly passed to both test execution workflows.
Also applies to: 184-185
.github/workflows/ci-test-limited-with-count.yml (3)
133-140
: LGTM! Good addition of snapshot artifact upload.
The step to upload existing snapshots is correctly implemented with appropriate conditions.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 133-133: trailing spaces
(trailing-spaces)
[error] 140-140: trailing spaces
(trailing-spaces)
379-391
: LGTM! Well-structured conditional Cypress execution.
The implementation correctly handles snapshot updates based on the input parameter.
495-502
: LGTM! Good addition of new snapshot comparison.
The step to upload new compared snapshots ensures proper tracking of snapshot changes.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 501-501: trailing spaces
(trailing-spaces)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm
## Description Adding workflow for updating snapshot with github CI. Fixes #[`36419` ](#36419) ## 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/12426577557> > Commit: 2660c07 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=12426577557&attempt=2" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Sanity` > Spec: > <hr>Fri, 20 Dec 2024 07:02:16 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 CI workflow with new input parameters for snapshot updates and spec execution. - Added steps for uploading existing and new snapshot data during Cypress tests. - **Bug Fixes** - Improved artifact management and error handling in the CI process. - **Documentation** - Added comments for better understanding of workflow logic and input parameters. - **Refactor** - Streamlined job execution logic and conditional checks for test reruns. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Description
Adding workflow for updating snapshot with github CI.
Fixes #
36419
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/12426577557
Commit: 2660c07
Cypress dashboard.
Tags:
@tag.Sanity
Spec:
Fri, 20 Dec 2024 07:02:16 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor