Skip to content
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:PartialExport_Widgets spec file updated and import app method updated with validations #33343

Merged
merged 15 commits into from
May 10, 2024

Conversation

NandanAnantharamu
Copy link
Collaborator

@NandanAnantharamu NandanAnantharamu commented May 10, 2024

RCA:
PartialExport_Widgets_spec.ts was failing where we export and compare files.

Solution:

  • Updated the WidgetsExportedOnly.json, now the comparison step is working as expected
  • Improved the assertions within importApp() reduced wait time and waiting for import modal to close as a check.
  • Also updated tags which used importApp() below are the list of files [AppNavigation_spec.ts,Sidebar_spec.ts,TopStacked_spec.ts,TopInline_spec.ts,Editor_Segment_Context_Switching_spec.ts,JSOnLoad2_Spec.ts]
  • Reverted the intercept success check within ImportApp()
  • Updated PartialExport_Widgets_spec.ts with network call assertion post import

/ok-to-test tags="@tag.Workspace,@tag.ImportExport"

Tip

🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9028944129
Commit: b9ad3c9
Cypress dashboard url: Click here!

Copy link
Contributor

coderabbitai bot commented May 10, 2024

Walkthrough

Walkthrough

The updates primarily focus on standardizing the use of message constants across various Cypress test suites and support files. This involves importing and utilizing createMessage with constants like IMPORT_APP_SUCCESSFUL for consistent toast message verification. Additionally, there are enhancements in test categorization through the addition of tags, and minor adjustments in test structure and assertions to improve clarity and maintainability.

Changes

File Path Change Summary
.../ImportExportForkApplication_spec.js, .../Widgets/Migration_Spec.js, .../ReconnectDatasource_spec.js, .../gitSync.js, .../HomePage.ts, .../GitSync.ts Replaced static strings with createMessage(IMPORT_APP_SUCCESSFUL) for toast message verification.
.../AppNavigation_spec.ts, .../TopInline_spec.ts, .../TopStacked_spec.ts, .../JSOnLoad2_Spec.ts, .../Editor_Segment_Context_Switching_spec.ts Added @tag.ImportExport to test descriptions for enhanced categorization.
.../Sidebar_spec.ts Minor formatting and tagging adjustments; no logic change.
.../PartialExport_Widgets_spec.ts Added new assertion within the before() hook.
.../messages.ts, .../ApplicationSagas.tsx, .../GitSyncSagas.ts Introduced new constants for application import messages and updated related functions.

