Repository: electron/electron
Pull request: test: fix visibility-state-spec.ts flaky test

#### Description of Change
The visibility-state-spec.ts has been significantly flaky on Windows x64 lately.  This PR refactors that spec to be more resilient.  Additionally, this PR runs the visibility-state-spec first when testing because it appears that other tests/specs may be interfering with this spec but I wasn't able to successfully identify which spec(s) were causing the issue, so for now we can resolve that issue by running that spec first.
<!--
Thank you for your Pull Request. Please provide a description above and review
the requirements below.

Contributors guide: https://github.com/electron/electron/blob/main/CONTRIBUTING.md
-->

#### Checklist
<!-- Remove items that do not apply. For completed items, change [ ] to [x]. -->

- [x] PR description included and stakeholders cc'd
- [x] `npm test` passes
- [x] tests are [changed or added](https://github.com/electron/electron/blob/main/docs/development/testing.md)
- [x] [PR release notes](https://github.com/electron/clerk/blob/main/README.md) describe the change in a way relevant to app developers, and are [capitalized, punctuated, and past tense](https://github.com/electron/clerk/blob/main/README.md#examples).

#### Release Notes

Notes: <!-- Please add a one-line description for app developers to read in the release notes, or 'none' if no notes relevant to app developers. Examples and help on special cases: https://github.com/electron/clerk/blob/main/README.md#examples -->none


Changed files:
filename: appveyor-woa.yml
patch: @@ -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
 

filename: appveyor.yml
patch: @@ -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"

filename: script/split-tests.js
patch: @@ -1,6 +1,9 @@
 const glob = require('glob');
 
 const fs = require('node:fs');
+const path = require('node:path');
+
+const VISIBILITY_SPEC = 'spec\\visibility-state-spec.ts';
 
 const currentShard = parseInt(process.argv[2], 10);
 const shardCount = parseInt(process.argv[3], 10);
@@ -25,9 +28,16 @@ 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;
 }
 
+const visiblitySpecIdx = buckets[currentShard - 1];
+if (visiblitySpecIdx > -1) {
+  // 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.
+  buckets[currentShard - 1].splice(visiblitySpecIdx, 1);
+  buckets[currentShard - 1].unshift(VISIBILITY_SPEC);
+}
 console.log(buckets[currentShard - 1].join(' '));

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

filename: spec/visibility-state-spec.ts
patch: @@ -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 { closeWindow } from './lib/window-helpers';
+import { ifdescribe, waitUntil } from './lib/spec-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(() => {
-    return closeWindow(w);
+  before(() => {
+    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,83 +56,71 @@ 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');
-    expect(state).to.equal('visible');
+    await expect(waitUntil(async () => await haveVisibilityState('visible'))).to.eventually.be.fulfilled();
   });
 
   itWithOptions('should be visible when the window is initially shown', {
     show: true
   }, async () => {
     load();
-    const [, state] = await once(ipcMain, 'initial-visibility-state');
-    expect(state).to.equal('visible');
+    await expect(waitUntil(async () => await haveVisibilityState('visible'))).to.eventually.be.fulfilled();
   });
 
   itWithOptions('should be hidden when the window is initially hidden', {
     show: false
   }, async () => {
     load();
-    const [, state] = await once(ipcMain, 'initial-visibility-state');
-    expect(state).to.equal('hidden');
+    await expect(waitUntil(async () => await haveVisibilityState('hidden'))).to.eventually.be.fulfilled();
   });
 
   itWithOptions('should be visible when the window is initially hidden but shown before the page is loaded', {
     show: false
   }, async () => {
     w.show();
     load();
-    const [, state] = await once(ipcMain, 'initial-visibility-state');
-    expect(state).to.equal('visible');
+    await expect(waitUntil(async () => await haveVisibilityState('visible'))).to.eventually.be.fulfilled();
   });
 
   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');
-    expect(state).to.equal('hidden');
+    await expect(waitUntil(async () => await haveVisibilityState('hidden'))).to.eventually.be.fulfilled();
   });
 
   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');
-    expect(initialState).to.equal('visible');
+    await expect(waitUntil(async () => await haveVisibilityState('visible'))).to.eventually.be.fulfilled();
     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');
-    expect(initialState).to.equal('visible');
-    w.minimize();
-    const p = once(ipcMain, 'visibility-change-hidden');
+    await expect(waitUntil(async () => await haveVisibilityState('visible'))).to.eventually.be.fulfilled();
     w.minimize();
-    await p;
+    await expect(waitUntil(async () => await haveVisibilityState('hidden'))).to.eventually.be.fulfilled();
   });
 
   itWithOptions('should become visible when a window is restored', {}, async () => {
     load();
-    const [, initialState] = await once(ipcMain, 'initial-visibility-state');
-    expect(initialState).to.equal('visible');
+    await expect(waitUntil(async () => await haveVisibilityState('visible'))).to.eventually.be.fulfilled();
     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');
-      expect(state).to.equal('visible');
+      await expect(waitUntil(async () => await haveVisibilityState('visible'))).to.eventually.be.fulfilled();
     });
 
     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');
-      expect(state).to.equal('visible');
+      await expect(waitUntil(async () => await haveVisibilityState('visible'))).to.eventually.be.fulfilled();
     });
 
     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');
-      expect(state).to.equal('visible');
+      await expect(waitUntil(async () => await haveVisibilityState('visible'))).to.eventually.be.fulfilled();
       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();
     });
   });
 });