test: migrate to helpers & disabled tests list (electron#37513)

* test: migrate to helpers & disabled tests list

* can't disable a test suite

* correct condition

* address review comments

Author: clavin

spec/api-app-spec.ts

@@ -188,12 +188,7 @@ describe('app module', () => {
       expect(code).to.equal(123, 'exit code should be 123, if you see this please tag @MarshallOfSound');
     });
 
-    it('exits gracefully', async function () {
-      if (!['darwin', 'linux'].includes(process.platform)) {
-        this.skip();
-        return;
-      }
-
+    ifit(['darwin', 'linux'].includes(process.platform))('exits gracefully', async function () {
       const electronPath = process.execPath;
       const appPath = path.join(fixturesPath, 'api', 'singleton');
       appProcess = cp.spawn(electronPath, [appPath]);
@@ -409,13 +404,7 @@ describe('app module', () => {
     });
   });
 
-  describe('app.setUserActivity(type, userInfo)', () => {
-    before(function () {
-      if (process.platform !== 'darwin') {
-        this.skip();
-      }
-    });
-
+  ifdescribe(process.platform === 'darwin')('app.setUserActivity(type, userInfo)', () => {
     it('sets the current activity', () => {
       app.setUserActivity('com.electron.testActivity', { testData: '123' });
       expect(app.getCurrentActivityType()).to.equal('com.electron.testActivity');
@@ -737,9 +726,7 @@ describe('app module', () => {
       expect(app.getLoginItemSettings().openAtLogin).to.equal(false);
     });
 
-    it('correctly sets and unsets the LoginItem as hidden', function () {
-      if (process.platform !== 'darwin') this.skip();
-
+    ifit(process.platform === 'darwin')('correctly sets and unsets the LoginItem as hidden', function () {
       expect(app.getLoginItemSettings().openAtLogin).to.equal(false);
       expect(app.getLoginItemSettings().openAsHidden).to.equal(false);
 
@@ -1099,13 +1086,10 @@ describe('app module', () => {
     });
   });
 
-  describe('select-client-certificate event', () => {
+  ifdescribe(process.platform !== 'linux')('select-client-certificate event', () => {
     let w: BrowserWindow;
 
     before(function () {
-      if (process.platform === 'linux') {
-        this.skip();
-      }
       session.fromPartition('empty-certificate').setCertificateVerifyProc((req, cb) => { cb(0); });
     });
 
@@ -1134,7 +1118,7 @@ describe('app module', () => {
     });
   });
 
-  describe('setAsDefaultProtocolClient(protocol, path, args)', () => {
+  ifdescribe(process.platform === 'win32')('setAsDefaultProtocolClient(protocol, path, args)', () => {
     const protocol = 'electron-test';
     const updateExe = path.resolve(path.dirname(process.execPath), '..', 'Update.exe');
     const processStartArgs = [
@@ -1146,16 +1130,12 @@ describe('app module', () => {
     let classesKey: any;
 
     before(function () {
-      if (process.platform !== 'win32') {
-        this.skip();
-      } else {
-        Winreg = require('winreg');
+      Winreg = require('winreg');
 
-        classesKey = new Winreg({
-          hive: Winreg.HKCU,
-          key: '\\Software\\Classes\\'
-        });
-      }
+      classesKey = new Winreg({
+        hive: Winreg.HKCU,
+        key: '\\Software\\Classes\\'
+      });
     });
 
     after(function (done) {
@@ -1240,14 +1220,11 @@ describe('app module', () => {
   });
 
   describe('getApplicationNameForProtocol()', () => {
-    it('returns application names for common protocols', function () {
+    // TODO: Linux CI doesn't have registered http & https handlers
+    ifit(!(process.env.CI && process.platform === 'linux'))('returns application names for common protocols', function () {
       // We can't expect particular app names here, but these protocols should
       // at least have _something_ registered. Except on our Linux CI
       // environment apparently.
-      if (process.platform === 'linux') {
-        this.skip();
-      }
-
       const protocols = [
         'http://',
         'https://'
@@ -1492,28 +1469,25 @@ describe('app module', () => {
     });
   });
 
-  describe('sandbox options', () => {
+  ifdescribe(!(process.platform === 'linux' && (process.arch === 'arm64' || process.arch === 'arm')))('sandbox options', () => {
+    // Our ARM tests are run on VSTS rather than CircleCI, and the Docker
+    // setup on VSTS disallows syscalls that Chrome requires for setting up
+    // sandboxing.
+    // See:
+    // - https://docs.docker.com/engine/security/seccomp/#significant-syscalls-blocked-by-the-default-profile
+    // - https://chromium.googlesource.com/chromium/src/+/70.0.3538.124/sandbox/linux/services/credentials.cc#292
+    // - https://github.com/docker/docker-ce/blob/ba7dfc59ccfe97c79ee0d1379894b35417b40bca/components/engine/profiles/seccomp/seccomp_default.go#L497
+    // - https://blog.jessfraz.com/post/how-to-use-new-docker-seccomp-profiles/
+    //
+    // Adding `--cap-add SYS_ADMIN` or `--security-opt seccomp=unconfined`
+    // to the Docker invocation allows the syscalls that Chrome needs, but
+    // are probably more permissive than we'd like.
+
     let appProcess: cp.ChildProcess = null as any;
     let server: net.Server = null as any;
     const socketPath = process.platform === 'win32' ? '\\\\.\\pipe\\electron-mixed-sandbox' : '/tmp/electron-mixed-sandbox';
 
     beforeEach(function (done) {
-      if (process.platform === 'linux' && (process.arch === 'arm64' || process.arch === 'arm')) {
-        // Our ARM tests are run on VSTS rather than CircleCI, and the Docker
-        // setup on VSTS disallows syscalls that Chrome requires for setting up
-        // sandboxing.
-        // See:
-        // - https://docs.docker.com/engine/security/seccomp/#significant-syscalls-blocked-by-the-default-profile
-        // - https://chromium.googlesource.com/chromium/src/+/70.0.3538.124/sandbox/linux/services/credentials.cc#292
-        // - https://github.com/docker/docker-ce/blob/ba7dfc59ccfe97c79ee0d1379894b35417b40bca/components/engine/profiles/seccomp/seccomp_default.go#L497
-        // - https://blog.jessfraz.com/post/how-to-use-new-docker-seccomp-profiles/
-        //
-        // Adding `--cap-add SYS_ADMIN` or `--security-opt seccomp=unconfined`
-        // to the Docker invocation allows the syscalls that Chrome needs, but
-        // are probably more permissive than we'd like.
-        this.skip();
-        return;
-      }
       fs.unlink(socketPath, () => {
         server = net.createServer();
         server.listen(socketPath);
@@ -1613,8 +1587,7 @@ describe('app module', () => {
     });
   });
 
-  const dockDescribe = process.platform === 'darwin' ? describe : describe.skip;
-  dockDescribe('dock APIs', () => {
+  ifdescribe(process.platform === 'darwin')('dock APIs', () => {
     after(async () => {
       await app.dock.show();
     });

spec/api-autoupdater-darwin-spec.ts

@@ -29,6 +29,7 @@ ifdescribe(process.platform === 'darwin' && !(process.env.CI && process.arch ===
       if (process.env.CI && !process.env.CIRCLE_PR_NUMBER) {
         throw new Error('No valid signing identity available to run autoUpdater specs');
       }
+
       this.skip();
     } else {
       identity = result.stdout.toString().trim();

spec/api-browser-window-spec.ts

@@ -359,9 +359,9 @@ describe('BrowserWindow module', () => {
       w.loadURL('about:blank');
       await readyToShow;
     });
-    // TODO(deepak1556): The error code now seems to be `ERR_FAILED`, verify what
+    // DISABLED-FIXME(deepak1556): The error code now seems to be `ERR_FAILED`, verify what
     // changed and adjust the test.
-    it.skip('should emit did-fail-load event for files that do not exist', async () => {
+    it('should emit did-fail-load event for files that do not exist', async () => {
       const didFailLoad = once(w.webContents, 'did-fail-load');
       w.loadURL('file://a.txt');
       const [, code, desc,, isMainFrame] = await didFailLoad;
@@ -4083,9 +4083,9 @@ describe('BrowserWindow module', () => {
       }
     });
 
-    // FIXME(MarshallOfSound): This test fails locally 100% of the time, on CI it started failing
+    // DISABLED-FIXME(MarshallOfSound): This test fails locally 100% of the time, on CI it started failing
     // when we introduced the compositor recycling patch.  Should figure out how to fix this
-    it.skip('visibilityState remains visible if backgroundThrottling is disabled', async () => {
+    it('visibilityState remains visible if backgroundThrottling is disabled', async () => {
       const w = new BrowserWindow({
         show: false,
         width: 100,

spec/api-media-handler-spec.ts

@@ -23,14 +23,16 @@ ifdescribe(features.isDesktopCapturerEnabled())('setDisplayMediaRequestHandler',
     server.close();
   });
 
-  // NOTE(nornagon): this test fails on our macOS CircleCI runners with the
+  // FIXME(nornagon): this test fails on our macOS CircleCI runners with the
   // error message:
   // [ERROR:video_capture_device_client.cc(659)] error@ OnStart@content/browser/media/capture/desktop_capture_device_mac.cc:98, CGDisplayStreamCreate failed, OS message: Value too large to be stored in data type (84)
   // This is possibly related to the OS/VM setup that CircleCI uses for macOS.
   // Our arm64 runners are in @jkleinsc's office, and are real machines, so the
   // test works there.
   ifit(!(process.platform === 'darwin' && process.arch === 'x64'))('works when calling getDisplayMedia', async function () {
-    if ((await desktopCapturer.getSources({ types: ['screen'] })).length === 0) { return this.skip(); }
+    if ((await desktopCapturer.getSources({ types: ['screen'] })).length === 0) {
+      return this.skip();
+    }
     const ses = session.fromPartition('' + Math.random());
     let requestHandlerCalled = false;
     let mediaRequest: any = null;

spec/api-menu-item-spec.ts

@@ -1,5 +1,6 @@
 import { BrowserWindow, app, Menu, MenuItem, MenuItemConstructorOptions } from 'electron/main';
 import { expect } from 'chai';
+import { ifdescribe } from './lib/spec-helpers';
 import { closeAllWindows } from './lib/window-helpers';
 import { roleList, execute } from '../lib/browser/api/menu-item-roles';
 
@@ -280,13 +281,7 @@ describe('MenuItems', () => {
     });
   });
 
-  describe('MenuItem appMenu', () => {
-    before(function () {
-      if (process.platform !== 'darwin') {
-        this.skip();
-      }
-    });
-
+  ifdescribe(process.platform === 'darwin')('MenuItem appMenu', () => {
     it('includes a default submenu layout when submenu is empty', () => {
       const item = new MenuItem({ role: 'appMenu' });
 

spec/api-menu-spec.ts

@@ -891,8 +891,8 @@ describe('Menu module', function () {
       expect(Menu.getApplicationMenu()).to.not.be.null('application menu');
     });
 
-    // TODO(nornagon): this causes the focus handling tests to fail
-    it.skip('unsets a menu with null', () => {
+    // DISABLED-FIXME(nornagon): this causes the focus handling tests to fail
+    it('unsets a menu with null', () => {
       Menu.setApplicationMenu(null);
       expect(Menu.getApplicationMenu()).to.be.null('application menu');
     });

spec/api-process-spec.ts

@@ -204,9 +204,9 @@ describe('process module', () => {
     });
 
     describe('process.takeHeapSnapshot()', () => {
-      // TODO(nornagon): this seems to take a really long time when run in the
+      // DISABLED-FIXME(nornagon): this seems to take a really long time when run in the
       // main process, for unknown reasons.
-      it.skip('returns true on success', () => {
+      it('returns true on success', () => {
         const filePath = path.join(app.getPath('temp'), 'test.heapsnapshot');
         defer(() => {
           try {

spec/api-protocol-spec.ts

@@ -282,7 +282,8 @@ describe('protocol module', () => {
         ipcMain.once('loaded-iframe-custom-protocol', () => done());
       });
 
-      it.skip('throws an error when custom headers are invalid', (done) => {
+      // DISABLED-FIXME
+      it('throws an error when custom headers are invalid', (done) => {
         registerFileProtocol(protocolName, (request, callback) => {
           expect(() => callback({
             path: filePath,
@@ -879,7 +880,8 @@ describe('protocol module', () => {
       await requestReceived;
     });
 
-    it.skip('can access files through the FileSystem API', (done) => {
+    // DISABLED-FIXME
+    it('can access files through the FileSystem API', (done) => {
       const filePath = path.join(fixturesPath, 'pages', 'filesystem.html');
       protocol.registerFileProtocol(standardScheme, (request, callback) => callback({ path: filePath }));
       w.loadURL(origin);
@@ -928,8 +930,8 @@ describe('protocol module', () => {
       });
     });
 
-    // FIXME: Figure out why this test is failing
-    it.skip('disallows CORS and fetch requests when only supportFetchAPI is specified', async () => {
+    // DISABLED-FIXME: Figure out why this test is failing
+    it('disallows CORS and fetch requests when only supportFetchAPI is specified', async () => {
       await allowsCORSRequests('no-cors', ['failed xhr', 'failed fetch'], /has been blocked by CORS policy/, () => {
         const { ipcRenderer } = require('electron');
         Promise.all([

spec/api-screen-spec.ts

@@ -1,5 +1,6 @@
 import { expect } from 'chai';
 import { screen } from 'electron/main';
+import { ifit } from './lib/spec-helpers';
 
 describe('screen module', () => {
   describe('methods reassignment', () => {
@@ -28,8 +29,7 @@ describe('screen module', () => {
       expect(display).to.be.an('object');
     });
 
-    it('has the correct non-object properties', function () {
-      if (process.platform === 'linux') this.skip();
+    ifit(process.platform !== 'linux')('has the correct non-object properties', function () {
       const display = screen.getPrimaryDisplay();
 
       expect(display).to.have.property('scaleFactor').that.is.a('number');
@@ -46,8 +46,7 @@ describe('screen module', () => {
       expect(display).to.have.property('displayFrequency').that.is.a('number');
     });
 
-    it('has a size object property', function () {
-      if (process.platform === 'linux') this.skip();
+    ifit(process.platform !== 'linux')('has a size object property', function () {
       const display = screen.getPrimaryDisplay();
 
       expect(display).to.have.property('size').that.is.an('object');
@@ -56,8 +55,7 @@ describe('screen module', () => {
       expect(size).to.have.property('height').that.is.greaterThan(0);
     });
 
-    it('has a workAreaSize object property', function () {
-      if (process.platform === 'linux') this.skip();
+    ifit(process.platform !== 'linux')('has a workAreaSize object property', function () {
       const display = screen.getPrimaryDisplay();
 
       expect(display).to.have.property('workAreaSize').that.is.an('object');
@@ -66,8 +64,7 @@ describe('screen module', () => {
       expect(workAreaSize).to.have.property('height').that.is.greaterThan(0);
     });
 
-    it('has a bounds object property', function () {
-      if (process.platform === 'linux') this.skip();
+    ifit(process.platform !== 'linux')('has a bounds object property', function () {
       const display = screen.getPrimaryDisplay();
 
       expect(display).to.have.property('bounds').that.is.an('object');

spec/api-session-spec.ts

@@ -172,7 +172,8 @@ describe('session module', () => {
       expect(list.some(cookie => cookie.name === name && cookie.value === value)).to.equal(false);
     });
 
-    it.skip('should set cookie for standard scheme', async () => {
+    // DISABLED-FIXME
+    it('should set cookie for standard scheme', async () => {
       const { cookies } = session.defaultSession;
       const domain = 'fake-host';
       const url = `${standardScheme}://${domain}`;

spec/api-web-contents-spec.ts

@@ -5,7 +5,7 @@ import * as fs from 'fs';
 import * as http from 'http';
 import { BrowserWindow, ipcMain, webContents, session, app, BrowserView } from 'electron/main';
 import { closeAllWindows } from './lib/window-helpers';
-import { ifdescribe, defer, waitUntil, listen } from './lib/spec-helpers';
+import { ifdescribe, defer, waitUntil, listen, ifit } from './lib/spec-helpers';
 import { once } from 'events';
 import { setTimeout } from 'timers/promises';
 
@@ -372,10 +372,9 @@ describe('webContents module', () => {
         .and.have.property('code', 'ERR_FILE_NOT_FOUND');
     });
 
-    // Temporarily disable on WOA until
+    // FIXME: Temporarily disable on WOA until
     // https://github.com/electron/electron/issues/20008 is resolved
-    const testFn = (process.platform === 'win32' && process.arch === 'arm64' ? it.skip : it);
-    testFn('rejects when loading fails due to DNS not resolved', async () => {
+    ifit(!(process.platform === 'win32' && process.arch === 'arm64'))('rejects when loading fails due to DNS not resolved', async () => {
       await expect(w.loadURL('https://err.name.not.resolved')).to.eventually.be.rejected()
         .and.have.property('code', 'ERR_NAME_NOT_RESOLVED');
     });
@@ -489,8 +488,8 @@ describe('webContents module', () => {
   describe('getFocusedWebContents() API', () => {
     afterEach(closeAllWindows);
 
-    const testFn = (process.platform === 'win32' && process.arch === 'arm64' ? it.skip : it);
-    testFn('returns the focused web contents', async () => {
+    // FIXME
+    ifit(!(process.platform === 'win32' && process.arch === 'arm64'))('returns the focused web contents', async () => {
       const w = new BrowserWindow({ show: true });
       await w.loadFile(path.join(__dirname, 'fixtures', 'blank.html'));
       expect(webContents.getFocusedWebContents().id).to.equal(w.webContents.id);

spec/api-web-frame-main-spec.ts

@@ -185,9 +185,8 @@ describe('webFrameMain module', () => {
   });
 
   describe('WebFrame.visibilityState', () => {
-    // TODO(MarshallOfSound): Fix flaky test
-    // @flaky-test
-    it.skip('should match window state', async () => {
+    // DISABLED-FIXME(MarshallOfSound): Fix flaky test
+    it('should match window state', async () => {
       const w = new BrowserWindow({ show: true });
       await w.loadURL('about:blank');
       const webFrame = w.webContents.mainFrame;

spec/asar-spec.ts

@@ -1410,8 +1410,8 @@ describe('asar package', function () {
         expect(process.noAsar).to.be.false();
       });
     });
-    /*
 
+    /*
     describe('process.env.ELECTRON_NO_ASAR', function () {
       before(function () {
         if (!features.isRunAsNodeEnabled()) {