[e2e] Account page tests - wait for transaction table to be visible #3530
Conversation
actual-github-bot
bot
changed the title
WalkthroughThe changes in this pull request involve updates to the onboarding process tests and the associated functionality in the configuration page. The test case in Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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
CodeRabbit Configuration File (
|
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
joel-jeremy
changed the title
Bundle Stats — desktop-clientHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset No files were changed View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger No assets were bigger Smaller No assets were smaller Unchanged
|
Bundle Stats — loot-coreHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset No files were changed View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger No assets were bigger Smaller No assets were smaller Unchanged
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
packages/desktop-client/e2e/page-models/configuration-page.js (2)
22-23: LGTM: Method updated to return AccountPage. Consider removing blank line.The
startFreshmethod has been appropriately updated to return a newAccountPageinstance, which aligns with its purpose and improves the e2e test flow. This change is consistent with other methods in the class that return page objects.Consider removing the blank line (line 22) before the return statement for consistency with other methods in the class:
async startFresh() { await this.page.getByRole('button', { name: 'Start fresh' }).click(); - return new AccountPage(this.page); }
Action Required: Capture and utilize the returned
AccountPageinstance fromstartFreshin all relevant tests.
File:
packages/desktop-client/e2e/onboarding.test.js
- Test:
creates a new empty budget file
- Issue: Calls
startFreshwithout capturing its return value.- Action: Assign the returned
AccountPageinstance to a variable and use it as needed within the test.const accountPage = await configurationPage.startFresh(); // Add assertions or interactions using accountPageEnsure all tests invoking
startFreshhandle the returnedAccountPageinstance appropriately to maintain test integrity and leverage the updated functionality.🔗 Analysis chain
Line range hint
1-23: Verify impact on existing tests using startFresh method.The changes to the
ConfigurationPageclass, particularly thestartFreshmethod, improve its functionality and allow for a more seamless flow in e2e tests. These modifications are consistent with the existing code structure and patterns.Please ensure that all existing tests using the
startFreshmethod are updated to handle the new return value (AccountPage instance). Run the following script to identify potential areas that may need updates:Review the results and update the relevant test files to properly utilize the returned AccountPage instance.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find usages of startFresh method that might need updating # Search for startFresh method calls in test files echo "Searching for startFresh method calls in test files:" rg --type js "startFresh\(\)" packages/desktop-client/e2e/tests -C 2 # Search for startFresh method calls in other files (excluding the configuration-page.js) echo "Searching for startFresh method calls in other files:" rg --type js "startFresh\(\)" packages/desktop-client/e2e -g "!configuration-page.js" -C 2Length of output: 1411
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- packages/desktop-client/e2e/onboarding.test.js (1 hunks)
- packages/desktop-client/e2e/page-models/configuration-page.js (2 hunks)
🔇 Additional comments (2)
packages/desktop-client/e2e/page-models/configuration-page.js (1)
1-1: LGTM: Import statement added correctly.The import statement for
AccountPagehas been added appropriately, following the existing import pattern. This import is necessary for the changes made to thestartFreshmethod.packages/desktop-client/e2e/onboarding.test.js (1)
96-98: Excellent addition to ensure UI readiness!These changes effectively implement the PR objective of waiting for the transaction table to be visible. By assigning the result of
startFresh()toaccountPageand adding an expectation for the transaction table's visibility, the test now ensures that the UI is in the correct state before proceeding. This enhancement improves the test's reliability and aligns perfectly with the PR's goal.
joel-jeremy
force-pushed
the
vrt-wait-for-transactiontable-to-be-visible
branch
from
ea5389b to
a0fabf3
Compare
joel-jeremy
force-pushed
the
vrt-wait-for-transactiontable-to-be-visible
branch
from
b7a1e4f to
d19cb93
Compare
Fix the flaky
navigates back to start page by clicking on “no server” in an empty budget filetest from onboarding.test.js