From 6781d5e3c82681336c73631ed1f480e228070108 Mon Sep 17 00:00:00 2001 From: Jeremy Apthorp Date: Wed, 30 Oct 2019 16:38:21 -0700 Subject: [PATCH] test: there is only --ci (#20794) --- .circleci/config.yml | 2 +- appveyor.yml | 2 +- azure-pipelines-woa.yml | 4 +- docs/development/build-instructions-gn.md | 2 +- shell/app/atom_main.cc | 14 +--- spec-main/ambient.d.ts | 1 - spec-main/api-app-spec.ts | 51 ++---------- spec-main/api-autoupdater-darwin-spec.ts | 2 +- spec-main/api-browser-window-spec.ts | 11 +-- spec-main/api-global-shortcut-spec.ts | 9 +-- spec-main/api-menu-spec.ts | 95 +---------------------- spec-main/api-net-log-spec.ts | 43 +++------- spec-main/api-session-spec.ts | 6 +- spec-main/index.js | 5 +- spec-main/version-bump-spec.ts | 2 +- spec-main/visibility-state-spec.ts | 2 +- spec/static/index.html | 38 +++------ spec/static/main.js | 15 ++-- vsts-arm-test-steps.yml | 2 +- 19 files changed, 55 insertions(+), 251 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index e9ae15ddd2..17d6083b3a 100644 --- a/.circleci/config.yml +++ b/.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: | diff --git a/appveyor.yml b/appveyor.yml index ade405856c..618a25345e 100644 --- a/appveyor.yml +++ b/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" diff --git a/azure-pipelines-woa.yml b/azure-pipelines-woa.yml index 4dde4afb03..5d60382fe0 100644 --- a/azure-pipelines-woa.yml +++ b/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() \ No newline at end of file + condition: always() diff --git a/docs/development/build-instructions-gn.md b/docs/development/build-instructions-gn.md index c7e6984c6d..6256e6d70b 100644 --- a/docs/development/build-instructions-gn.md +++ b/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 diff --git a/shell/app/atom_main.cc b/shell/app/atom_main.cc index e09c23d733..a570b0cdbc 100644 --- a/shell/app/atom_main.cc +++ b/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); diff --git a/spec-main/ambient.d.ts b/spec-main/ambient.d.ts index bb3b467b5b..54847a3550 100644 --- a/spec-main/ambient.d.ts +++ b/spec-main/ambient.d.ts @@ -1,4 +1,3 @@ -declare var isCI: boolean; declare var standardScheme: string; declare namespace Electron { diff --git a/spec-main/api-app-spec.ts b/spec-main/api-app-spec.ts index 62946cd0ca..40c7244f1d 100644 --- a/spec-main/api-app-spec.ts +++ b/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) diff --git a/spec-main/api-autoupdater-darwin-spec.ts b/spec-main/api-autoupdater-darwin-spec.ts index 25cd47e5d2..17f41ffccf 100644 --- a/spec-main/api-autoupdater-darwin-spec.ts +++ b/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() diff --git a/spec-main/api-browser-window-spec.ts b/spec-main/api-browser-window-spec.ts index 53d943c4f0..d3249d5f55 100644 --- a/spec-main/api-browser-window-spec.ts +++ b/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, diff --git a/spec-main/api-global-shortcut-spec.ts b/spec-main/api-global-shortcut-spec.ts index f2934ac6eb..969a63207a 100644 --- a/spec-main/api-global-shortcut-spec.ts +++ b/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() }) diff --git a/spec-main/api-menu-spec.ts b/spec-main/api-menu-spec.ts index 5ee9bdd2fb..584d4b02af 100644 --- a/spec-main/api-menu-spec.ts +++ b/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) - }) - }) }) diff --git a/spec-main/api-net-log-spec.ts b/spec-main/api-net-log-spec.ts index bae26a5c01..7afda21065 100644 --- a/spec-main/api-net-log-spec.ts +++ b/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') }) }) diff --git a/spec-main/api-session-spec.ts b/spec-main/api-session-spec.ts index 40975b9713..9f9b3c6188 100644 --- a/spec-main/api-session-spec.ts +++ b/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) }) diff --git a/spec-main/index.js b/spec-main/index.js index dafa8db3fc..06ce999c1d 100644 --- a/spec-main/index.js +++ b/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() diff --git a/spec-main/version-bump-spec.ts b/spec-main/version-bump-spec.ts index 172f760050..ed6178cde8 100644 --- a/spec-main/version-bump-spec.ts +++ b/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 diff --git a/spec-main/visibility-state-spec.ts b/spec-main/visibility-state-spec.ts index 4fec94b1ee..43f258d29e 100644 --- a/spec-main/visibility-state-spec.ts +++ b/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(() => { diff --git a/spec/static/index.html b/spec/static/index.html index 1fa0bf5cac..e77555a364 100644 --- a/spec/static/index.html +++ b/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) }) }) diff --git a/spec/static/main.js b/spec/static/main.js index 688a0bd567..624c0a8404 100644 --- a/spec/static/main.js +++ b/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: { diff --git a/vsts-arm-test-steps.yml b/vsts-arm-test-steps.yml index 29c4a54500..3d850847e0 100644 --- a/vsts-arm-test-steps.yml +++ b/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: