test: disable flaky macOS panel test & refactor screen capture testing (#41461)

* Disable flaky test

Co-authored-by: clavin <clavin@electronjs.org>

* Add helper for storing test artifacts

Co-authored-by: clavin <clavin@electronjs.org>

* Refactor screen capture tests

We have a pattern for inspecting a screen capture, so this refactor codifies that pattern into a helper. This gives us shorter test code, consistency (previously, the display in test code and the display captured could theoretically be different), and better debugging/observability on failure.

Co-authored-by: clavin <clavin@electronjs.org>

---------

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

.circleci/config/base.yml

@@ -1642,6 +1642,8 @@ commands:
             fi
       - store_test_results:
           path: src/junit
+      - store_artifacts:
+          path: src/electron/spec/artifacts
 
       - *step-verify-mksnapshot
       - *step-verify-chromedriver

spec/api-browser-view-spec.ts

@@ -3,7 +3,7 @@ import * as path from 'node:path';
 import { BrowserView, BrowserWindow, screen, webContents } from 'electron/main';
 import { closeWindow } from './lib/window-helpers';
 import { defer, ifit, startRemoteControlApp } from './lib/spec-helpers';
-import { areColorsSimilar, captureScreen, getPixelColor } from './lib/screen-helpers';
+import { ScreenCapture } from './lib/screen-helpers';
 import { once } from 'node:events';
 
 describe('BrowserView module', () => {
@@ -88,13 +88,8 @@ describe('BrowserView module', () => {
       w.setBrowserView(view);
       await view.webContents.loadURL('data:text/html,hello there');
 
-      const screenCapture = await captureScreen();
+      const screenCapture = await ScreenCapture.createForDisplay(display);
-      const centerColor = getPixelColor(screenCapture, {
+      await screenCapture.expectColorAtCenterMatches(WINDOW_BACKGROUND_COLOR);
-        x: display.size.width / 2,
-        y: display.size.height / 2
-      });
-
-      expect(areColorsSimilar(centerColor, WINDOW_BACKGROUND_COLOR)).to.be.true();
     });
 
     // Linux and arm64 platforms (WOA and macOS) do not return any capture sources
@@ -114,13 +109,8 @@ describe('BrowserView module', () => {
       w.setBackgroundColor(VIEW_BACKGROUND_COLOR);
       await view.webContents.loadURL('data:text/html,hello there');
 
-      const screenCapture = await captureScreen();
+      const screenCapture = await ScreenCapture.createForDisplay(display);
-      const centerColor = getPixelColor(screenCapture, {
+      await screenCapture.expectColorAtCenterMatches(VIEW_BACKGROUND_COLOR);
-        x: display.size.width / 2,
-        y: display.size.height / 2
-      });
-
-      expect(areColorsSimilar(centerColor, VIEW_BACKGROUND_COLOR)).to.be.true();
     });
   });
 

spec/api-browser-window-spec.ts

@@ -11,7 +11,7 @@ import { app, BrowserWindow, BrowserView, dialog, ipcMain, OnBeforeSendHeadersLi
 import { emittedUntil, emittedNTimes } from './lib/events-helpers';
 import { ifit, ifdescribe, defer, listen } from './lib/spec-helpers';
 import { closeWindow, closeAllWindows } from './lib/window-helpers';
-import { areColorsSimilar, captureScreen, HexColors, getPixelColor, hasCapturableScreen } from './lib/screen-helpers';
+import { HexColors, hasCapturableScreen, ScreenCapture } from './lib/screen-helpers';
 import { once } from 'node:events';
 import { setTimeout } from 'node:timers/promises';
 import { setTimeout as syncSetTimeout } from 'node:timers';
@@ -1246,6 +1246,7 @@ describe('BrowserWindow module', () => {
         }
       });
 
+      // FIXME: disabled in `disabled-tests.json`
       ifit(process.platform === 'darwin')('it does not activate the app if focusing an inactive panel', async () => {
         // Show to focus app, then remove existing window
         w.show();
@@ -6496,18 +6497,22 @@ describe('BrowserWindow module', () => {
       await foregroundWindow.loadFile(colorFile);
 
       await setTimeout(1000);
-      const screenCapture = await captureScreen();
-      const leftHalfColor = getPixelColor(screenCapture, {
-        x: display.size.width / 4,
-        y: display.size.height / 2
-      });
-      const rightHalfColor = getPixelColor(screenCapture, {
-        x: display.size.width - (display.size.width / 4),
-        y: display.size.height / 2
-      });
 
-      expect(areColorsSimilar(leftHalfColor, HexColors.GREEN)).to.be.true();
+      const screenCapture = await ScreenCapture.createForDisplay(display);
-      expect(areColorsSimilar(rightHalfColor, HexColors.RED)).to.be.true();
+      await screenCapture.expectColorAtPointOnDisplayMatches(
+        HexColors.GREEN,
+        (size) => ({
+          x: size.width / 4,
+          y: size.height / 2
+        })
+      );
+      await screenCapture.expectColorAtPointOnDisplayMatches(
+        HexColors.RED,
+        (size) => ({
+          x: size.width * 3 / 4,
+          y: size.height / 2
+        })
+      );
     });
 
     ifit(process.platform === 'darwin')('Allows setting a transparent window via CSS', async () => {
@@ -6537,13 +6542,9 @@ describe('BrowserWindow module', () => {
       await once(ipcMain, 'set-transparent');
 
       await setTimeout(1000);
-      const screenCapture = await captureScreen();
-      const centerColor = getPixelColor(screenCapture, {
-        x: display.size.width / 2,
-        y: display.size.height / 2
-      });
 
-      expect(areColorsSimilar(centerColor, HexColors.PURPLE)).to.be.true();
+      const screenCapture = await ScreenCapture.createForDisplay(display);
+      await screenCapture.expectColorAtCenterMatches(HexColors.PURPLE);
     });
 
     // Linux and arm64 platforms (WOA and macOS) do not return any capture sources
@@ -6560,15 +6561,11 @@ describe('BrowserWindow module', () => {
         await window.webContents.loadURL('data:text/html,<head><meta name="color-scheme" content="dark"></head>');
 
         await setTimeout(1000);
-        const screenCapture = await captureScreen();
+        const screenCapture = await ScreenCapture.createForDisplay(display);
-        const centerColor = getPixelColor(screenCapture, {
-          x: display.size.width / 2,
-          y: display.size.height / 2
-        });
-        window.close();
-
         // color-scheme is set to dark so background should not be white
-        expect(areColorsSimilar(centerColor, HexColors.WHITE)).to.be.false();
+        await screenCapture.expectColorAtCenterDoesNotMatch(HexColors.WHITE);
+
+        window.close();
       }
     });
   });
@@ -6590,13 +6587,9 @@ describe('BrowserWindow module', () => {
       await once(w, 'ready-to-show');
 
       await setTimeout(1000);
-      const screenCapture = await captureScreen();
-      const centerColor = getPixelColor(screenCapture, {
-        x: display.size.width / 2,
-        y: display.size.height / 2
-      });
 
-      expect(areColorsSimilar(centerColor, HexColors.BLUE)).to.be.true();
+      const screenCapture = await ScreenCapture.createForDisplay(display);
+      await screenCapture.expectColorAtCenterMatches(HexColors.BLUE);
     });
   });
 

