Browse Source

test: there is only --ci (#20794)

Jeremy Apthorp 5 years ago
parent
commit
6781d5e3c8

+ 1 - 1
.circleci/config.yml

@@ -1178,7 +1178,7 @@ steps-tests: &steps-tests
         command: |
           cd src
           export ELECTRON_OUT_DIR=Default
-          (cd electron && node script/yarn test -- --ci --enable-logging)
+          (cd electron && node script/yarn test -- --enable-logging)
     - run:
         name: Check test results existence
         command: |

+ 1 - 1
appveyor.yml

@@ -133,7 +133,7 @@ test_script:
         echo "Skipping tests for $env:GN_CONFIG build"
       }
   - cd electron
-  - if "%RUN_TESTS%"=="true" ( echo Running test suite & node script/yarn test -- --ci --enable-logging)
+  - if "%RUN_TESTS%"=="true" ( echo Running test suite & node script/yarn test -- --enable-logging)
   - cd ..
   - if "%RUN_TESTS%"=="true" ( 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"

+ 2 - 2
azure-pipelines-woa.yml

@@ -63,7 +63,7 @@ steps:
     set npm_config_nodedir=%cd%\out\Default\gen\node_headers
     set npm_config_arch=arm64
     cd electron
-    node script/yarn test -- --ci --enable-logging --verbose
+    node script/yarn test -- --enable-logging --verbose
   displayName: 'Run Electron tests'
   env:
     ELECTRON_OUT_DIR: Default
@@ -89,4 +89,4 @@ steps:
     Get-Process | Where Name –Like "electron*" | Stop-Process
     Get-Process | Where Name –Like "MicrosoftEdge*" | Stop-Process
   displayName: 'Kill processes left running from last test run'
-  condition: always()
+  condition: always()

+ 1 - 1
docs/development/build-instructions-gn.md

@@ -236,7 +236,7 @@ the Electron binary:
 
 ```sh
 $ ./out/Debug/Electron.app/Contents/MacOS/Electron electron/spec \
-  --ci --enable-logging -g 'BrowserWindow module'
+  --enable-logging -g 'BrowserWindow module'
 ```
 
 ## Sharing the git cache between multiple machines

+ 2 - 12
shell/app/atom_main.cc

@@ -102,18 +102,8 @@ int APIENTRY wWinMain(HINSTANCE instance, HINSTANCE, wchar_t* cmd, int) {
 
 #ifdef _DEBUG
   // Don't display assert dialog boxes in CI test runs
-  static const char* kCI = "ELECTRON_CI";
-  bool is_ci = IsEnvSet(kCI);
-  if (!is_ci) {
-    for (int i = 0; i < arguments.argc; ++i) {
-      if (!_wcsicmp(arguments.argv[i], L"--ci")) {
-        is_ci = true;
-        _putenv_s(kCI, "1");  // set flag for child processes
-        break;
-      }
-    }
-  }
-  if (is_ci) {
+  static const char* kCI = "CI";
+  if (IsEnvSet(kCI)) {
     _CrtSetReportMode(_CRT_ERROR, _CRTDBG_MODE_DEBUG | _CRTDBG_MODE_FILE);
     _CrtSetReportFile(_CRT_ERROR, _CRTDBG_FILE_STDERR);
 

+ 0 - 1
spec-main/ambient.d.ts

@@ -1,4 +1,3 @@
-declare var isCI: boolean;
 declare var standardScheme: string;
 
 declare namespace Electron {

+ 6 - 45
spec-main/api-app-spec.ts

@@ -5,7 +5,6 @@ import * as https from 'https'
 import * as net from 'net'
 import * as fs from 'fs'
 import * as path from 'path'
-import { homedir } from 'os'
 import split = require('split')
 import { app, BrowserWindow, Menu } from 'electron'
 import { emittedOnce } from './events-helpers';
@@ -129,7 +128,7 @@ describe('app module', () => {
   describe('app.getLocaleCountryCode()', () => {
     it('should be empty or have length of two', () => {
       let expectedLength = 2
-      if (isCI && process.platform === 'linux') {
+      if (process.platform === 'linux') {
         // Linux CI machines have no locale.
         expectedLength = 0
       }
@@ -143,13 +142,7 @@ describe('app module', () => {
     })
   })
 
-  describe('app.isInApplicationsFolder()', () => {
-    before(function () {
-      if (process.platform !== 'darwin') {
-        this.skip()
-      }
-    })
-
+  ifdescribe(process.platform === 'darwin')('app.isInApplicationsFolder()', () => {
     it('should be false during tests', () => {
       expect(app.isInApplicationsFolder()).to.equal(false)
     })
@@ -679,30 +672,6 @@ describe('app module', () => {
     })
   })
 
-  describe('getPath("logs")', () => {
-    const logsPaths = {
-      'darwin': path.resolve(homedir(), 'Library', 'Logs'),
-      'linux': path.resolve(homedir(), 'AppData', app.name),
-      'win32': path.resolve(homedir(), 'AppData', app.name),
-    }
-
-    it('has no logs directory by default', () => {
-      // this won't be deterministic except on CI since
-      // users may or may not have this dir
-      if (!isCI) return
-
-      const osLogPath = (logsPaths as any)[process.platform]
-      expect(fs.existsSync(osLogPath)).to.be.false
-    })
-
-    it('creates a new logs directory if one does not exist', () => {
-      expect(() => { app.getPath('logs') }).to.not.throw()
-
-      const osLogPath = (logsPaths as any)[process.platform]
-      expect(fs.existsSync(osLogPath)).to.be.true
-    })
-  })
-
   describe('getPath(name)', () => {
     it('returns paths that exist', () => {
       const paths = [
@@ -729,9 +698,9 @@ describe('app module', () => {
     it('does not create a new directory by default', () => {
       const badPath = path.join(__dirname, 'music')
 
-      expect(fs.existsSync(badPath)).to.be.false
+      expect(fs.existsSync(badPath)).to.be.false()
       app.setPath('music', badPath)
-      expect(fs.existsSync(badPath)).to.be.false
+      expect(fs.existsSync(badPath)).to.be.false()
 
       expect(() => { app.getPath(badPath as any) }).to.throw()
     })
@@ -923,7 +892,8 @@ describe('app module', () => {
     })
   })
 
-  describe('getFileIcon() API', () => {
+  // FIXME Get these specs running on Linux CI
+  ifdescribe(process.platform !== 'linux')('getFileIcon() API', () => {
     const iconPath = path.join(__dirname, 'fixtures/assets/icon.ico')
     const sizes = {
       small: 16,
@@ -931,15 +901,6 @@ describe('app module', () => {
       large: process.platform === 'win32' ? 32 : 48
     }
 
-    // (alexeykuzmin): `.skip()` called in `before`
-    // doesn't affect nested `describe`s.
-    beforeEach(function () {
-      // FIXME Get these specs running on Linux CI
-      if (process.platform === 'linux' && isCI) {
-        this.skip()
-      }
-    })
-
     it('fetches a non-empty icon', async () => {
       const icon = await app.getFileIcon(iconPath)
       expect(icon.isEmpty()).to.equal(false)

+ 1 - 1
spec-main/api-autoupdater-darwin-spec.ts

@@ -24,7 +24,7 @@ describeFn('autoUpdater behavior', function () {
     if (result.status !== 0 || result.stdout.toString().trim().length === 0)  {
       // Per https://circleci.com/docs/2.0/env-vars:
       // CIRCLE_PR_NUMBER is only present on forked PRs
-      if (isCI && !process.env.CIRCLE_PR_NUMBER) {
+      if (process.env.CI && !process.env.CIRCLE_PR_NUMBER) {
         throw new Error('No valid signing identity available to run autoUpdater specs')
       }
       this.skip()

+ 2 - 9
spec-main/api-browser-window-spec.ts

@@ -535,14 +535,7 @@ describe('BrowserWindow module', () => {
     describe('BrowserWindow.show()', () => {
       it('should focus on window', () => {
         w.show()
-        if (process.platform === 'darwin' && !isCI) {
-          // on CI, the Electron window will be the only one open, so it'll get
-          // focus. on not-CI, some other window will have focus, and we don't
-          // steal focus any more, so we expect isFocused to be false.
-          expect(w.isFocused()).to.equal(false)
-        } else {
-          expect(w.isFocused()).to.equal(true)
-        }
+        expect(w.isFocused()).to.equal(true)
       })
       it('should make the window visible', () => {
         w.show()
@@ -2514,7 +2507,7 @@ describe('BrowserWindow module', () => {
       expect(visibilityState).to.equal('visible')
     })
 
-    ifit(!(isCI && process.platform === 'win32'))('visibilityState changes when window is shown inactive', async () => {
+    ifit(process.platform !== 'win32')('visibilityState changes when window is shown inactive', async () => {
       const w = new BrowserWindow({
         width: 100,
         height: 100,

+ 2 - 7
spec-main/api-global-shortcut-spec.ts

@@ -1,13 +1,8 @@
 import { expect } from 'chai'
 import { globalShortcut } from 'electron'
+import { ifdescribe } from './spec-helpers'
 
-describe('globalShortcut module', () => {
-  before(function () {
-    if (isCI && process.platform === 'win32') {
-      this.skip()
-    }
-  })
-
+ifdescribe(process.platform !== 'win32')('globalShortcut module', () => {
   beforeEach(() => {
     globalShortcut.unregisterAll()
   })

+ 1 - 94
spec-main/api-menu-spec.ts

@@ -1,5 +1,5 @@
 import { expect } from 'chai'
-import { BrowserWindow, globalShortcut, Menu, MenuItem } from 'electron'
+import { BrowserWindow, Menu, MenuItem } from 'electron'
 import { sortMenuItems } from '../lib/browser/api/menu-utils'
 import { closeWindow } from './window-helpers'
 
@@ -840,97 +840,4 @@ describe('Menu module', function () {
       expect(Menu.getApplicationMenu()).to.be.null('application menu')
     })
   })
-
-  describe('menu accelerators', async () => {
-    const sendRobotjsKey = (key: string, modifiers: string | string[] = [], delay = 500) => {
-      return new Promise((resolve, reject) => {
-        try {
-          require('robotjs').keyTap(key, modifiers)
-          setTimeout(() => {
-            resolve()
-          }, delay)
-        } catch (e) {
-          reject(e)
-        }
-      })
-    }
-
-    before(async function () {
-      // --ci flag breaks accelerator and robotjs interaction
-      if (isCI) {
-        this.skip()
-      }
-
-      // before accelerator tests, use globalShortcut to test if
-      // RobotJS is working at all
-      let isKeyPressed = false
-      globalShortcut.register('q', () => {
-        isKeyPressed = true
-      })
-      try {
-        await sendRobotjsKey('q')
-      } catch (e) {
-        this.skip()
-      }
-
-      if (!isKeyPressed) {
-        this.skip()
-      }
-
-      globalShortcut.unregister('q')
-    })
-
-    it('should perform the specified action', async () => {
-      let hasBeenClicked = false
-      const menu = Menu.buildFromTemplate([
-        {
-          label: 'Test',
-          submenu: [
-            {
-              label: 'Test Item',
-              accelerator: 'T',
-              click: (a, b, event) => {
-                hasBeenClicked = true
-                expect(event).to.deep.equal({
-                  shiftKey: false,
-                  ctrlKey: false,
-                  altKey: false,
-                  metaKey: false,
-                  triggeredByAccelerator: true
-                })
-              },
-              id: 'test'
-            }
-          ]
-        }
-      ])
-      Menu.setApplicationMenu(menu)
-      expect(Menu.getApplicationMenu()).to.not.be.null('application menu')
-      await sendRobotjsKey('t')
-      expect(hasBeenClicked).to.equal(true)
-    })
-
-    it('should not activate upon clicking another key combination', async () => {
-      let hasBeenClicked = false
-      const menu = Menu.buildFromTemplate([
-        {
-          label: 'Test',
-          submenu: [
-            {
-              label: 'Test Item',
-              accelerator: 'T',
-              click: (a, b, event) => {
-                hasBeenClicked = true
-              },
-              id: 'test'
-            }
-          ]
-        }
-      ])
-      Menu.setApplicationMenu(menu)
-      expect(Menu.getApplicationMenu()).to.not.be.null('application menu')
-      await sendRobotjsKey('t', 'shift')
-      expect(hasBeenClicked).to.equal(false)
-    })
-  })
 })

+ 12 - 31
spec-main/api-net-log-spec.ts

@@ -6,6 +6,8 @@ import * as path from 'path'
 import * as ChildProcess from 'child_process'
 import { session, net } from 'electron'
 import { Socket, AddressInfo } from 'net';
+import { ifit } from './spec-helpers'
+import { emittedOnce } from './events-helpers'
 
 const appPath = path.join(__dirname, 'fixtures', 'api', 'net-log')
 const dumpFile = path.join(os.tmpdir(), 'net_log.json')
@@ -121,12 +123,7 @@ describe('netLog module', () => {
     expect(JSON.parse(dump).events.some((x: any) => x.params && x.params.bytes && Buffer.from(x.params.bytes, 'base64').includes(unique))).to.be.true('uuid present in dump')
   })
 
-  it('should begin and end logging automatically when --log-net-log is passed', done => {
-    if (isCI && process.platform === 'linux') {
-      done()
-      return
-    }
-
+  ifit(process.platform !== 'linux')('should begin and end logging automatically when --log-net-log is passed', async () => {
     const appProcess = ChildProcess.spawn(process.execPath,
       [appPath], {
         env: {
@@ -135,18 +132,11 @@ describe('netLog module', () => {
         }
       })
 
-    appProcess.once('exit', () => {
-      expect(fs.existsSync(dumpFile)).to.be.true('dump file exists')
-      done()
-    })
+    await emittedOnce(appProcess, 'exit')
+    expect(fs.existsSync(dumpFile)).to.be.true('dump file exists')
   })
 
-  it('should begin and end logging automtically when --log-net-log is passed, and behave correctly when .startLogging() and .stopLogging() is called', done => {
-    if (isCI && process.platform === 'linux') {
-      done()
-      return
-    }
-
+  ifit(process.platform !== 'linux')('should begin and end logging automtically when --log-net-log is passed, and behave correctly when .startLogging() and .stopLogging() is called', async () => {
     const appProcess = ChildProcess.spawn(process.execPath,
       [appPath], {
         env: {
@@ -157,19 +147,12 @@ describe('netLog module', () => {
         }
       })
 
-    appProcess.once('exit', () => {
-      expect(fs.existsSync(dumpFile)).to.be.true('dump file exists')
-      expect(fs.existsSync(dumpFileDynamic)).to.be.true('dynamic dump file exists')
-      done()
-    })
+    await emittedOnce(appProcess, 'exit')
+    expect(fs.existsSync(dumpFile)).to.be.true('dump file exists')
+    expect(fs.existsSync(dumpFileDynamic)).to.be.true('dynamic dump file exists')
   })
 
-  it('should end logging automatically when only .startLogging() is called', done => {
-    if (isCI && process.platform === 'linux') {
-      done()
-      return
-    }
-
+  ifit(process.platform !== 'linux')('should end logging automatically when only .startLogging() is called', async () => {
     const appProcess = ChildProcess.spawn(process.execPath,
       [appPath], {
         env: {
@@ -178,9 +161,7 @@ describe('netLog module', () => {
         }
       })
 
-    appProcess.once('close', () => {
-      expect(fs.existsSync(dumpFileDynamic)).to.be.true('dynamic dump file exists')
-      done()
-    })
+    await emittedOnce(appProcess, 'close')
+    expect(fs.existsSync(dumpFileDynamic)).to.be.true('dynamic dump file exists')
   })
 })

+ 3 - 3
spec-main/api-session-spec.ts

@@ -449,8 +449,8 @@ describe('session module', () => {
 
     it('accepts the request when the callback is called with 0', async () => {
       session.defaultSession.setCertificateVerifyProc(({ hostname, certificate, verificationResult, errorCode }, callback) => {
-        expect(['net::ERR_CERT_AUTHORITY_INVALID', 'net::ERR_CERT_COMMON_NAME_INVALID'].includes(verificationResult)).to.be.true
-        expect([-202, -200].includes(errorCode)).to.be.true
+        expect(['net::ERR_CERT_AUTHORITY_INVALID', 'net::ERR_CERT_COMMON_NAME_INVALID'].includes(verificationResult)).to.be.true()
+        expect([-202, -200].includes(errorCode)).to.be.true()
         callback(0)
       })
 
@@ -471,7 +471,7 @@ describe('session module', () => {
         expect(certificate.issuerCert.issuerCert.issuer.commonName).to.equal('Root CA')
         expect(certificate.issuerCert.issuerCert.subject.commonName).to.equal('Root CA')
         expect(certificate.issuerCert.issuerCert.issuerCert).to.equal(undefined)
-        expect(['net::ERR_CERT_AUTHORITY_INVALID', 'net::ERR_CERT_COMMON_NAME_INVALID'].includes(verificationResult)).to.be.true
+        expect(['net::ERR_CERT_AUTHORITY_INVALID', 'net::ERR_CERT_COMMON_NAME_INVALID'].includes(verificationResult)).to.be.true()
         callback(-2)
       })
 

+ 1 - 4
spec-main/index.js

@@ -47,9 +47,6 @@ app.whenReady().then(() => {
     .boolean('i').alias('i', 'invert')
     .argv
 
-  const isCi = !!argv.ci
-  global.isCI = isCi
-
   const Mocha = require('mocha')
   const mochaOptions = {}
   if (process.env.MOCHA_REPORTER) {
@@ -65,7 +62,7 @@ app.whenReady().then(() => {
   if (!process.env.MOCHA_REPORTER) {
     mocha.ui('bdd').reporter('tap')
   }
-  mocha.timeout(isCi ? 30000 : 10000)
+  mocha.timeout(30000)
 
   if (argv.grep) mocha.grep(argv.grep)
   if (argv.invert) mocha.invert()

+ 1 - 1
spec-main/version-bump-spec.ts

@@ -43,7 +43,7 @@ describe('version-bumper', () => {
 
   // On macOS Circle CI we don't have a real git environment due to running
   // gclient sync on a linux machine. These tests therefore don't run as expected.
-  ifdescribe(!(process.platform === 'linux' && process.arch === 'arm') && !(isCI && process.platform === 'darwin'))('nextVersion', () => {
+  ifdescribe(!(process.platform === 'linux' && process.arch === 'arm') && process.platform !== 'darwin')('nextVersion', () => {
     const nightlyPattern = /[0-9.]*(-nightly.(\d{4})(\d{2})(\d{2}))$/g
     const betaPattern = /[0-9.]*(-beta[0-9.]*)/g
 

+ 1 - 1
spec-main/visibility-state-spec.ts

@@ -9,7 +9,7 @@ import { ifdescribe } from './spec-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' || !isCI)('document.visibilityState', () => {
+ifdescribe(process.platform !== 'linux')('document.visibilityState', () => {
   let w: BrowserWindow
 
   afterEach(() => {

+ 11 - 27
spec/static/index.html

@@ -15,7 +15,7 @@
 
   const path = require('path')
   const electron = require('electron')
-  const { remote, ipcRenderer } = electron
+  const { ipcRenderer } = electron
 
   // Set up chai-as-promised here first to avoid conflicts
   // It must be loaded first or really strange things happen inside
@@ -23,32 +23,16 @@
   // DO NOT MOVE, REMOVE OR EDIT THIS LINE
   require('chai').use(require('chai-as-promised'))
 
-  // Check if we are running in CI.
-  const isCi = remote.getGlobal('isCi')
-
-  if (!isCi) {
-    const win = remote.getCurrentWindow()
-    win.show()
-    win.focus()
-  }
-
-  // Show DevTools.
-  document.oncontextmenu = (e) => {
-    remote.getCurrentWindow().inspectElement(e.clientX, e.clientY)
-  }
-
   // Rediret all output to browser.
-  if (isCi) {
-    const fakeConsole = {}
-    for (const k in console) {
-      if (console.hasOwnProperty(k) && k !== 'assert') {
-        fakeConsole[k] = (...args) => ipcRenderer.send('console-call', k, args)
-      }
+  const fakeConsole = {}
+  for (const k in console) {
+    if (console.hasOwnProperty(k) && k !== 'assert') {
+      fakeConsole[k] = (...args) => ipcRenderer.send('console-call', k, args)
     }
-    global.__defineGetter__('console', function () {
-      return fakeConsole
-    })
   }
+  global.__defineGetter__('console', function () {
+    return fakeConsole
+  })
 
   const Mocha = require('mocha')
   const mochaOptions = {}
@@ -63,9 +47,9 @@
   const mocha = new Mocha(mochaOptions)
 
   if (!process.env.MOCHA_REPORTER) {
-    mocha.ui('bdd').reporter(isCi ? 'tap' : 'html')
+    mocha.ui('bdd').reporter('tap')
   }
-  mocha.timeout(isCi ? 30000 : 10000)
+  mocha.timeout(30000)
 
   const query = Mocha.utils.parseQuery(window.location.search || '')
   if (query.grep) mocha.grep(query.grep)
@@ -97,7 +81,7 @@
       // Ensure the callback is called after runner is defined
       setTimeout(() => {
         Mocha.utils.highlightTags('code')
-        if (isCi) ipcRenderer.send('process.exit', runner.failures)
+        ipcRenderer.send('process.exit', runner.failures)
       }, 0)
     })
   })

+ 6 - 9
spec/static/main.js

@@ -76,14 +76,11 @@ ipcMain.on('echo', function (event, msg) {
 
 global.setTimeoutPromisified = util.promisify(setTimeout)
 
-global.isCi = !!argv.ci
-if (global.isCi) {
-  process.removeAllListeners('uncaughtException')
-  process.on('uncaughtException', function (error) {
-    console.error(error, error.stack)
-    process.exit(1)
-  })
-}
+process.removeAllListeners('uncaughtException')
+process.on('uncaughtException', function (error) {
+  console.error(error, error.stack)
+  process.exit(1)
+})
 
 global.nativeModulesEnabled = !process.env.ELECTRON_SKIP_NATIVE_MODULE_TESTS
 
@@ -122,7 +119,7 @@ app.on('ready', function () {
 
   window = new BrowserWindow({
     title: 'Electron Tests',
-    show: !global.isCi,
+    show: false,
     width: 800,
     height: 600,
     webPreferences: {

+ 1 - 1
vsts-arm-test-steps.yml

@@ -71,7 +71,7 @@ steps:
 - bash: |
     cd src
     export ELECTRON_OUT_DIR=Default
-    (cd electron && node script/yarn test -- --ci --enable-logging)
+    (cd electron && node script/yarn test -- --enable-logging)
   displayName: 'Run Electron tests'
   timeoutInMinutes: 20
   env: