-
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
test: Failing cypress tests due to removal of empty canvas prompts #34037
test: Failing cypress tests due to removal of empty canvas prompts #34037
Conversation
… remove obsolete empty canvas spec and related selectors.
WalkthroughThe recent changes focus on updating the Cypress visual regression tests for the AppPageLayout by importing additional modules and constants, and modifying the method used to add a new page. Additionally, the locators and methods related to generating CRUD pages have been removed, aligning with the objective to eliminate certain prompts from the first page of new apps. Changes
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
app/client/cypress/e2e/Regression/ClientSide/VisualTests/AppPageLayout_spec.js
Outdated
Show resolved
Hide resolved
@rahulbarwal Please change title of PR to |
@ApekshaBhosale Updated both title and body to reflect fixes. These 2 were reported as well, but should be resolved already. I have run tag.templates to make sure there is no issues with them. |
@rahulbarwal can you please add the RCA of the spec in the description as well? |
i didn't understand about mentioned two specs. There 2 were reported? can you give little more context please? |
In this message from Trisha But my original PR: #33993 took care of templates related issues. |
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
Outside diff range and nitpick comments (1)
app/client/cypress/support/Pages/HomePage.ts (1)
[!TIP]
Codebase VerificationThe
_buildFromDataTableActionCard
property is still referenced in the following files, albeit commented out:
app/client/cypress/e2e/Regression/ServerSide/QueryPane/Mongo1_spec.ts
app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/Postgres1_Spec.ts
app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/MySQL1_Spec.ts
These commented-out references should be removed to maintain code cleanliness and avoid confusion.
Analysis chain
Line range hint
1-1
: Removal of_buildFromDataTableActionCard
property aligns with PR objectives.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that no other methods are using the removed property. # Test: Search for the property usage in the HomePage class. Expect: No occurrences. rg --type ts '_buildFromDataTableActionCard' app/client/cypress/support/PagesLength of output: 77
Script:
#!/bin/bash # Description: Verify that no other parts of the codebase are using the removed property. # Test: Search for the property usage in the entire codebase. Expect: No occurrences. rg '_buildFromDataTableActionCard'Length of output: 520
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- app/client/cypress/e2e/Regression/ClientSide/VisualTests/AppPageLayout_spec.js (2 hunks)
- app/client/cypress/locators/GeneratePage.json (1 hunks)
- app/client/cypress/support/Pages/HomePage.ts (1 hunks)
Additional context used
Biome
app/client/cypress/support/Pages/HomePage.ts
[error] 269-269: The computed expression can be simplified without the use of a string literal. (lint/complexity/useLiteralKeys)
Unsafe fix: Use a literal key instead.
[error] 272-272: The computed expression can be simplified without the use of a string literal. (lint/complexity/useLiteralKeys)
Unsafe fix: Use a literal key instead.
[error] 402-402: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
Additional comments not posted (2)
app/client/cypress/locators/GeneratePage.json (1)
1-1
: Removal ofgenerateCRUDPageActionCard
locator aligns with PR objectives.app/client/cypress/e2e/Regression/ClientSide/VisualTests/AppPageLayout_spec.js (1)
1-7
: Imports and method update inAppPageLayout_spec.js
are appropriate and align with PR objectives.
…o fix/failing-specs-due-to-removal-of-empty-canvas-prompts
app/client/cypress/e2e/Regression/ClientSide/VisualTests/AppPageLayout_spec.js
Outdated
Show resolved
Hide resolved
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- app/client/cypress/e2e/Regression/ClientSide/VisualTests/AppPageLayout_spec.js (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- app/client/cypress/e2e/Regression/ClientSide/VisualTests/AppPageLayout_spec.js
Description
This PR fixes impact of #33993
Refactors visual regression tests to use PageList for page generation; remove obsolete empty canvas spec and related selectors.
cypress/e2e/Regression/ClientSide/OtherUIFeatures/EmptyCanvas_spec.js
cypress/e2e/Regression/ClientSide/VisualTests/AppPageLayout_spec.js
RCA:
The original PR catered to removal of empty canvas prompts and visual tests were not run leading to subsequent failures in the CI for EmptyCanvas_spec & AppPageLayout_spec.
This PR caters to failing visual tests, while running
@tag.Visual
we noticed that other (unrelated) visual specs started failing. These new failures fail in local as well.Whereas they were not failing in TBP or
@tag.All
runs and@tag.All
succeeded for this PR as well.Fixes #33874
or
Fixes
Issue URL
Warning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.All"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9413838227
Commit: b6d7f60
Cypress dashboard url: Click here!
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Refactor
Chores