spec/disabled-tests.json

@@ -7,5 +7,6 @@
   "session module ses.cookies should set cookie for standard scheme",
   "webFrameMain module WebFrame.visibilityState should match window state",
   "reporting api sends a report for a deprecation",
-  "chromium features SpeechSynthesis should emit lifecycle events"
+  "chromium features SpeechSynthesis should emit lifecycle events",
+  "BrowserWindow module focus and visibility BrowserWindow.focus() it does not activate the app if focusing an inactive panel"
 ]

spec/guest-window-manager-spec.ts

@@ -1,6 +1,6 @@
 import { BrowserWindow, screen } from 'electron';
 import { expect, assert } from 'chai';
-import { areColorsSimilar, captureScreen, HexColors, getPixelColor } from './lib/screen-helpers';
+import { HexColors, ScreenCapture } from './lib/screen-helpers';
 import { ifit } from './lib/spec-helpers';
 import { closeAllWindows } from './lib/window-helpers';
 import { once } from 'node:events';
@@ -209,12 +209,8 @@ describe('webContents.setWindowOpenHandler', () => {
     childWindow.setBounds(display.bounds);
     await childWindow.webContents.executeJavaScript("const meta = document.createElement('meta'); meta.name = 'color-scheme'; meta.content = 'dark'; document.head.appendChild(meta); true;");
     await setTimeoutAsync(1000);
-    const screenCapture = await captureScreen();
+    const screenCapture = await ScreenCapture.createForDisplay(display);
-    const centerColor = getPixelColor(screenCapture, {
-      x: display.size.width / 2,
-      y: display.size.height / 2
-    });
     // color-scheme is set to dark so background should not be white
-    expect(areColorsSimilar(centerColor, HexColors.WHITE)).to.be.false();
+    await screenCapture.expectColorAtCenterDoesNotMatch(HexColors.WHITE);
   });
 });

