Browse Source

test: fixup flaky visibility test (#43064)

John Kleinschmidt 8 months ago
parent
commit
1a6e651844
2 changed files with 39 additions and 32 deletions
  1. 8 3
      spec/api-web-contents-view-spec.ts
  2. 31 29
      spec/lib/spec-helpers.ts

+ 8 - 3
spec/api-web-contents-view-spec.ts

@@ -3,7 +3,7 @@ import { BaseWindow, BrowserWindow, View, WebContentsView, webContents, screen }
 import { once } from 'node:events';
 
 import { closeAllWindows } from './lib/window-helpers';
-import { defer, ifdescribe } from './lib/spec-helpers';
+import { defer, ifdescribe, waitUntil } from './lib/spec-helpers';
 import { HexColors, ScreenCapture, hasCapturableScreen, nextFrameTime } from './lib/screen-helpers';
 
 describe('WebContentsView', () => {
@@ -136,6 +136,11 @@ describe('WebContentsView', () => {
   });
 
   describe('visibilityState', () => {
+    async function haveVisibilityState (view: WebContentsView, state: string) {
+      const docVisState = await view.webContents.executeJavaScript('document.visibilityState');
+      return docVisState === state;
+    }
+
     it('is initially hidden', async () => {
       const v = new WebContentsView();
       await v.webContents.loadURL('data:text/html,<script>initialVisibility = document.visibilityState</script>');
@@ -172,7 +177,7 @@ describe('WebContentsView', () => {
       const v = new WebContentsView();
       w.setContentView(v);
       await v.webContents.loadURL('about:blank');
-      expect(await v.webContents.executeJavaScript('document.visibilityState')).to.equal('visible');
+      await expect(waitUntil(async () => await haveVisibilityState(v, 'visible'))).to.eventually.be.fulfilled();
       const p = v.webContents.executeJavaScript('new Promise(resolve => document.addEventListener("visibilitychange", resolve))');
       // We have to wait until the listener above is fully registered before hiding the window.
       // On Windows, the executeJavaScript and the visibilitychange can happen out of order
@@ -204,7 +209,7 @@ describe('WebContentsView', () => {
       const v = new WebContentsView();
       w.setContentView(v);
       await v.webContents.loadURL('about:blank');
-      expect(await v.webContents.executeJavaScript('document.visibilityState')).to.equal('visible');
+      await expect(waitUntil(async () => await haveVisibilityState(v, 'visible'))).to.eventually.be.fulfilled();
 
       const p = v.webContents.executeJavaScript('new Promise(resolve => document.addEventListener("visibilitychange", () => resolve(document.visibilityState)))');
       // Ensure the listener has been registered.

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

@@ -9,6 +9,7 @@ import * as url from 'node:url';
 import { SuiteFunction, TestFunction } from 'mocha';
 import { BrowserWindow } from 'electron/main';
 import { AssertionError } from 'chai';
+import { setTimeout } from 'node:timers/promises';
 
 const addOnly = <T>(fn: Function): T => {
   const wrapped = (...args: any[]) => {
@@ -92,49 +93,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> (