Recent Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between abbf326 and b9ad3c9.
Files ignored due to path filters (1)
  • app/client/cypress/fixtures/PartialImportExport/WidgetsExportedOnly.json is excluded by !**/*.json
Files selected for processing (17)
  • app/client/cypress/e2e/Regression/Apps/ImportExportForkApplication_spec.js (3 hunks)
  • app/client/cypress/e2e/Regression/ClientSide/AppNavigation/AppNavigation_spec.ts (1 hunks)
  • app/client/cypress/e2e/Regression/ClientSide/AppNavigation/Sidebar_spec.ts (1 hunks)
  • app/client/cypress/e2e/Regression/ClientSide/AppNavigation/TopInline_spec.ts (1 hunks)
  • app/client/cypress/e2e/Regression/ClientSide/AppNavigation/TopStacked_spec.ts (1 hunks)
  • app/client/cypress/e2e/Regression/ClientSide/Git/GitImport/GitImport_spec.js (2 hunks)
  • app/client/cypress/e2e/Regression/ClientSide/IDE/Editor_Segment_Context_Switching_spec.ts (1 hunks)
  • app/client/cypress/e2e/Regression/ClientSide/PartialImportExport/PartialExport_Widgets_spec.ts (2 hunks)
  • app/client/cypress/e2e/Regression/ClientSide/Widgets/Migration_Spec.js (2 hunks)
  • app/client/cypress/e2e/Regression/ServerSide/OnLoadTests/JSOnLoad2_Spec.ts (1 hunks)
  • app/client/cypress/e2e/Smoke/Apps/ReconnectDatasource_spec.js (2 hunks)
  • app/client/cypress/support/Pages/GitSync.ts (2 hunks)
  • app/client/cypress/support/Pages/HomePage.ts (3 hunks)
  • app/client/cypress/support/gitSync.js (2 hunks)
  • app/client/src/ce/constants/messages.ts (1 hunks)
  • app/client/src/ce/sagas/ApplicationSagas.tsx (3 hunks)
  • app/client/src/sagas/GitSyncSagas.ts (2 hunks)
Files not reviewed due to errors (2)
  • app/client/src/ce/sagas/ApplicationSagas.tsx (no review received)
  • app/client/src/sagas/GitSyncSagas.ts (no review received)
Files skipped from review due to trivial changes (6)
  • app/client/cypress/e2e/Regression/ClientSide/AppNavigation/AppNavigation_spec.ts
  • app/client/cypress/e2e/Regression/ClientSide/AppNavigation/Sidebar_spec.ts
  • app/client/cypress/e2e/Regression/ClientSide/AppNavigation/TopInline_spec.ts
  • app/client/cypress/e2e/Regression/ClientSide/AppNavigation/TopStacked_spec.ts
  • app/client/cypress/e2e/Regression/ClientSide/Git/GitImport/GitImport_spec.js
  • app/client/cypress/e2e/Regression/ServerSide/OnLoadTests/JSOnLoad2_Spec.ts
Additional comments not posted (20)
app/client/cypress/e2e/Regression/ClientSide/PartialImportExport/PartialExport_Widgets_spec.ts (2)

5-5: Import of assertHelper added.

This import is necessary for the new assertion added in the before hook.


16-16: Addition of network status assertion.

This assertion ensures that the network status is checked before proceeding with the test, which is crucial for the reliability of network-dependent tests.

app/client/cypress/e2e/Smoke/Apps/ReconnectDatasource_spec.js (2)

7-10: Import of createMessage and IMPORT_APP_SUCCESSFUL added.

These imports are necessary for the dynamic message generation used in the toast message assertion, enhancing maintainability and localization support.


55-55: Usage of createMessage in toast message assertion.

This change replaces a hardcoded string with a dynamic message, which is a good practice for easier localization and consistency across the application.

app/client/cypress/e2e/Regression/ClientSide/IDE/Editor_Segment_Context_Switching_spec.ts (1)

14-14: Addition of @tag.ImportExport to test suite tags.

This tag categorizes the test correctly under Import/Export functionalities, aiding in better test management and selective execution during CI/CD pipelines.

app/client/cypress/e2e/Regression/Apps/ImportExportForkApplication_spec.js (2)

9-12: Import of createMessage and IMPORT_APP_SUCCESSFUL added.

These imports are necessary for the dynamic message generation used in the toast message assertion, enhancing maintainability and localization support.


43-43: Usage of createMessage in toast message assertion.

This change replaces a hardcoded string with a dynamic message, which is a good practice for easier localization and consistency across the application.

app/client/cypress/support/gitSync.js (2)

11-14: Import of createMessage and UNABLE_TO_IMPORT_APP added.

These imports are necessary for the dynamic message generation used in the custom command for discarding changes, enhancing maintainability and localization support.


237-237: Usage of createMessage in custom command for discarding changes.

This change replaces a hardcoded string with a dynamic message, which is a good practice for easier localization and consistency across the application.

app/client/cypress/e2e/Regression/ClientSide/Widgets/Migration_Spec.js (2)

13-16: Import of createMessage and IMPORT_APP_SUCCESSFUL added.

These imports are necessary for the dynamic message generation used in the toast message assertion, enhancing maintainability and localization support.


35-35: Usage of createMessage in toast message assertion.

This change replaces a hardcoded string with a dynamic message, which is a good practice for easier localization and consistency across the application.

app/client/cypress/support/Pages/GitSync.ts (1)

4-7: Import of createMessage and UNABLE_TO_IMPORT_APP added.

These imports are necessary for the dynamic message generation used in the CreateGitBranch method, enhancing maintainability and localization support.

app/client/cypress/support/Pages/HomePage.ts (2)

6-10: Ensure that the newly imported constants are used appropriately throughout the file.

Verification successful

The constants IMPORT_APP_SUCCESSFUL and UNABLE_TO_IMPORT_APP are indeed used within the file HomePage.ts. The script output confirms their usage in methods that seem to handle UI feedback related to app import operations, which aligns with the expected functionality of these constants.

  • The constant UNABLE_TO_IMPORT_APP is used in a method that appears to handle a specific toast notification for unsuccessful app imports.
  • The constant IMPORT_APP_SUCCESSFUL is used in an assertion method that likely checks for the presence of a success message in the UI.

This confirms that the constants are not only imported but are also utilized appropriately within the file, which addresses the original concern raised in the review comment.


Constants are used as expected in HomePage.ts

  • The constants IMPORT_APP_SUCCESSFUL and UNABLE_TO_IMPORT_APP are actively used in the HomePage.ts file to manage UI feedback related to app import operations.
  • Locations:
    • UNABLE_TO_IMPORT_APP is used in a toast notification method.
    • IMPORT_APP_SUCCESSFUL is used in an assertion method to check for a success message.
Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify that the constants `IMPORT_APP_SUCCESSFUL` and `UNABLE_TO_IMPORT_APP` are used in the file.

# Test: Search for the constants usage. Expect: Occurrences of the constants in relevant methods.
rg --type typescript --files-with-matches 'IMPORT_APP_SUCCESSFUL|UNABLE_TO_IMPORT_APP' app/client/cypress/support/Pages/HomePage.ts

Length of output: 168



Script:

#!/bin/bash
# Description: Re-verify that the constants `IMPORT_APP_SUCCESSFUL` and `UNABLE_TO_IMPORT_APP` are used in the file HomePage.ts.

# Test: Search for the constants usage without specifying file type. Expect: Occurrences of the constants in relevant methods.
rg 'IMPORT_APP_SUCCESSFUL|UNABLE_TO_IMPORT_APP' app/client/cypress/support/Pages/HomePage.ts

Length of output: 287


741-741: LGTM! Good use of the createMessage function to standardize toast messages.

app/client/src/ce/constants/messages.ts (6)

Line range hint 3-6: This function is well-implemented for creating formatted messages using a dynamic format function and arguments.


Line range hint 11-13: Correct use of template literals for constructing version display strings.


Line range hint 14-16: Proper implementation for returning a static consent message.


Line range hint 17-19: Simple and correct implementation for returning a static string "Yes".


Line range hint 20-22: Correctly returns a static string for confirmation dialogs.


Line range hint 23-25: Properly returns a static string used for initiating chats or support.


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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added skip-changelog Adding this label to a PR prevents it from being listed in the changelog Test labels May 10, 2024
@NandanAnantharamu NandanAnantharamu added the ok-to-test Required label for CI label May 10, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Comment on lines +645 to +649
this.agHelper.WaitUntilEleDisappear(
HomePageLocators.workspaceImportAppModal,
);
this.agHelper.AssertElementAbsence(
this.locator._specificToast("Unable to import application in workspace"),
this.locator._specificToast(createMessage(UNABLE_TO_IMPORT_APP)),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactor to improve readability by combining assertions into a single method if possible.

- this.agHelper.WaitUntilEleDisappear(HomePageLocators.workspaceImportAppModal);
- this.agHelper.AssertElementAbsence(this.locator._specificToast(createMessage(UNABLE_TO_IMPORT_APP)));
+ this.agHelper.WaitForElementAndAssertAbsence(HomePageLocators.workspaceImportAppModal, this.locator._specificToast(createMessage(UNABLE_TO_IMPORT_APP)));

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.

Suggested change
this.agHelper.WaitUntilEleDisappear(
HomePageLocators.workspaceImportAppModal,
);
this.agHelper.AssertElementAbsence(
this.locator._specificToast("Unable to import application in workspace"),
this.locator._specificToast(createMessage(UNABLE_TO_IMPORT_APP)),
this.agHelper.WaitForElementAndAssertAbsence(
HomePageLocators.workspaceImportAppModal,
this.locator._specificToast(createMessage(UNABLE_TO_IMPORT_APP)),
);

@NandanAnantharamu NandanAnantharamu changed the title test: Test/fix partially import test: import app updated with validations and added relevant tags for some spec files May 10, 2024
@NandanAnantharamu NandanAnantharamu changed the title test: import app updated with validations and added relevant tags for some spec files test:Partial import test updated and import app method updated with validations May 10, 2024
@NandanAnantharamu NandanAnantharamu changed the title test:Partial import test updated and import app method updated with validations test:PartialExport_Widgets spec file updated and import app method updated with validations May 10, 2024
@NandanAnantharamu NandanAnantharamu merged commit 7135aca into release May 10, 2024
53 of 54 checks passed
@NandanAnantharamu NandanAnantharamu deleted the test/fixPartiallyImport branch May 10, 2024 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test Required label for CI skip-changelog Adding this label to a PR prevents it from being listed in the changelog Test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants