test: fix visibility-state-spec.ts flaky test (#44199)

* test: refactor visibility-state-spec

Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org>

* ci: shard tests

Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org>

* test: update split-tests for use on Windows

Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org>

* test: run visibility-state-spec.ts first

Co-Authored-By: John Kleinschmidt <jkleinsc@electronjs.org>

---------

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org>

appveyor-woa.yml

@@ -45,10 +45,16 @@ environment:
   matrix:
 
     - job_name: Build Arm on X64 Windows
-    - job_name: Test On Windows On Arm Hardware
+    - job_name: Test On Windows On Arm Hardware 1
       job_depends_on: Build Arm on X64 Windows
       APPVEYOR_BUILD_WORKER_IMAGE: base-woa
       APPVEYOR_BUILD_WORKER_CLOUD: electronhq-woa
+      shard: 1
+    - job_name: Test On Windows On Arm Hardware 2
+      job_depends_on: Build Arm on X64 Windows
+      APPVEYOR_BUILD_WORKER_IMAGE: base-woa
+      APPVEYOR_BUILD_WORKER_CLOUD: electronhq-woa
+      shard: 2
 
 clone_script:
 - ps: git clone -q $("--branch=" + $Env:APPVEYOR_REPO_BRANCH) $("https://github.com/" + $Env:APPVEYOR_REPO_NAME + ".git") $Env:APPVEYOR_BUILD_FOLDER
@@ -254,7 +260,8 @@ for:
           }
   - matrix:
       only:
-        - job_name: Test On Windows On Arm Hardware
+        - job_name: Test On Windows On Arm Hardware 1
+        - job_name: Test On Windows On Arm Hardware 2
 
     environment:
         IGNORE_YARN_INSTALL_ERROR: 1
@@ -326,7 +333,9 @@ for:
           if ($env:TARGET_ARCH -eq 'ia32') {
             $env:npm_config_arch = "ia32"
           }
-      - echo Running main test suite & node script/yarn test  --runners=main --enable-logging --disable-features=CalculateNativeWinOcclusion
+      - ps: $env:tests_files=node script\split-tests $env:shard 2
+      - echo "Running shard %shard% specs %tests_files%"
+      - echo Running main test suite & node script/yarn test  --runners=main --enable-logging --disable-features=CalculateNativeWinOcclusion --files %tests_files%
       - cd ..
       - echo Verifying non proprietary ffmpeg & python electron\script\verify-ffmpeg.py --build-dir out\Default --source-root %cd% --ffmpeg-path out\ffmpeg
 

appveyor.yml

@@ -45,8 +45,12 @@ environment:
   matrix:
 
     - job_name: Build
-    - job_name: Test
+    - job_name: Test 1
       job_depends_on: Build
+      shard: 1
+    - job_name: Test 2
+      job_depends_on: Build
+      shard: 2
 
 clone_script:
 - ps: git clone -q $("--branch=" + $Env:APPVEYOR_REPO_BRANCH) $("https://github.com/" + $Env:APPVEYOR_REPO_NAME + ".git") $Env:APPVEYOR_BUILD_FOLDER
@@ -246,7 +250,8 @@ for:
           }
   - matrix:
       only:
-        - job_name: Test
+        - job_name: Test 1
+        - job_name: Test 2
 
     environment:
         DD_ENV: ci
@@ -316,7 +321,9 @@ for:
           if ($env:TARGET_ARCH -eq 'ia32') {
             $env:npm_config_arch = "ia32"
           }
-      - echo Running main test suite & node script/yarn test -- --trace-uncaught --runners=main --enable-logging
+      - ps: $env:tests_files=node script\split-tests $env:shard 2
+      - echo "Running shard %shard% specs %tests_files%"
+      - echo Running main test suite & node script/yarn test -- --trace-uncaught --runners=main --enable-logging --files %tests_files%
       - cd ..
       - echo Verifying non proprietary ffmpeg & python electron\script\verify-ffmpeg.py --build-dir out\Default --source-root %cd% --ffmpeg-path out\ffmpeg
       - echo "About to verify mksnapshot"

script/split-tests.js

@@ -1,6 +1,7 @@
 const glob = require('glob');
 
 const fs = require('node:fs');
+const path = require('node:path');
 
 const currentShard = parseInt(process.argv[2], 10);
 const shardCount = parseInt(process.argv[3], 10);
