From d936c38af0f353c1b99ce980e41f07708c0eb216 Mon Sep 17 00:00:00 2001 From: Marat Abdullin Date: Wed, 25 Feb 2026 11:17:11 +0100 Subject: [PATCH 1/2] A little bit of asynchronosity for the validations during the Playwright actions. --- src/core.ts | 13 +++--- tests/testingMode/testingMode.test.ts | 41 +++++-------------- tools/playwright/src/page-injector.ts | 6 ++- .../tests/page-injector.test.spec.ts | 12 +++--- 4 files changed, 26 insertions(+), 46 deletions(-) diff --git a/src/core.ts b/src/core.ts index aa9cf02..74d85e0 100644 --- a/src/core.ts +++ b/src/core.ts @@ -574,10 +574,7 @@ export class AbleDOM { return issues; } - idle( - markAsRead?: boolean, - timeout?: number, - ): Promise { + idle(markAsRead?: boolean, timeout?: number): Promise { if (!this._clearValidationTimeout) { return Promise.resolve(this._getCurrentIssues(!!markAsRead)); } @@ -586,11 +583,10 @@ export class AbleDOM { let timeoutResolve: (() => void) | undefined; let timedOut = false; let timeoutPromise = timeout - ? new Promise((resolve) => { + ? new Promise((resolve) => { timeoutResolve = () => { timeoutClear?.(); - timeoutResolve = undefined; - resolve(null); + resolve(this._getCurrentIssues(!!markAsRead)); }; let timeoutTimer = this._win.setTimeout(() => { @@ -601,6 +597,7 @@ export class AbleDOM { timeoutClear = () => { this._win.clearTimeout(timeoutTimer); + timeoutResolve = undefined; timeoutClear = undefined; }; }) @@ -612,7 +609,7 @@ export class AbleDOM { delete this._idlePromise; delete this._idleResolve; resolve(this._getCurrentIssues(timedOut ? false : !!markAsRead)); - timeoutResolve?.(); + timeoutClear?.(); }; }); } diff --git a/tests/testingMode/testingMode.test.ts b/tests/testingMode/testingMode.test.ts index 87be697..7b29965 100644 --- a/tests/testingMode/testingMode.test.ts +++ b/tests/testingMode/testingMode.test.ts @@ -7,7 +7,7 @@ import { loadTestPage, issueSelector } from "../utils"; interface WindowWithAbleDOMInstance extends Window { ableDOMInstanceForTesting?: { - idle: (markAsRead?: boolean, timeout?: number) => Promise; + idle: (markAsRead?: boolean, timeout?: number) => Promise; highlightElement: (element: HTMLElement, scrollIntoView: boolean) => void; }; } @@ -209,7 +209,7 @@ test.describe("exposeInstanceForTesting prop", () => { expect(secondCallIssues).toBe(1); }); - test("idle() with timeout should return null when timeout expires before validation completes", async ({ + test("idle() with timeout should return current issues when timeout expires before validation completes", async ({ page, }) => { await loadTestPage(page, "tests/testingMode/exposed-headless.html"); @@ -234,13 +234,12 @@ test.describe("exposeInstanceForTesting prop", () => { btn.setAttribute("data-test", "changed"); } - // Call idle with 1ms timeout - should return null if validation is still pending + // Call idle with 1ms timeout - should return current issues even if validation is still pending return await instance?.idle(false, 1); }); - // Result should be null (timeout) or an array (validation completed fast enough) - // We accept both outcomes since timing can vary - expect(result === null || Array.isArray(result)).toBe(true); + // Result should always be an array (either empty or with current issues) + expect(Array.isArray(result)).toBe(true); }); test("idle() with sufficient timeout should return issues", async ({ @@ -298,22 +297,12 @@ test.describe("exposeInstanceForTesting prop", () => { btn.setAttribute("data-test", Date.now().toString()); } - // Call with markAsRead=true but 1ms timeout - should return null + // Call with markAsRead=true but 1ms timeout - should return current issues return await instance?.idle(true, 1); }); - // If timeout occurred, issues should NOT have been marked as read - if (timedOutResult === null) { - const subsequentIssues = await page.evaluate(async () => { - const instance = (window as WindowWithAbleDOMInstance) - .ableDOMInstanceForTesting; - const result = await instance?.idle(false, 5000); - return result?.length ?? 0; - }); - - // Issues should still be available since timeout prevented markAsRead - expect(subsequentIssues).toBe(1); - } + // Result should always be an array + expect(Array.isArray(timedOutResult)).toBe(true); }); test("concurrent idle() calls - one with timeout, one without - both should get issues", async ({ @@ -352,17 +341,9 @@ test.describe("exposeInstanceForTesting prop", () => { }; }); - // The call with short timeout may return null (timed out) or issues (validation was fast) - // The call with long timeout should always return issues - expect( - results.withoutTimeout === null || Array.isArray(results.withoutTimeout), - ).toBe(true); - - // If the short timeout call returned null, the long timeout call should still get issues - if (results.withTimeout === null) { - expect(Array.isArray(results.withoutTimeout)).toBe(true); - expect((results.withoutTimeout as unknown[])?.length).toBeGreaterThan(0); - } + // Both calls should return arrays (current issues or completed validation issues) + expect(Array.isArray(results.withTimeout)).toBe(true); + expect(Array.isArray(results.withoutTimeout)).toBe(true); }); test("concurrent idle() calls without timeout should both receive issues", async ({ diff --git a/tools/playwright/src/page-injector.ts b/tools/playwright/src/page-injector.ts index 976f8f8..7e50fc8 100644 --- a/tools/playwright/src/page-injector.ts +++ b/tools/playwright/src/page-injector.ts @@ -18,7 +18,7 @@ export interface AbleDOMIdleOptions { markAsRead?: boolean; /** * Timeout in milliseconds to wait for validation to complete. - * If validation doesn't complete within the timeout, returns null. + * If validation doesn't complete within the timeout, returns the current issues. * @default 1000 */ timeout?: number; @@ -302,7 +302,9 @@ export async function attachAbleDOMMethodsToPage( (locatorProto as unknown as LocatorProtoWithActions)[action] = async function (this: Locator, ...args: unknown[]) { const ret = await originalAction.apply(this, args); - await reportAbleDOMIssues(this, 0); // Setting the timeout for actions to 0 to avoid side effects. + await reportAbleDOMIssues(this, 1); // 0 would block idle() till it's resolved, + // default timeout will give side effects in a form of timeouts, + // so, we're adding just a little bit of asynchronosity. return ret; }; } diff --git a/tools/playwright/tests/page-injector.test.spec.ts b/tools/playwright/tests/page-injector.test.spec.ts index 81cb197..6408433 100644 --- a/tools/playwright/tests/page-injector.test.spec.ts +++ b/tools/playwright/tests/page-injector.test.spec.ts @@ -334,20 +334,20 @@ test.describe("idle options (markAsRead and timeout)", () => { expect(calls[0]).toEqual({ markAsRead: true, timeout: 1000 }); }); - test("should handle null return from idle() when timeout expires", async ({ + test("should handle empty array return from idle() when timeout expires", async ({ page, }, testInfo) => { await page.goto( "data:text/html,", ); - // Mock AbleDOM to return null (simulating timeout expiration) + // Mock AbleDOM to return empty array (simulating timeout with no current issues) await page.evaluate(() => { const win = window as WindowWithAbleDOMInstance; win.ableDOMInstanceForTesting = { idle: async () => { - // Return null to simulate timeout - return null; + // Return empty array to simulate timeout with no current issues + return []; }, highlightElement: () => { /* noop */ @@ -355,10 +355,10 @@ test.describe("idle options (markAsRead and timeout)", () => { }; }); - // This should not throw even when idle() returns null + // This should not throw even when idle() returns empty array await page.locator("button").waitFor(); - // Verify no issues were reported (since null was returned) + // Verify no issues were reported (since empty array was returned) const customDataAttachments = testInfo.attachments.filter( (att) => att.name === "abledom-test-data", ); From 88da1d6230437038fd1986fc09808489eee5e72e Mon Sep 17 00:00:00 2001 From: Marat Abdullin Date: Wed, 25 Feb 2026 11:18:02 +0100 Subject: [PATCH 2/2] Version bump. --- tools/playwright/package-lock.json | 4 ++-- tools/playwright/package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/playwright/package-lock.json b/tools/playwright/package-lock.json index 74886f6..c41f8c4 100644 --- a/tools/playwright/package-lock.json +++ b/tools/playwright/package-lock.json @@ -1,12 +1,12 @@ { "name": "abledom-playwright", - "version": "0.0.19", + "version": "0.0.20", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "abledom-playwright", - "version": "0.0.19", + "version": "0.0.20", "license": "MIT", "devDependencies": { "@eslint/js": "^9.39.2", diff --git a/tools/playwright/package.json b/tools/playwright/package.json index 5149600..14b8486 100644 --- a/tools/playwright/package.json +++ b/tools/playwright/package.json @@ -1,6 +1,6 @@ { "name": "abledom-playwright", - "version": "0.0.19", + "version": "0.0.20", "description": "AbleDOM tools for Playwright", "author": "Marat Abdullin ", "license": "MIT",