From dd46110f6b4b4c9703504765b08ddfc89d7c452f Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Sun, 17 Sep 2023 12:55:13 +0200 Subject: [PATCH 1/2] Use the `page.$eval` method in the `bug1844576` integration test We already use `page.$eval` in most other integration tests and it's simpler because it already takes the selector as argument, so we don't have to do a separate `querySelector` call ourselves. --- test/integration/scripting_spec.js | 42 ++++++++++++------------------ 1 file changed, 16 insertions(+), 26 deletions(-) diff --git a/test/integration/scripting_spec.js b/test/integration/scripting_spec.js index 7680ec9de..98f12a6b9 100644 --- a/test/integration/scripting_spec.js +++ b/test/integration/scripting_spec.js @@ -1996,45 +1996,35 @@ describe("Interaction", () => { it("must check that a field has the correct formatted value", async () => { await Promise.all( pages.map(async ([browserName, page]) => { - const hasVisibleCanvas = await page.evaluate(_ => { - const elem = document.querySelector( - `[data-annotation-id="9R"] > canvas` - ); - return elem && !elem.hasAttribute("hidden"); - }); + const hasVisibleCanvas = await page.$eval( + `[data-annotation-id="9R"] > canvas`, + elem => elem && !elem.hasAttribute("hidden") + ); expect(hasVisibleCanvas) .withContext(`In ${browserName}`) .toEqual(true); - const hasHiddenInput = await page.evaluate(_ => { - const elem = document.querySelector( - `[data-annotation-id="9R"] > input` - ); - return elem?.hasAttribute("hidden"); - }); - + const hasHiddenInput = await page.$eval( + `[data-annotation-id="9R"] > input`, + elem => elem?.hasAttribute("hidden") + ); expect(hasHiddenInput).withContext(`In ${browserName}`).toEqual(true); await page.click(getSelector("12R")); await page.waitForTimeout(10); - const hasHiddenCanvas = await page.evaluate(_ => { - const elem = document.querySelector( - `[data-annotation-id="9R"] > canvas` - ); - return elem?.hasAttribute("hidden"); - }); + const hasHiddenCanvas = await page.$eval( + `[data-annotation-id="9R"] > canvas`, + elem => elem?.hasAttribute("hidden") + ); expect(hasHiddenCanvas) .withContext(`In ${browserName}`) .toEqual(true); - const hasVisibleInput = await page.evaluate(_ => { - const elem = document.querySelector( - `[data-annotation-id="9R"] > input` - ); - return elem && !elem.hasAttribute("hidden"); - }); - + const hasVisibleInput = await page.$eval( + `[data-annotation-id="9R"] > input`, + elem => elem && !elem.hasAttribute("hidden") + ); expect(hasVisibleInput) .withContext(`In ${browserName}`) .toEqual(true); From 0d3fbc18187c7d35bbeef6d6d8b11a87e58a8bda Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Sun, 17 Sep 2023 12:59:56 +0200 Subject: [PATCH 2/2] Wait for selector instead of timeout in the `bug1844576` integration test This integration test currently fails intermittently on the bots because of the fixed timeout in the test, which is sometimes too low on slower systems. The issue can be reproduced 100% of the time by introducing a delay in the `WidgetAnnotationElement.showElementAndHideCanvas` method. Puppeteer also discourages this and instead recommends waiting for a selector instead, which we now do here. This ensures that the test only continues if the element under test is available and therefore prevents any timing problems. --- test/integration/scripting_spec.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/integration/scripting_spec.js b/test/integration/scripting_spec.js index 98f12a6b9..540d0c7fb 100644 --- a/test/integration/scripting_spec.js +++ b/test/integration/scripting_spec.js @@ -2011,7 +2011,9 @@ describe("Interaction", () => { expect(hasHiddenInput).withContext(`In ${browserName}`).toEqual(true); await page.click(getSelector("12R")); - await page.waitForTimeout(10); + await page.waitForSelector( + `[data-annotation-id="9R"] > canvas[hidden]` + ); const hasHiddenCanvas = await page.$eval( `[data-annotation-id="9R"] > canvas`,