@@ -25,7 +26,7 @@ specFiles.sort((a, b) => {
 
 let shard = 0;
 for (const specFile of specFiles) {
-  buckets[shard].push(specFile);
+  buckets[shard].push(path.normalize(specFile));
   shard++;
   if (shard === shardCount) shard = 0;
 }

spec/fixtures/chromium/visibilitystate.html

@@ -7,12 +7,6 @@
   <title>Document</title>
 </head>
 <body>
-  <script>
+  <h1>Visibility Test</h1>
-    const { ipcRenderer } = require('electron')
-    ipcRenderer.send('initial-visibility-state', document.visibilityState)
-    document.addEventListener('visibilitychange', () => {
-      ipcRenderer.send(`visibility-change-${document.visibilityState}`)
-    })
-  </script>
 </body>
 </html>

spec/index.js

@@ -149,7 +149,17 @@ app.whenReady().then(async () => {
 
   const { getFiles } = require('./get-files');
   const testFiles = await getFiles(__dirname, filter);
-  for (const file of testFiles.sort()) {
+  const VISIBILITY_SPEC = ('visibility-state-spec.ts');
+  const sortedFiles = testFiles.sort((a, b) => {
+    // If visibility-state-spec is in the list, move it to the first position
+    // so that it gets executed first to avoid other specs interferring with it.
+    if (a.indexOf(VISIBILITY_SPEC) > -1) {
+      return -1;
+    } else {
+      return a.localeCompare(b);
+    }
+  });
+  for (const file of sortedFiles) {
     mocha.addFile(file);
   }
 

spec/visibility-state-spec.ts

@@ -1,26 +1,37 @@
-import { BaseWindow, BrowserWindow, BrowserWindowConstructorOptions, ipcMain, WebContents, WebContentsView } from 'electron/main';
+import { BaseWindow, BrowserWindow, BrowserWindowConstructorOptions, WebContents, WebContentsView } from 'electron/main';
 
 import { expect } from 'chai';
 
 import * as cp from 'node:child_process';
 import { once } from 'node:events';
 import * as path from 'node:path';
-import { setTimeout } from 'node:timers/promises';
 
-import { ifdescribe } from './lib/spec-helpers';
+import { ifdescribe, waitUntil } from './lib/spec-helpers';
-import { closeWindow } from './lib/window-helpers';
+import { closeAllWindows } from './lib/window-helpers';
 
 // visibilityState specs pass on linux with a real window manager but on CI
 // the environment does not let these specs pass
 ifdescribe(process.platform !== 'linux')('document.visibilityState', () => {
   let w: BaseWindow & {webContents: WebContents};
 
-  afterEach(() => {
+  before(() => {
-    return closeWindow(w);
+    for (const checkWin of BaseWindow.getAllWindows()) {
+      console.log('WINDOW EXISTS BEFORE TEST STARTED:', checkWin.title, checkWin.id);
+    }
+  });
+
+  afterEach(async () => {
+    await closeAllWindows();
+    w = null as unknown as BrowserWindow;
   });
 
   const load = () => w.webContents.loadFile(path.resolve(__dirname, 'fixtures', 'chromium', 'visibilitystate.html'));
 
+  async function haveVisibilityState (state: string) {
+    const docVisState = await w.webContents.executeJavaScript('document.visibilityState');
+    return docVisState === state;
+  }
+
   const itWithOptions = (name: string, options: BrowserWindowConstructorOptions, fn: Mocha.Func) => {
     it(name, async function (...args) {
       w = new BrowserWindow({
@@ -32,6 +43,9 @@ ifdescribe(process.platform !== 'linux')('document.visibilityState', () => {
           contextIsolation: false
         }
       });
+      if (options.show && process.platform === 'darwin') {
+        await once(w, 'show');
+      }
       await Promise.resolve(fn.apply(this, args));
     });
 
@@ -42,30 +56,30 @@ ifdescribe(process.platform !== 'linux')('document.visibilityState', () => {
       const wcv = new WebContentsView({ webPreferences: { ...(options.webPreferences ?? {}), nodeIntegration: true, contextIsolation: false } });
       baseWindow.contentView = wcv;
       w = Object.assign(baseWindow, { webContents: wcv.webContents });
+      if (options.show && process.platform === 'darwin') {
+        await once(w, 'show');
+      }
       await Promise.resolve(fn.apply(this, args));
     });
   };
 
   itWithOptions('should be visible when the window is initially shown by default', {}, async () => {
     load();
-    const [, state] = await once(ipcMain, 'initial-visibility-state');
+    await expect(waitUntil(async () => await haveVisibilityState('visible'))).to.eventually.be.fulfilled();
-    expect(state).to.equal('visible');
   });
 
   itWithOptions('should be visible when the window is initially shown', {
     show: true
   }, async () => {
     load();
-    const [, state] = await once(ipcMain, 'initial-visibility-state');
+    await expect(waitUntil(async () => await haveVisibilityState('visible'))).to.eventually.be.fulfilled();
-    expect(state).to.equal('visible');
   });
 
   itWithOptions('should be hidden when the window is initially hidden', {
     show: false
   }, async () => {
     load();
-    const [, state] = await once(ipcMain, 'initial-visibility-state');
+    await expect(waitUntil(async () => await haveVisibilityState('hidden'))).to.eventually.be.fulfilled();
-    expect(state).to.equal('hidden');
   });
 
   itWithOptions('should be visible when the window is initially hidden but shown before the page is loaded', {
@@ -73,52 +87,40 @@ ifdescribe(process.platform !== 'linux')('document.visibilityState', () => {
   }, async () => {
     w.show();
     load();
-    const [, state] = await once(ipcMain, 'initial-visibility-state');
+    await expect(waitUntil(async () => await haveVisibilityState('visible'))).to.eventually.be.fulfilled();
-    expect(state).to.equal('visible');
   });
 
   itWithOptions('should be hidden when the window is initially shown but hidden before the page is loaded', {
     show: true
   }, async () => {
-    // TODO(MarshallOfSound): Figure out if we can work around this 1 tick issue for users
-    if (process.platform === 'darwin') {
-      // Wait for a tick, the window being "shown" takes 1 tick on macOS
-      await setTimeout(10000);
-    }
     w.hide();
     load();
-    const [, state] = await once(ipcMain, 'initial-visibility-state');
+    await expect(waitUntil(async () => await haveVisibilityState('hidden'))).to.eventually.be.fulfilled();
-    expect(state).to.equal('hidden');
   });
 
   itWithOptions('should be toggle between visible and hidden as the window is hidden and shown', {}, async () => {
     load();
-    const [, initialState] = await once(ipcMain, 'initial-visibility-state');
+    await expect(waitUntil(async () => await haveVisibilityState('visible'))).to.eventually.be.fulfilled();
-    expect(initialState).to.equal('visible');
     w.hide();
-    await once(ipcMain, 'visibility-change-hidden');
+    await expect(waitUntil(async () => await haveVisibilityState('hidden'))).to.eventually.be.fulfilled();
     w.show();
-    await once(ipcMain, 'visibility-change-visible');
+    await expect(waitUntil(async () => await haveVisibilityState('visible'))).to.eventually.be.fulfilled();
   });
 
   itWithOptions('should become hidden when a window is minimized', {}, async () => {
     load();
-    const [, initialState] = await once(ipcMain, 'initial-visibility-state');
+    await expect(waitUntil(async () => await haveVisibilityState('visible'))).to.eventually.be.fulfilled();
-    expect(initialState).to.equal('visible');
     w.minimize();
-    const p = once(ipcMain, 'visibility-change-hidden');
+    await expect(waitUntil(async () => await haveVisibilityState('hidden'))).to.eventually.be.fulfilled();
-    w.minimize();
-    await p;
   });
 
   itWithOptions('should become visible when a window is restored', {}, async () => {
     load();
-    const [, initialState] = await once(ipcMain, 'initial-visibility-state');
+    await expect(waitUntil(async () => await haveVisibilityState('visible'))).to.eventually.be.fulfilled();
-    expect(initialState).to.equal('visible');
     w.minimize();
-    await once(ipcMain, 'visibility-change-hidden');
+    await expect(waitUntil(async () => await haveVisibilityState('hidden'))).to.eventually.be.fulfilled();
     w.restore();
-    await once(ipcMain, 'visibility-change-visible');
+    await expect(waitUntil(async () => await haveVisibilityState('visible'))).to.eventually.be.fulfilled();
   });
 
   ifdescribe(process.platform === 'darwin')('on platforms that support occlusion detection', () => {
@@ -152,8 +154,7 @@ ifdescribe(process.platform !== 'linux')('document.visibilityState', () => {
         height: 200
       });
       load();
-      const [, state] = await once(ipcMain, 'initial-visibility-state');
+      await expect(waitUntil(async () => await haveVisibilityState('visible'))).to.eventually.be.fulfilled();
-      expect(state).to.equal('visible');
     });
 
     itWithOptions('should be visible when two windows are on screen that overlap partially', {
@@ -169,8 +170,7 @@ ifdescribe(process.platform !== 'linux')('document.visibilityState', () => {
         height: 200
       });
       load();
-      const [, state] = await once(ipcMain, 'initial-visibility-state');
+      await expect(waitUntil(async () => await haveVisibilityState('visible'))).to.eventually.be.fulfilled();
-      expect(state).to.equal('visible');
     });
 
     itWithOptions('should be hidden when a second window completely occludes the current window', {
@@ -181,15 +181,14 @@ ifdescribe(process.platform !== 'linux')('document.visibilityState', () => {
     }, async function () {
       this.timeout(240000);
       load();
-      const [, state] = await once(ipcMain, 'initial-visibility-state');
+      await expect(waitUntil(async () => await haveVisibilityState('visible'))).to.eventually.be.fulfilled();
-      expect(state).to.equal('visible');
       makeOtherWindow({
         x: 0,
         y: 0,
         width: 300,
         height: 300
       });
-      await once(ipcMain, 'visibility-change-hidden');
+      await expect(waitUntil(async () => await haveVisibilityState('hidden'))).to.eventually.be.fulfilled();
     });
   });
 });