-
Notifications
You must be signed in to change notification settings - Fork 4.2k
test: updated flow for GitSync #34239
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
Conversation
WalkthroughThe recent changes primarily focus on enhancing the functionality and reliability of the Changes
Sequence Diagram(s)sequenceDiagram
participant TestRunner as Cypress Test Runner
participant GitSync as GitSync Class
participant Network as Network Status
TestRunner->>GitSync: Call mergeBranch()
GitSync->>GitSync: Wait until _mergeBranchDropdownmenu appears
GitSync->>Network: Assert network status
GitSync->>GitSync: Verify _mergeBranchDropdownmenu disappearance
GitSync->>GitSync: Verify destination branch presence
GitSync-->>TestRunner: Return result
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 (
|
/ci-test-limit |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/9511001047. |
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 (5)
app/client/cypress/support/Pages/GitSync.ts (5)
Line range hint
1-1
: Consider removing the commented outGITEA_API_BASE
if it is no longer needed. Keeping unused code can lead to confusion and clutter in the codebase.
Line range hint
175-183
: This block of code is responsible for creating and connecting to a Git repository. It's important to handle potential errors in network requests and provide feedback to the user or system if something goes wrong.Consider adding error handling for the network requests in the
CreateNConnectToGit
method to improve robustness and user experience.
Line range hint
200-220
: TheImportAppFromGit
method appears to be handling keys and making network requests without error handling. This could lead to unhandled exceptions if the network requests fail.Add error handling for the network requests in the
ImportAppFromGit
method to ensure stability and provide clear feedback during failures.
Line range hint
344-367
: The methodSwitchGitBranch
uses a hardcoded delay which can lead to brittle tests if the expected state is not reached within the timeout. Consider using dynamic waits or polling to check for the state.Refactor the hardcoded delay in
SwitchGitBranch
to use dynamic waits or polling mechanisms to enhance test reliability.
Line range hint
370-370
: The methodCheckMergeConflicts
uses a direct call toAssertElementAbsence
which might not handle cases where the element takes longer than expected to disappear. Consider adding a timeout parameter to this assertion for better reliability.Refactor
AssertElementAbsence
inCheckMergeConflicts
to accept a timeout parameter, allowing for more flexible and reliable checks.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- app/client/cypress/support/Pages/GitSync.ts (1 hunks)
Additional comments not posted (1)
app/client/cypress/support/Pages/GitSync.ts (1)
390-390
: The change from_dropdownmenu
to_mergeBranchDropdownmenu
in the methodGetNClickByContains
appears to be aligned with the PR objectives. Ensure that this new locator is correctly functioning in all scenarios where it is used.
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9511001047. |
/ci-test-limit runId=9511001047 |
12 similar comments
/ci-test-limit runId=9511001047 |
/ci-test-limit runId=9511001047 |
/ci-test-limit runId=9511001047 |
/ci-test-limit runId=9511001047 |
/ci-test-limit runId=9511001047 |
/ci-test-limit runId=9511001047 |
/ci-test-limit runId=9511001047 |
/ci-test-limit runId=9511001047 |
/ci-test-limit runId=9511001047 |
/ci-test-limit runId=9511001047 |
/ci-test-limit runId=9511001047 |
/ci-test-limit runId=9511001047 |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/9511202819. |
/ci-test-limit runId=9511001047 |
2 similar comments
/ci-test-limit runId=9511001047 |
/ci-test-limit runId=9511001047 |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/9511203611. |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/9511203907. |
/ci-test-limit runId=9511001047 |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/9511204235. |
/ci-test-limit runId=9641162211 |
1 similar comment
/ci-test-limit runId=9641162211 |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/9641881583. |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/9641883412. |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/9641884450. |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/9641885341. |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/9641885076. |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/9641885360. |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/9641886002. |
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9641883412. |
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9641885076. |
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9641886002. |
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9641885360. |
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9641884450. |
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/support/Pages/GitSync.ts (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- app/client/cypress/support/Pages/GitSync.ts
RCA: Click on the dropdown was not working intermittently Solution: updated the flow, 1. Wait for the destination dropdown 2. Click on the destination dropdown 3. Assert the branch name visible post dropdown has opened 4. Click on the destination branch <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Enhanced the `mergeBranch` process in GitSync to ensure better stability and accuracy by adding assertions for element appearances, disappearances, network status, and the presence of a destination branch. - **Tests** - Switched limited test spec from `Fork_Template_spec.js` to `GitSyncedApps_spec.js` and added `GitBugs_Spec.ts` for improved test coverage and accuracy. - **Refactor** - Changed `className` attribute to `data-testid` in the `Select` component within the GitSync merge tab for improved testing and readability. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: albinAppsmith <87797149+albinAppsmith@users.noreply.github.com>
RCA:
Click on the dropdown was not working intermittently
Solution:
updated the flow,
Summary by CodeRabbit
Bug Fixes
mergeBranch
process in GitSync to ensure better stability and accuracy by adding assertions for element appearances, disappearances, network status, and the presence of a destination branch.Tests
Fork_Template_spec.js
toGitSyncedApps_spec.js
and addedGitBugs_Spec.ts
for improved test coverage and accuracy.Refactor
className
attribute todata-testid
in theSelect
component within the GitSync merge tab for improved testing and readability.