e2e: Add test for testing wallet name editing#459
e2e: Add test for testing wallet name editing#459Abhay349 wants to merge 1 commit intocaravan-bitcoin:mainfrom
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Legend101Zz
left a comment
Legend101Zz
left a comment
There was a problem hiding this comment.
Thanks for the PR , overall looks good , just a couple of reviews
| await nameInput.fill(newName); | ||
|
|
||
| // Save change | ||
| await page.locator('[data-cy="save-button"]').click(); |
There was a problem hiding this comment.
question here , will this create a race condition ? like after clicking save, there's no guarantee the save operation has completed right or does playwright handle this ?
There was a problem hiding this comment.
I agree, there is a chance of a race condition if we don’t wait for the UI to update. While playwright ensures that the click itself is safe, it doesn’t automatically guarantee that the underlying save operation has completed.
To make this more reliable, I’ll update the test to explicitly wait for the updated wallet name to appear and for the edit mode to close. This way, the test only proceeds once the save is actually reflected in the UI.
|
Hi @Legend101Zz I have updated the test to address the race condition and kept it focused on validating the user-visible behavior only. The test now checks that the wallet name updates correctly in the UI and that the UI exits edit mode after saving. |
|
@Legend101Zz ~Thanks |
|
|
||
| // Wallet name should be visible (display mode) | ||
| let nameDisplay = page.locator('[data-cy="editable-name-value"]').first(); | ||
| await expect(nameDisplay).toBeVisible({ timeout: 15000 }); |
There was a problem hiding this comment.
these are some really long timeouts. Do we really expect it could take up to 15 seconds for the field to become visible?
There was a problem hiding this comment.
Ok, it seems this is a sane default, but I think we could avoid having to write this every time by adding this to the main config instead. what do you think?
There was a problem hiding this comment.
@bucko13 Thanks for the review and feedback, that makes sense. I have increased the timeout to 15s to reduce potential flakiness, taking cues from some existing tests. That said, I agree it’s better not to repeat this inline. I’ll move it to the main Playwright config.
There was a problem hiding this comment.
And I'm happy to modify existing tests based on feedback.
Thanks :)
| throw new Error(`Error in wallet name editing: ${error}`); | ||
| } | ||
| }); | ||
| }); No newline at end of file |
Signed-off-by: Abhay349 <pandeyabhay967@gmail.com>
3 participants
What kind of change does this PR introduce?
Issue Number:
Fixes #442
If relevant, did you update the documentation?
Summary
This PR adds an end-to-end test to cover the wallet name editing functionality in the Caravan coordinator.
Previously, editing and saving a wallet name was not covered by E2E tests. This change ensures that:
The test follows the existing E2E test structure and error-handling conventions and runs as part of the full coordinator E2E suite.
Does this PR introduce a breaking change?
Checklist
npm run changeset).Other information
data-cyattributes.Have you read the contributing guide?