From d4633ba4780110cacc05dffb4b9aff018f606ea4 Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Tue, 9 Apr 2024 15:59:23 +0200 Subject: [PATCH] Implement a linting rule to discourage using `waitForTimeout` for new tests The `waitForTimeout` function should not be used anymore and only exists for old usages that have to be rewritten, but there was nothing in place to signal this. This commit therefore implements a linting rule, specific to the integration tests, to make it clear that this function should no longer be used. We exclude the old usages from it because we are already tracking those in #17656 (so this patch is mostly to not make the scope of that issue bigger). --- test/integration/.eslintrc | 14 ++++++++++++++ test/integration/annotation_spec.mjs | 8 ++++++++ test/integration/freetext_editor_spec.mjs | 6 ++++++ test/integration/scripting_spec.mjs | 20 ++++++++++++++++++++ test/integration/test_utils.mjs | 2 ++ 5 files changed, 50 insertions(+) create mode 100644 test/integration/.eslintrc diff --git a/test/integration/.eslintrc b/test/integration/.eslintrc new file mode 100644 index 000000000..8aedbe37f --- /dev/null +++ b/test/integration/.eslintrc @@ -0,0 +1,14 @@ +{ + "extends": [ + "../.eslintrc" + ], + + "rules": { + "no-restricted-syntax": ["error", + { + "selector": "CallExpression[callee.name='waitForTimeout']", + "message": "`waitForTimeout` can cause intermittent failures and should not be used (see issue #17656 for replacements).", + }, + ], + }, +} diff --git a/test/integration/annotation_spec.mjs b/test/integration/annotation_spec.mjs index c03f952c2..3ade40082 100644 --- a/test/integration/annotation_spec.mjs +++ b/test/integration/annotation_spec.mjs @@ -170,6 +170,7 @@ describe("Checkbox annotation", () => { ); for (const selector of selectors) { await page.click(selector); + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(10); } for (const selector of selectors) { @@ -229,6 +230,7 @@ describe("Text widget", () => { pages.map(async ([browserName, page]) => { await page.type(getSelector("22R"), "a"); await page.keyboard.press("Tab"); + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(10); const text = await page.$eval(getSelector("22R"), el => el.value); @@ -515,11 +517,13 @@ describe("ResetForm action", () => { `document.querySelector("[data-annotation-id='25R']").hidden === false` ); await page.click("#editorFreeText"); + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(10); await page.waitForFunction( `document.querySelector("[data-annotation-id='25R']").hidden === true` ); await page.click("#editorFreeText"); + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(10); await page.waitForFunction( `document.querySelector("[data-annotation-id='25R']").hidden === false` @@ -583,6 +587,7 @@ describe("ResetForm action", () => { expect(hidden).withContext(`In ${browserName}`).toEqual(true); await page.focus("[data-annotation-id='20R']"); await page.keyboard.press("Enter"); + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(10); hidden = await page.$eval( "[data-annotation-id='21R']", @@ -591,6 +596,7 @@ describe("ResetForm action", () => { expect(hidden).withContext(`In ${browserName}`).toEqual(false); await page.keyboard.press("Enter"); + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(10); hidden = await page.$eval( "[data-annotation-id='21R']", @@ -599,6 +605,7 @@ describe("ResetForm action", () => { expect(hidden).withContext(`In ${browserName}`).toEqual(true); await page.keyboard.press("Enter"); + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(10); hidden = await page.$eval( "[data-annotation-id='21R']", @@ -607,6 +614,7 @@ describe("ResetForm action", () => { expect(hidden).withContext(`In ${browserName}`).toEqual(false); await page.keyboard.press("Escape"); + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(10); hidden = await page.$eval( "[data-annotation-id='21R']", diff --git a/test/integration/freetext_editor_spec.mjs b/test/integration/freetext_editor_spec.mjs index 4aa5d9e76..c7aa0e3c4 100644 --- a/test/integration/freetext_editor_spec.mjs +++ b/test/integration/freetext_editor_spec.mjs @@ -56,6 +56,7 @@ const copyPaste = async page => { await kbCopy(page); await promise; + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(10); promise = waitForEvent(page, "paste"); @@ -1147,6 +1148,7 @@ describe("FreeText Editor", () => { await kbUndo(page); // Nothing should happen, it's why we can't wait for something // specific! + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(200); // We check that the editor hasn't been removed. @@ -1353,6 +1355,7 @@ describe("FreeText Editor", () => { // Enter in editing mode. await switchToFreeText(page); + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(200); // Disable editing mode. @@ -2399,6 +2402,7 @@ describe("FreeText Editor", () => { // The editor must be moved in the DOM and potentially the focus // will be lost, hence there's a callback will get back the focus. + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(200); const focused = await page.evaluate(sel => { @@ -3656,6 +3660,7 @@ describe("FreeText Editor", () => { ); // Nothing should change, so it's hard to wait on something. + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(100); text = await getText(editorSelector); @@ -3677,6 +3682,7 @@ describe("FreeText Editor", () => { ); // Nothing should change, so it's hard to wait on something. + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(100); const html = await getHTML(); diff --git a/test/integration/scripting_spec.mjs b/test/integration/scripting_spec.mjs index 5f29be1ef..5896e7f59 100644 --- a/test/integration/scripting_spec.mjs +++ b/test/integration/scripting_spec.mjs @@ -1712,6 +1712,7 @@ describe("Interaction", () => { await clearInput(page, getSelector("27R")); await page.type(getSelector("27R"), exportValue); await page.click("[data-annotation-id='28R']"); + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(10); value = await page.$eval(getSelector("24R"), el => el.value); @@ -1759,6 +1760,7 @@ describe("Interaction", () => { await page.waitForFunction( `${getQuerySelector("30R")}.value !== "abc"` ); + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(100); const focusedId = await page.evaluate(_ => @@ -1855,6 +1857,7 @@ describe("Interaction", () => { expect(text).withContext(`In ${browserName}`).toEqual("00000000123"); await page.click(getSelector("26R")); + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(10); text = await page.$eval(getSelector("25R"), el => el.value); @@ -1889,12 +1892,14 @@ describe("Interaction", () => { expect(text).withContext(`In ${browserName}`).toEqual("5,25"); await page.click(getSelector("22R")); + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(10); text = await page.$eval(getSelector("22R"), el => el.value); expect(text).withContext(`In ${browserName}`).toEqual("5,25"); await page.click(getSelector("31R")); + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(10); text = await page.$eval(getSelector("31R"), el => el.value); @@ -1926,6 +1931,7 @@ describe("Interaction", () => { expect(text).withContext(`In ${browserName}`).toEqual(""); await page.select(getSelector("6R"), "Yes"); + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(10); text = await page.$eval(getSelector("44R"), el => el.value); @@ -1934,6 +1940,7 @@ describe("Interaction", () => { await clearInput(page, getSelector("44R")); await page.select(getSelector("6R"), "No"); + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(10); text = await page.$eval(getSelector("44R"), el => el.value); @@ -1991,6 +1998,7 @@ describe("Interaction", () => { await page.type(getSelector("26R"), "abcde", { delay: 10 }); await page.click(getSelector("23R")); + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(10); await page.click(getSelector("26R")); @@ -1998,6 +2006,7 @@ describe("Interaction", () => { await page.keyboard.press("Backspace"); await page.click(getSelector("23R")); + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(10); text = await page.$eval(getSelector("26R"), el => el.value); @@ -2094,6 +2103,7 @@ describe("Interaction", () => { expect(visibility).withContext(`In ${browserName}`).toEqual("hidden"); await page.click(getSelector("11R")); + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(10); visibility = await page.$eval( @@ -2140,27 +2150,32 @@ describe("Interaction", () => { ); expect(readonly).withContext(`In ${browserName}`).toEqual(true); await page.click(getSelector("334R")); + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(10); readonly = await page.$eval(getSelector("353R"), el => el.disabled); expect(readonly).withContext(`In ${browserName}`).toEqual(true); await page.click(getSelector("351R")); + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(10); readonly = await page.$eval(getSelector("353R"), el => el.disabled); expect(readonly).withContext(`In ${browserName}`).toEqual(true); await page.click(getSelector("352R")); + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(10); readonly = await page.$eval(getSelector("353R"), el => el.disabled); expect(readonly).withContext(`In ${browserName}`).toEqual(false); await page.click(getSelector("353R")); + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(10); let checked = await page.$eval(getSelector("353R"), el => el.checked); expect(checked).withContext(`In ${browserName}`).toEqual(true); await page.click(getSelector("334R")); + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(10); readonly = await page.$eval(getSelector("353R"), el => el.disabled); @@ -2200,15 +2215,19 @@ describe("Interaction", () => { await page.click(getSelector("55R")); await page.type(getSelector("55R"), "Hello", { delay: 10 }); await page.click(getSelector("56R")); + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(10); await page.click(getSelector("55R")); await page.type(getSelector("55R"), " World", { delay: 10 }); + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(10); await otherPages[i].bringToFront(); + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(100); await page.bringToFront(); + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(100); const text = await page.$eval(getSelector("55R"), el => el.value); @@ -2244,6 +2263,7 @@ describe("Interaction", () => { ); await page.click(getSelector("25R")); + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(10); await page.click(getSelector("26R")); diff --git a/test/integration/test_utils.mjs b/test/integration/test_utils.mjs index 34718bb42..7d1a08c96 100644 --- a/test/integration/test_utils.mjs +++ b/test/integration/test_utils.mjs @@ -105,6 +105,7 @@ async function clearInput(page, selector) { await page.click(selector); await kbSelectAll(page); await page.keyboard.press("Backspace"); + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(10); } @@ -343,6 +344,7 @@ async function serializeBitmapDimensions(page) { async function dragAndDropAnnotation(page, startX, startY, tX, tY) { await page.mouse.move(startX, startY); await page.mouse.down(); + // eslint-disable-next-line no-restricted-syntax await waitForTimeout(10); await page.mouse.move(startX + tX, startY + tY); await page.mouse.up();