spec/lib/artifacts.ts

@@ -0,0 +1,36 @@
+import path = require('node:path');
+import fs = require('node:fs/promises');
+import { randomBytes } from 'node:crypto';
+
+const IS_CI = !!process.env.CI;
+const ARTIFACT_DIR = path.join(__dirname, '..', 'artifacts');
+
+async function ensureArtifactDir (): Promise<void> {
+  if (!IS_CI) {
+    return;
+  }
+
+  await fs.mkdir(ARTIFACT_DIR, { recursive: true });
+}
+
+export async function createArtifact (
+  fileName: string,
+  data: Buffer
+): Promise<void> {
+  if (!IS_CI) {
+    return;
+  }
+
+  await ensureArtifactDir();
+  await fs.writeFile(path.join(ARTIFACT_DIR, fileName), data);
+}
+
+export async function createArtifactWithRandomId (
+  makeFileName: (id: string) => string,
+  data: Buffer
+): Promise<string> {
+  const randomId = randomBytes(12).toString('hex');
+  const fileName = makeFileName(randomId);
+  await createArtifact(fileName, data);
+  return fileName;
+}

spec/lib/screen-helpers.ts

@@ -1,60 +1,18 @@
-import * as path from 'node:path';
-import * as fs from 'node:fs';
 import { screen, desktopCapturer, NativeImage } from 'electron';
-
+import { createArtifactWithRandomId } from './artifacts';
-const fixtures = path.resolve(__dirname, '..', 'fixtures');
+import { AssertionError } from 'chai';
 
 export enum HexColors {
   GREEN = '#00b140',
   PURPLE = '#6a0dad',
   RED = '#ff0000',
   BLUE = '#0000ff',
-  WHITE = '#ffffff'
+  WHITE = '#ffffff',
-};
+}
 
-/**
+function hexToRgba (
- * Capture the screen at the given point.
+  hexColor: string
- *
+): [number, number, number, number] | undefined {
- * NOTE: Not yet supported on Linux in CI due to empty sources list.
- */
-export const captureScreen = async (point: Electron.Point = { x: 0, y: 0 }): Promise<NativeImage> => {
-  const display = screen.getDisplayNearestPoint(point);
-  const sources = await desktopCapturer.getSources({ types: ['screen'], thumbnailSize: display.size });
-  // Toggle to save screen captures for debugging.
-  const DEBUG_CAPTURE = process.env.DEBUG_CAPTURE || false;
-  if (DEBUG_CAPTURE) {
-    for (const source of sources) {
-      await fs.promises.writeFile(path.join(fixtures, `screenshot_${source.display_id}_${Date.now()}.png`), source.thumbnail.toPNG());
-    }
-  }
-  const screenCapture = sources.find(source => source.display_id === `${display.id}`);
-  // Fails when HDR is enabled on Windows.
-  // https://bugs.chromium.org/p/chromium/issues/detail?id=1247730
-  if (!screenCapture) {
-    const displayIds = sources.map(source => source.display_id);
-    throw new Error(`Unable to find screen capture for display '${display.id}'\n\tAvailable displays: ${displayIds.join(', ')}`);
-  }
-  return screenCapture.thumbnail;
-};
-
-const formatHexByte = (val: number): string => {
-  const str = val.toString(16);
-  return str.length === 2 ? str : `0${str}`;
-};
-
-/**
- * Get the hex color at the given pixel coordinate in an image.
- */
-export const getPixelColor = (image: Electron.NativeImage, point: Electron.Point): string => {
-  // image.crop crashes if point is fractional, so round to prevent that crash
-  const pixel = image.crop({ x: Math.round(point.x), y: Math.round(point.y), width: 1, height: 1 });
-  // TODO(samuelmaddock): NativeImage.toBitmap() should return the raw pixel
-  // color, but it sometimes differs. Why is that?
-  const [b, g, r] = pixel.toBitmap();
-  return `#${formatHexByte(r)}${formatHexByte(g)}${formatHexByte(b)}`;
-};
-
-const hexToRgba = (hexColor: string) => {
   const match = hexColor.match(/^#([0-9a-fA-F]{6,8})$/);
   if (!match) return;
 
@@ -63,34 +21,148 @@ const hexToRgba = (hexColor: string) => {
     parseInt(colorStr.substring(0, 2), 16),
     parseInt(colorStr.substring(2, 4), 16),
     parseInt(colorStr.substring(4, 6), 16),
-    parseInt(colorStr.substring(6, 8), 16) || 0xFF
+    parseInt(colorStr.substring(6, 8), 16) || 0xff
   ];
-};
+}
+
+function formatHexByte (val: number): string {
+  const str = val.toString(16);
+  return str.length === 2 ? str : `0${str}`;
+}
+
+/**
+ * Get the hex color at the given pixel coordinate in an image.
+ */
+function getPixelColor (
+  image: Electron.NativeImage,
+  point: Electron.Point
+): string {
+  // image.crop crashes if point is fractional, so round to prevent that crash
+  const pixel = image.crop({
+    x: Math.round(point.x),
+    y: Math.round(point.y),
+    width: 1,
+    height: 1
+  });
+  // TODO(samuelmaddock): NativeImage.toBitmap() should return the raw pixel
+  // color, but it sometimes differs. Why is that?
+  const [b, g, r] = pixel.toBitmap();
+  return `#${formatHexByte(r)}${formatHexByte(g)}${formatHexByte(b)}`;
+}
 
 /** Calculate euclidian distance between colors. */
