-
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: adding tests for custom lib #37901
Conversation
WalkthroughThe changes in this pull request enhance the testing framework for managing JavaScript libraries using Cypress. A new test suite in Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
/ci-test-limit |
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 (3)
app/client/cypress/e2e/Regression/ClientSide/JSLibrary/Custom_lib_spec.ts (3)
53-58
: Remove commented-out code to maintain code cleanlinessThere are blocks of commented-out code in your test cases. Consider removing them if they're no longer needed to keep the codebase clean and maintainable.
Also applies to: 104-109
118-118
: Avoid using strings for assertionsUsing string literals in assertions can be brittle. Consider using more robust methods provided by your testing framework to validate error messages.
102-102
: Enhance download verification without relying on file namesRelying on exact file names for download verification can lead to flaky tests if the file names change. Consider verifying the download by checking the file content or existence instead.
Also applies to: 126-126
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
app/client/cypress/e2e/Regression/ClientSide/JSLibrary/Custom_lib_spec.ts
(1 hunks)app/client/cypress/limited-tests.txt
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
app/client/cypress/limited-tests.txt (1)
Pattern app/client/cypress/**/**.*
: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
app/client/cypress/e2e/Regression/ClientSide/JSLibrary/Custom_lib_spec.ts (1)
Pattern app/client/cypress/**/**.*
: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
🔇 Additional comments (1)
app/client/cypress/limited-tests.txt (1)
2-2
: LGTM
The test specification file has been correctly updated to include Custom_lib_spec.ts
for limited tests.
/ci-test-limit |
/ci-test-limit-count run_count=02 |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/12132433060. |
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12132433060.
|
/ci-test-limit-count run_count=25 |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/12138513675. |
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12138513675.
|
/ci-test-limit-count run_count=1 |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/12209617145. |
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 (3)
app/client/cypress/e2e/Regression/ClientSide/JSLibrary/Library_Test_Git_spec.ts (3)
65-65
: Specify the library name in the test descriptionIn the test description at line 65, "Verify install/uninstall of Library", consider specifying the library name (e.g., "Verify install/uninstall of Swiper library") for clarity and better readability.
68-71
: Avoid hardcoding library URLs and versionsIn lines 68-71, the Swiper library is installed via a hardcoded URL with a specific version. Consider using configuration variables or constants for the library URL and version to facilitate easier updates and maintenance.
78-114
: Refactor duplicate test code into reusable functionsThe test cases in
it("3. Verify jspdf library", ...)
(lines 78-114) andit("4. Verify deleting jspdf library deletes all its references as well", ...)
(lines 116-145) contain duplicate code for installing thejspdf
library, configuring the button click handler, and verifying PDF downloads. Consider refactoring this code into reusable helper functions to improve maintainability and reduce duplication.Also applies to: 116-145
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
app/client/cypress/e2e/Regression/ClientSide/JSLibrary/Library_Test_Git_spec.ts
(1 hunks)app/client/cypress/limited-tests.txt
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/client/cypress/limited-tests.txt
🧰 Additional context used
📓 Path-based instructions (1)
app/client/cypress/e2e/Regression/ClientSide/JSLibrary/Library_Test_Git_spec.ts (1)
Pattern app/client/cypress/**/**.*
: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
🔇 Additional comments (2)
app/client/cypress/e2e/Regression/ClientSide/JSLibrary/Library_Test_Git_spec.ts (2)
84-86
: Verify the availability of doc.table
method in jspdf
Ensure that the doc.table
method used in line 86 is available in the jspdf
library version you're using. The table
method may require an additional plugin or may not be part of the core library.
84-84
: Verify the initialization of jsPDF
In line 84, const doc = new jspdf.jsPDF();
, ensure that jspdf.jsPDF()
is the correct way to initialize the jsPDF object for the version of the library you're using. Depending on the import and version, it might be new jsPDF()
or new jspdf()
.
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12209617145.
|
/ci-test-limit-count run_count=1 |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/12239920908. |
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12239920908.
|
/ci-test-limit-count run_count=1 |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/12240560156. |
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12240560156.
|
/ci-test-limit-count run_count=1 |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/12241742459. |
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12241742459.
|
/ci-test-limit-count run_count=25 |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/12242146502. |
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12242146502.
|
Adding tests for custom library https://docs.google.com/spreadsheets/d/1Vq5vRUyAMv2npPv4j41omzZrYcRUb8LEOhNVu_O4OE8/edit?gid=1961024675#gid=1961024675 EE PR: appsmithorg/appsmith-ee#5703 /ok-to-test tags="@tag.Sanity" <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/12260773766> > Commit: 3423a83 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=12260773766&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Sanity` > Spec: > <hr>Tue, 10 Dec 2024 17:30:21 UTC <!-- end of auto-generated comment: Cypress test results --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced a comprehensive suite of end-to-end tests for managing JavaScript libraries, covering functionalities such as installation, usage, error handling, and cleanup. - **Bug Fixes** - Enhanced error handling for incompatible library URLs and ensured proper behavior upon library uninstallation and reinstallation. - **Chores** - Updated test execution configurations to reference the new TypeScript test specification file. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: “NandanAnantharamu” <“nandan@thinkify.io”>
Adding tests for custom library
https://docs.google.com/spreadsheets/d/1Vq5vRUyAMv2npPv4j41omzZrYcRUb8LEOhNVu_O4OE8/edit?gid=1961024675#gid=1961024675
EE PR: https://github.com/appsmithorg/appsmith-ee/pull/5703
/ok-to-test tags="@tag.Sanity"
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12260773766
Commit: 3423a83
Cypress dashboard.
Tags:
@tag.Sanity
Spec:
Tue, 10 Dec 2024 17:30:21 UTC
Summary by CodeRabbit
New Features
Bug Fixes
Chores