AG-12646 - Stabilise tests. #2634
Conversation
| const point = await canvasToPageTransformer(page); | ||
| const hover = point(200, 200); | ||
| const leave = point(300, 400); | ||
|
|
||
| // Test 1. Check that the Delete & Backspace keys work: | ||
| await page.locator(SELECTORS.textAnnotationMenu).click(); | ||
| await page.locator(SELECTORS.commentMenuItem).click(); | ||
| await page.mouse.move(hover.x, hover.y); |
There was a problem hiding this comment.
Is this move necessary? There are no other moves in the steps after this which is unusual.
There was a problem hiding this comment.
Maybe not, but it reflects user behaviour better, since clicks don't just appear in the middle of the canvas without some hover events beforehand.
| @@ -155,31 +158,44 @@ test.describe('toolbar', () => { | |||
| await page.keyboard.press('Backspace'); | |||
| await page.keyboard.press('Home'); | |||
| await page.keyboard.press('Delete'); | |||
| await expect(page).toHaveScreenshot('delete-erased-text.png'); | |||
| await expect(wrapper).toHaveAttribute('data-update-pending', 'false'); | |||
| await expect(page).toHaveScreenshot('delete-erased-text.png', { animations: 'disabled' }); | |||
There was a problem hiding this comment.
Is delete-erased-text.png the only file that is causing trouble? I'm wondering whether so many data-update-pending checks are necessary 🤔
There was a problem hiding this comment.
Due to the nature of the failure, it looks like some interaction overtook a hover earlier in the flow - so waiting only before the snapshot wouldn't address this.
There was a problem hiding this comment.
✅ The updated snapshot is correct.
|
LGTM - but it would be helpful to understand why the AG-13008 test is the only one that's affected by the concurrency. Could it be that The documentation seems to suggest that they do:
|
alantreadway
force-pushed
the
ag-12646/stabilise-tests
branch
from
779dc2b to
ea7c915
Compare
There was a problem hiding this comment.
LGTM - This will make the .spec.ts files far more readable and reliable. Thanks.
|
|
||
| // Test 2. Check that Backspace key deletes the annotation when in idle state: | ||
| // (Click away from the annotation, then reclick it to go into idle state) | ||
| await page.mouse.click(leave.x, leave.y, { button: 'left' }); | ||
| await page.mouse.click(hover.x, hover.y, { button: 'left' }); | ||
| await page.keyboard.press('Backspace'); | ||
| await expect(page).toHaveScreenshot('delete-annotation-removed.png'); | ||
| await expect(page).toHaveScreenshot('delete-annotation-removed.png', { animations: 'disabled' }); |
There was a problem hiding this comment.
nit: We may want to add a toHaveScreenshot proxy to change the default to { animations: 'disable' }. This can be done in a follow-up PR though.
Addresses stability of some tests that have been highlighted as part of introducing AG-12646.