-const colorDistance = (hexColorA: string, hexColorB: string) => {
+function colorDistance (hexColorA: string, hexColorB: string): number {
   const colorA = hexToRgba(hexColorA);
   const colorB = hexToRgba(hexColorB);
   if (!colorA || !colorB) return -1;
   return Math.sqrt(
     Math.pow(colorB[0] - colorA[0], 2) +
-    Math.pow(colorB[1] - colorA[1], 2) +
+      Math.pow(colorB[1] - colorA[1], 2) +
-    Math.pow(colorB[2] - colorA[2], 2)
+      Math.pow(colorB[2] - colorA[2], 2)
   );
-};
+}
 
 /**
  * Determine if colors are similar based on distance. This can be useful when
  * comparing colors which may differ based on lossy compression.
  */
-export const areColorsSimilar = (
+function areColorsSimilar (
   hexColorA: string,
   hexColorB: string,
   distanceThreshold = 90
-): boolean => {
+): boolean {
   const distance = colorDistance(hexColorA, hexColorB);
   return distance <= distanceThreshold;
-};
+}
+
+function imageCenter (image: NativeImage): Electron.Point {
+  const size = image.getSize();
+  return {
+    x: size.width / 2,
+    y: size.height / 2
+  };
+}
+/**
+ * Utilities for creating and inspecting a screen capture.
+ *
+ * NOTE: Not yet supported on Linux in CI due to empty sources list.
+ */
+export class ScreenCapture {
+  /** Use the async constructor `ScreenCapture.create()` instead. */
+  private constructor (image: NativeImage) {
+    this.image = image;
+  }
+
+  public static async create (): Promise<ScreenCapture> {
+    const display = screen.getPrimaryDisplay();
+    return ScreenCapture._createImpl(display);
+  }
+
+  public static async createForDisplay (
+    display: Electron.Display
+  ): Promise<ScreenCapture> {
+    return ScreenCapture._createImpl(display);
+  }
+
+  public async expectColorAtCenterMatches (hexColor: string) {
+    return this._expectImpl(imageCenter(this.image), hexColor, true);
+  }
+
+  public async expectColorAtCenterDoesNotMatch (hexColor: string) {
+    return this._expectImpl(imageCenter(this.image), hexColor, false);
+  }
+
+  public async expectColorAtPointOnDisplayMatches (
+    hexColor: string,
+    findPoint: (displaySize: Electron.Size) => Electron.Point
+  ) {
+    return this._expectImpl(findPoint(this.image.getSize()), hexColor, true);
+  }
+
+  private static async _createImpl (display: Electron.Display) {
+    const sources = await desktopCapturer.getSources({
+      types: ['screen'],
+      thumbnailSize: display.size
+    });
+
+    const captureSource = sources.find(
+      (source) => source.display_id === display.id.toString()
+    );
+    if (captureSource === undefined) {
+      const displayIds = sources.map((source) => source.display_id).join(', ');
+      throw new Error(
+        `Unable to find screen capture for display '${display.id}'\n\tAvailable displays: ${displayIds}`
+      );
+    }
+
+    return new ScreenCapture(captureSource.thumbnail);
+  }
+
+  private async _expectImpl (
+    point: Electron.Point,
+    expectedColor: string,
+    matchIsExpected: boolean
+  ) {
+    const actualColor = getPixelColor(this.image, point);
+    const colorsMatch = areColorsSimilar(expectedColor, actualColor);
+    const gotExpectedResult = matchIsExpected ? colorsMatch : !colorsMatch;
+
+    if (!gotExpectedResult) {
+      // Save the image as an artifact for better debugging
+      const artifactName = await createArtifactWithRandomId(
+        (id) => `color-mismatch-${id}.png`,
+        this.image.toPNG()
+      );
+      throw new AssertionError(
+        `Expected color at (${point.x}, ${point.y}) to ${
+          matchIsExpected ? 'match' : '*not* match'
+        } '${expectedColor}', but got '${actualColor}'. See the artifact '${artifactName}' for more information.`
+      );
+    }
+  }
+
+  private image: NativeImage;
+}
 
 /**
  * Whether the current VM has a valid screen which can be used to capture.
@@ -101,6 +173,8 @@ export const areColorsSimilar = (
  * - Win32 ia32: skipped
  */
 export const hasCapturableScreen = () => {
-  return process.platform === 'darwin' ||
+  return (
-    (process.platform === 'win32' && process.arch === 'x64');
+    process.platform === 'darwin' ||
+    (process.platform === 'win32' && process.arch === 'x64')
+  );
 };

