Browse Source

test: fixup flaky tests (#44380)

* test: fixup flaky test

Co-authored-by: John Kleinschmidt <[email protected]>

* test: disable flaky protocol speed test on macOS

Co-authored-by: John Kleinschmidt <[email protected]>

* test: fixup flaky test in api-browser-window-spec.ts

Co-authored-by: John Kleinschmidt <[email protected]>

* test: update waitUntil to handle async functions

---------

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: John Kleinschmidt <[email protected]>
trop[bot] 5 months ago
parent
commit
e9988c2fc4
4 changed files with 45 additions and 38 deletions
  1. 5 3
      spec/api-browser-window-spec.ts
  2. 2 1
      spec/api-protocol-spec.ts
  3. 7 5
      spec/chromium-spec.ts
  4. 31 29
      spec/lib/spec-helpers.ts

+ 5 - 3
spec/api-browser-window-spec.ts

@@ -15,7 +15,7 @@ import { setTimeout } from 'node:timers/promises';
 
 import { emittedUntil, emittedNTimes } from './lib/events-helpers';
 import { HexColors, hasCapturableScreen, ScreenCapture } from './lib/screen-helpers';
-import { ifit, ifdescribe, defer, listen } from './lib/spec-helpers';
+import { ifit, ifdescribe, defer, listen, waitUntil } from './lib/spec-helpers';
 import { closeWindow, closeAllWindows } from './lib/window-helpers';
 
 const fixtures = path.resolve(__dirname, 'fixtures');
@@ -5978,8 +5978,10 @@ describe('BrowserWindow module', () => {
           w.webContents.on('enter-html-full-screen', async () => {
             enterCount++;
             if (w.isFullScreen()) reject(new Error('w.isFullScreen should be false'));
-            const isFS = await w.webContents.executeJavaScript('!!document.fullscreenElement');
-            if (!isFS) reject(new Error('Document should have fullscreen element'));
+            await waitUntil(async () => {
+              const isFS = await w.webContents.executeJavaScript('!!document.fullscreenElement');
+              return isFS === true;
+            });
             checkDone();
           });
 

+ 2 - 1
spec/api-protocol-spec.ts

@@ -1737,7 +1737,8 @@ describe('protocol module', () => {
     });
 
     // TODO(nornagon): this test doesn't pass on Linux currently, investigate.
-    ifit(process.platform !== 'linux')('is fast', async () => {
+    // test is also flaky on CI on macOS so it is currently disabled there as well.
+    ifit(process.platform !== 'linux' && (!process.env.CI || process.platform !== 'darwin'))('is fast', async () => {
       // 128 MB of spaces.
       const chunk = new Uint8Array(128 * 1024 * 1024);
       chunk.fill(' '.charCodeAt(0));

+ 7 - 5
spec/chromium-spec.ts

@@ -15,7 +15,7 @@ import * as path from 'node:path';
 import { setTimeout } from 'node:timers/promises';
 import * as url from 'node:url';
 
-import { ifit, ifdescribe, defer, itremote, listen, startRemoteControlApp } from './lib/spec-helpers';
+import { ifit, ifdescribe, defer, itremote, listen, startRemoteControlApp, waitUntil } from './lib/spec-helpers';
 import { closeAllWindows } from './lib/window-helpers';
 import { PipeTransport } from './pipe-transport';
 
@@ -2946,10 +2946,12 @@ describe('iframe using HTML fullscreen API while window is OS-fullscreened', ()
     );
     await once(w.webContents, 'leave-html-full-screen');
 
-    const width = await w.webContents.executeJavaScript(
-      "document.querySelector('iframe').offsetWidth"
-    );
-    expect(width).to.equal(0);
+    await expect(waitUntil(async () => {
+      const width = await w.webContents.executeJavaScript(
+        "document.querySelector('iframe').offsetWidth"
+      );
+      return width === 0;
+    })).to.eventually.be.fulfilled();
 
     w.setFullScreen(false);
     await once(w, 'leave-full-screen');

+ 31 - 29
spec/lib/spec-helpers.ts

@@ -9,6 +9,7 @@ import * as http2 from 'node:http2';
 import * as https from 'node:https';
 import * as net from 'node:net';
 import * as path from 'node:path';
+import { setTimeout } from 'node:timers/promises';
 import * as url from 'node:url';
 import * as v8 from 'node:v8';
 
@@ -94,49 +95,50 @@ export async function startRemoteControlApp (extraArgs: string[] = [], options?:
 }
 
 export function waitUntil (
-  callback: () => boolean,
+  callback: () => boolean|Promise<boolean>,
   opts: { rate?: number, timeout?: number } = {}
 ) {
   const { rate = 10, timeout = 10000 } = opts;
-  return new Promise<void>((resolve, reject) => {
-    let intervalId: NodeJS.Timeout | undefined; // eslint-disable-line prefer-const
-    let timeoutId: NodeJS.Timeout | undefined;
+  return (async () => {
+    const ac = new AbortController();
+    const signal = ac.signal;
+    let checkCompleted = false;
+    let timedOut = false;
 
-    const cleanup = () => {
-      if (intervalId) clearInterval(intervalId);
-      if (timeoutId) clearTimeout(timeoutId);
-    };
-
-    const check = () => {
+    const check = async () => {
       let result;
 
       try {
-        result = callback();
+        result = await callback();
       } catch (e) {
-        cleanup();
-        reject(e);
-        return;
+        ac.abort();
+        throw e;
       }
 
-      if (result === true) {
-        cleanup();
-        resolve();
-        return true;
-      }
+      return result;
     };
 
-    if (check()) {
-      return;
-    }
+    setTimeout(timeout, { signal })
+      .then(() => {
+        timedOut = true;
+        checkCompleted = true;
+      });
 
-    intervalId = setInterval(check, rate);
+    while (checkCompleted === false) {
+      const checkSatisfied = await check();
+      if (checkSatisfied === true) {
+        ac.abort();
+        checkCompleted = true;
+        return;
+      } else {
+        await setTimeout(rate);
+      }
+    }
 
-    timeoutId = setTimeout(() => {
-      timeoutId = undefined;
-      cleanup();
-      reject(new Error(`waitUntil timed out after ${timeout}ms`));
-    }, timeout);
-  });
+    if (timedOut) {
+      throw new Error(`waitUntil timed out after ${timeout}ms`);
+    }
+  })();
 }
 
 export async function repeatedly<T> (