From 0b619025208ec0f6b00bddc1b6894bd0f115e6c2 Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Fri, 21 Jun 2024 20:37:01 +0200 Subject: [PATCH] Refactor the copy/paste logic in the integration tests The integration tests are currently not consistent in how they do copy/pasting: some tests use the `kbCopy`/`kbPaste` functions with waiting for the event inline, some have their own helper function to combine those actions and some even call `kbCopy`/`kbPaste` without waiting for the event at all (which can cause intermittent failures). This commit fixes the issues by providing a set of four helper functions that all tests use and that abstract e.g. waiting for the event away from the caller. This makes the invididual tests simpler and consistent, reduces code duplication and fixes possible intermittent failures due to not waiting for events to trigger. --- test/integration/copy_paste_spec.mjs | 12 +-- test/integration/freetext_editor_spec.mjs | 100 ++++++++-------------- test/integration/stamp_editor_spec.mjs | 25 +++--- test/integration/test_utils.mjs | 23 ++++- 4 files changed, 72 insertions(+), 88 deletions(-) diff --git a/test/integration/copy_paste_spec.mjs b/test/integration/copy_paste_spec.mjs index 6c18929d913a00..94d6be9a76fd75 100644 --- a/test/integration/copy_paste_spec.mjs +++ b/test/integration/copy_paste_spec.mjs @@ -15,7 +15,7 @@ import { closePages, - kbCopy, + copy, kbSelectAll, loadAndWait, mockClipboard, @@ -55,10 +55,7 @@ describe("Copy and paste", () => { ); await selectAll(page); - const promise = waitForEvent(page, "copy"); - await kbCopy(page); - await promise; - + await copy(page); await page.waitForFunction( `document.querySelector('#viewerContainer').style.cursor !== "wait"` ); @@ -159,10 +156,7 @@ describe("Copy and paste", () => { ); await selectAll(page); - const promise = waitForEvent(page, "copy"); - await kbCopy(page); - await promise; - + await copy(page); await page.waitForFunction( `document.querySelector('#viewerContainer').style.cursor !== "wait"` ); diff --git a/test/integration/freetext_editor_spec.mjs b/test/integration/freetext_editor_spec.mjs index d7afe96ab52c4c..a73d9b5c219d17 100644 --- a/test/integration/freetext_editor_spec.mjs +++ b/test/integration/freetext_editor_spec.mjs @@ -16,6 +16,8 @@ import { awaitPromise, closePages, + copy, + copyData, createPromise, dragAndDropAnnotation, firstPageOnTop, @@ -30,21 +32,19 @@ import { kbBigMoveLeft, kbBigMoveRight, kbBigMoveUp, - kbCopy, kbGoToBegin, kbGoToEnd, kbModifierDown, kbModifierUp, - kbPaste, kbRedo, kbSelectAll, kbUndo, loadAndWait, - pasteFromClipboard, + paste, + pasteData, scrollIntoView, switchToEditor, waitForAnnotationEditorLayer, - waitForEvent, waitForSelectedEditor, waitForSerialized, waitForStorageEntries, @@ -53,16 +53,6 @@ import { } from "./test_utils.mjs"; import { PNG } from "pngjs"; -const copyPaste = async page => { - let promise = waitForEvent(page, "copy"); - await kbCopy(page); - await promise; - - promise = waitForEvent(page, "paste"); - await kbPaste(page); - await promise; -}; - const selectAll = async page => { await kbSelectAll(page); await page.waitForFunction( @@ -187,7 +177,8 @@ describe("FreeText Editor", () => { ); await waitForSelectedEditor(page, getEditorSelector(0)); - await copyPaste(page); + await copy(page); + await paste(page); await page.waitForSelector(getEditorSelector(1), { visible: true, }); @@ -203,7 +194,8 @@ describe("FreeText Editor", () => { expect(pastedContent).withContext(`In ${browserName}`).toEqual(content); - await copyPaste(page); + await copy(page); + await paste(page); await page.waitForSelector(getEditorSelector(2), { visible: true, }); @@ -263,7 +255,8 @@ describe("FreeText Editor", () => { ); await waitForSelectedEditor(page, getEditorSelector(3)); - await copyPaste(page); + await copy(page); + await paste(page); await page.waitForSelector(getEditorSelector(4), { visible: true, }); @@ -276,9 +269,7 @@ describe("FreeText Editor", () => { ); for (let i = 0; i < 2; i++) { - const promise = waitForEvent(page, "paste"); - await kbPaste(page); - await promise; + await paste(page); await page.waitForSelector(getEditorSelector(5 + i)); } @@ -597,7 +588,8 @@ describe("FreeText Editor", () => { .withContext(`In ${browserName}`) .toEqual([0, 1, 3]); - await copyPaste(page); + await copy(page); + await paste(page); await page.waitForSelector(getEditorSelector(6), { visible: true, }); @@ -1275,7 +1267,8 @@ describe("FreeText Editor", () => { ); await waitForSelectedEditor(page, getEditorSelector(1)); - await copyPaste(page); + await copy(page); + await paste(page); await page.waitForSelector(getEditorSelector(6), { visible: true, }); @@ -3425,14 +3418,11 @@ describe("FreeText Editor", () => { ); await select(0); - await pasteFromClipboard( - page, - { - "text/html": "Bold Foo", - "text/plain": "Foo", - }, - `${editorSelector} .internal` - ); + await copyData(page, { + "text/html": "Bold Foo", + "text/plain": "Foo", + }); + await pasteData(page, `${editorSelector} .internal`); let lastText = data; @@ -3442,14 +3432,11 @@ describe("FreeText Editor", () => { expect(text).withContext(`In ${browserName}`).toEqual(lastText); await select(3); - await pasteFromClipboard( - page, - { - "text/html": "Bold Bar
Oof", - "text/plain": "Bar\nOof", - }, - `${editorSelector} .internal` - ); + await copyData(page, { + "text/html": "Bold Bar
Oof", + "text/plain": "Bar\nOof", + }); + await pasteData(page, `${editorSelector} .internal`); await waitForTextChange(lastText, editorSelector); text = await getText(editorSelector); @@ -3457,13 +3444,8 @@ describe("FreeText Editor", () => { expect(text).withContext(`In ${browserName}`).toEqual(lastText); await select(0); - await pasteFromClipboard( - page, - { - "text/html": "basic html", - }, - `${editorSelector} .internal` - ); + await copyData(page, { "text/html": "basic html" }); + await pasteData(page, `${editorSelector} .internal`); // Nothing should change, so it's hard to wait on something. // eslint-disable-next-line no-restricted-syntax @@ -3477,15 +3459,12 @@ describe("FreeText Editor", () => { const prevHTML = await getHTML(); // Try to paste an image. - await pasteFromClipboard( - page, - { - "image/png": - // 1x1 transparent png. - "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==", - }, - `${editorSelector} .internal` - ); + await copyData(page, { + "image/png": + // 1x1 transparent png. + "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==", + }); + await pasteData(page, `${editorSelector} .internal`); // Nothing should change, so it's hard to wait on something. // eslint-disable-next-line no-restricted-syntax @@ -3505,14 +3484,11 @@ describe("FreeText Editor", () => { }); const fooBar = "Foo\nBar\nOof"; - await pasteFromClipboard( - page, - { - "text/html": "html", - "text/plain": fooBar, - }, - `${editorSelector} .internal` - ); + await copyData(page, { + "text/html": "html", + "text/plain": fooBar, + }); + await pasteData(page, `${editorSelector} .internal`); await waitForTextChange("", editorSelector); text = await getText(editorSelector); diff --git a/test/integration/stamp_editor_spec.mjs b/test/integration/stamp_editor_spec.mjs index 8856f5b26c8aed..178039ec6bc15b 100644 --- a/test/integration/stamp_editor_spec.mjs +++ b/test/integration/stamp_editor_spec.mjs @@ -17,6 +17,8 @@ import { applyFunctionToEditor, awaitPromise, closePages, + copy, + copyData, getEditorDimensions, getEditorSelector, getFirstSerialized, @@ -24,12 +26,11 @@ import { getSerialized, kbBigMoveDown, kbBigMoveRight, - kbCopy, - kbPaste, kbSelectAll, kbUndo, loadAndWait, - pasteFromClipboard, + paste, + pasteData, scrollIntoView, serializeBitmapDimensions, switchToEditor, @@ -78,12 +79,10 @@ const copyImage = async (page, imagePath, number) => { const data = fs .readFileSync(path.join(__dirname, imagePath)) .toString("base64"); - await pasteFromClipboard( - page, - { "image/png": `data:image/png;base64,${data}` }, - "", - 500 - ); + + await copyData(page, { "image/png": `data:image/png;base64,${data}` }); + await pasteData(page); + await waitForImage(page, getEditorSelector(number)); }; @@ -574,13 +573,13 @@ describe("Stamp Editor", () => { await page1.click("#editorStamp"); await copyImage(page1, "../images/firefox_logo.png", 0); - await kbCopy(page1); + await copy(page1); const [, page2] = pages2[i]; await page2.bringToFront(); await page2.click("#editorStamp"); - await kbPaste(page2); + await paste(page2); await waitForImage(page2, getEditorSelector(0)); } @@ -835,8 +834,8 @@ describe("Stamp Editor", () => { ); await page.waitForSelector(`${getEditorSelector(0)} .altText.done`); - await kbCopy(page); - await kbPaste(page); + await copy(page); + await paste(page); await page.waitForSelector(`${getEditorSelector(1)} .altText.done`); await waitForSerialized(page, 2); diff --git a/test/integration/test_utils.mjs b/test/integration/test_utils.mjs index 39fe88bd4c1e3c..4fd4e9e35db844 100644 --- a/test/integration/test_utils.mjs +++ b/test/integration/test_utils.mjs @@ -292,7 +292,13 @@ async function mockClipboard(pages) { ); } -async function pasteFromClipboard(page, data, selector, timeout = 100) { +async function copy(page) { + const promise = waitForEvent(page, "copy"); + await kbCopy(page); + await promise; +} + +async function copyData(page, data) { await page.evaluate(async dat => { const items = Object.create(null); for (const [type, value] of Object.entries(dat)) { @@ -305,7 +311,15 @@ async function pasteFromClipboard(page, data, selector, timeout = 100) { } await navigator.clipboard.write([new ClipboardItem(items)]); }, data); +} + +async function paste(page) { + const promise = waitForEvent(page, "paste"); + await kbPaste(page); + await promise; +} +async function pasteData(page, selector = null) { const validator = e => e.clipboardData.items.length !== 0; let hasPasteEvent = false; while (!hasPasteEvent) { @@ -634,6 +648,8 @@ export { clearInput, closePages, closeSinglePage, + copy, + copyData, createPromise, dragAndDropAnnotation, firstPageOnTop, @@ -654,7 +670,6 @@ export { kbBigMoveLeft, kbBigMoveRight, kbBigMoveUp, - kbCopy, kbDeleteLastWord, kbFocusNext, kbFocusPrevious, @@ -662,13 +677,13 @@ export { kbGoToEnd, kbModifierDown, kbModifierUp, - kbPaste, kbRedo, kbSelectAll, kbUndo, loadAndWait, mockClipboard, - pasteFromClipboard, + paste, + pasteData, scrollIntoView, serializeBitmapDimensions, switchToEditor,