spec/webview-spec.ts

@@ -1,6 +1,6 @@
 import * as path from 'node:path';
 import * as url from 'node:url';
-import { BrowserWindow, session, ipcMain, app, WebContents, screen } from 'electron/main';
+import { BrowserWindow, session, ipcMain, app, WebContents } from 'electron/main';
 import { closeAllWindows } from './lib/window-helpers';
 import { emittedUntil } from './lib/events-helpers';
 import { ifit, ifdescribe, defer, itremote, useRemoteContext, listen } from './lib/spec-helpers';
@@ -9,7 +9,7 @@ import * as http from 'node:http';
 import * as auth from 'basic-auth';
 import { once } from 'node:events';
 import { setTimeout } from 'node:timers/promises';
-import { areColorsSimilar, captureScreen, HexColors, getPixelColor } from './lib/screen-helpers';
+import { HexColors, ScreenCapture } from './lib/screen-helpers';
 
 declare let WebView: any;
 const features = process._linkedBinding('electron_common_features');
@@ -804,14 +804,8 @@ describe('<webview> tag', function () {
 
       await setTimeout(1000);
 
-      const display = screen.getPrimaryDisplay();
+      const screenCapture = await ScreenCapture.create();
-      const screenCapture = await captureScreen();
+      await screenCapture.expectColorAtCenterMatches(WINDOW_BACKGROUND_COLOR);
-      const centerColor = getPixelColor(screenCapture, {
-        x: display.size.width / 2,
-        y: display.size.height / 2
-      });
-
-      expect(areColorsSimilar(centerColor, WINDOW_BACKGROUND_COLOR)).to.be.true();
     });
 
     // Linux and arm64 platforms (WOA and macOS) do not return any capture sources
@@ -823,14 +817,8 @@ describe('<webview> tag', function () {
 
       await setTimeout(1000);
 
-      const display = screen.getPrimaryDisplay();
+      const screenCapture = await ScreenCapture.create();
-      const screenCapture = await captureScreen();
+      await screenCapture.expectColorAtCenterMatches(WINDOW_BACKGROUND_COLOR);
-      const centerColor = getPixelColor(screenCapture, {
-        x: display.size.width / 2,
-        y: display.size.height / 2
-      });
-
-      expect(areColorsSimilar(centerColor, WINDOW_BACKGROUND_COLOR)).to.be.true();
     });
 
     // Linux and arm64 platforms (WOA and macOS) do not return any capture sources
@@ -842,14 +830,8 @@ describe('<webview> tag', function () {
 
       await setTimeout(1000);
 
-      const display = screen.getPrimaryDisplay();
+      const screenCapture = await ScreenCapture.create();
-      const screenCapture = await captureScreen();
+      await screenCapture.expectColorAtCenterMatches(HexColors.WHITE);
-      const centerColor = getPixelColor(screenCapture, {
-        x: display.size.width / 2,
-        y: display.size.height / 2
-      });
-
-      expect(areColorsSimilar(centerColor, HexColors.WHITE)).to.be.true();
     });
   });