Merge pull request #9227 from electron/specs-on-new-ci-machines

Get specs passing on new macOS CI machines

lib/common/api/crash-reporter.js

@@ -56,7 +56,7 @@ class CrashReporter {
       const env = {
         ELECTRON_INTERNAL_CRASH_SERVICE: 1
       }
-      spawn(process.execPath, args, {
+      this._crashServiceProcess = spawn(process.execPath, args, {
         env: env,
         detached: true
       })

spec/api-browser-window-spec.js

@@ -1474,13 +1474,19 @@ describe('BrowserWindow module', function () {
       // Only implemented on macOS.
       if (process.platform !== 'darwin') return
 
-      it('can be changed with setKiosk method', function () {
+      it('can be changed with setKiosk method', function (done) {
         w.destroy()
         w = new BrowserWindow()
         w.setKiosk(true)
         assert.equal(w.isKiosk(), true)
-        w.setKiosk(false)
-        assert.equal(w.isKiosk(), false)
+
+        w.once('enter-full-screen', () => {
+          w.setKiosk(false)
+          assert.equal(w.isKiosk(), false)
+        })
+        w.once('leave-full-screen', () => {
+          done()
+        })
       })
     })
 

spec/api-crash-reporter-spec.js

@@ -33,8 +33,10 @@ describe('crashReporter module', function () {
   const generateSpecs = (description, browserWindowOpts) => {
     describe(description, function () {
       var w = null
+      var stopServer = null
 
       beforeEach(function () {
+        stopServer = null
         w = new BrowserWindow(Object.assign({
           show: false
         }, browserWindowOpts))
@@ -44,13 +46,25 @@ describe('crashReporter module', function () {
         return closeWindow(w).then(function () { w = null })
       })
 
+      afterEach(function () {
+        stopCrashService()
+      })
+
+      afterEach(function (done) {
+        if (stopServer != null) {
+          stopServer(done)
+        } else {
+          done()
+        }
+      })
+
       it('should send minidump when renderer crashes', function (done) {
         if (process.env.APPVEYOR === 'True') return done()
         if (process.env.TRAVIS === 'true') return done()
 
         this.timeout(120000)
 
-        startServer({
+        stopServer = startServer({
           callback (port) {
             const crashUrl = url.format({
               protocol: 'file',
@@ -70,11 +84,26 @@ describe('crashReporter module', function () {
 
         this.timeout(120000)
 
-        startServer({
+        stopServer = startServer({
           callback (port) {
-            const crashesDir = path.join(app.getPath('temp'), `${app.getName()} Crashes`)
+            const crashesDir = path.join(app.getPath('temp'), `${process.platform === 'win32' ? 'Zombies' : app.getName()} Crashes`)
             const version = app.getVersion()
             const crashPath = path.join(fixtures, 'module', 'crash.js')
+
+            if (process.platform === 'win32') {
+              const crashServiceProcess = childProcess.spawn(process.execPath, [
+                `--reporter-url=http://127.0.0.1:${port}`,
+                '--application-name=Zombies',
+                `--crashes-directory=${crashesDir}`
+              ], {
+                env: {
+                  ELECTRON_INTERNAL_CRASH_SERVICE: 1
+                },
+                detached: true
+              })
+              remote.process.crashServicePid = crashServiceProcess.pid
+            }
+
             childProcess.fork(crashPath, [port, version, crashesDir], {silent: true})
           },
           processType: 'browser',
@@ -85,7 +114,6 @@ describe('crashReporter module', function () {
       it('should not send minidump if uploadToServer is false', function (done) {
         this.timeout(120000)
 
-        let server
         let dumpFile
         let crashesDir = crashReporter.getCrashesDirectory()
         const existingDumpFiles = new Set()
@@ -96,9 +124,8 @@ describe('crashReporter module', function () {
         }
         const testDone = (uploaded) => {
           if (uploaded) {
-            return done(new Error('fail'))
+            return done(new Error('Uploaded crash report'))
           }
-          server.close()
           if (process.platform === 'darwin') {
             crashReporter.setUploadToServer(true)
           }
@@ -139,7 +166,7 @@ describe('crashReporter module', function () {
           })
         })
 
-        server = startServer({
+        stopServer = startServer({
           callback (port) {
             const crashUrl = url.format({
               protocol: 'file',
@@ -157,9 +184,9 @@ describe('crashReporter module', function () {
         if (process.env.APPVEYOR === 'True') return done()
         if (process.env.TRAVIS === 'true') return done()
 
-        this.timeout(10000)
+        this.timeout(120000)
 
-        startServer({
+        stopServer = startServer({
           callback (port) {
             const crashUrl = url.format({
               protocol: 'file',
@@ -176,7 +203,7 @@ describe('crashReporter module', function () {
   }
 
   generateSpecs('without sandbox', {})
-  generateSpecs('with sandbox ', {
+  generateSpecs('with sandbox', {
     webPreferences: {
       sandbox: true,
       preload: path.join(fixtures, 'module', 'preload-sandbox.js')
@@ -254,7 +281,6 @@ const waitForCrashReport = () => {
 const startServer = ({callback, processType, done}) => {
   var called = false
   var server = http.createServer((req, res) => {
-    server.close()
     var form = new multiparty.Form()
     form.parse(req, (error, fields) => {
       if (error) throw error
@@ -283,6 +309,15 @@ const startServer = ({callback, processType, done}) => {
       })
     })
   })
+
+  const activeConnections = new Set()
+  server.on('connection', (connection) => {
+    activeConnections.add(connection)
+    connection.once('close', () => {
+      activeConnections.delete(connection)
+    })
+  })
+
   let {port} = remote.process
   server.listen(port, '127.0.0.1', () => {
     port = server.address().port
@@ -295,5 +330,27 @@ const startServer = ({callback, processType, done}) => {
     }
     callback(port)
   })
-  return server
+
+  return function stopServer (done) {
+    for (const connection of activeConnections) {
+      connection.destroy()
+    }
+    server.close(function () {
+      done()
+    })
+  }
+}
+
+const stopCrashService = () => {
+  const {crashServicePid} = remote.process
+  if (crashServicePid) {
+    remote.process.crashServicePid = 0
+    try {
+      process.kill(crashServicePid)
+    } catch (error) {
+      if (error.code !== 'ESRCH') {
+        throw error
+      }
+    }
+  }
 }

spec/chromium-spec.js

@@ -13,6 +13,7 @@ const isCI = remote.getGlobal('isCi')
 describe('chromium feature', function () {
   var fixtures = path.resolve(__dirname, 'fixtures')
   var listener = null
+  let w = null
 
   afterEach(function () {
     if (listener != null) {
@@ -21,6 +22,10 @@ describe('chromium feature', function () {
     listener = null
   })
 
+  afterEach(function () {
+    return closeWindow(w).then(function () { w = null })
+  })
+
   describe('heap snapshot', function () {
     it('does not crash', function () {
       if (process.env.TRAVIS === 'true') return
@@ -44,11 +49,6 @@ describe('chromium feature', function () {
 
   describe('document.hidden', function () {
     var url = 'file://' + fixtures + '/pages/document-hidden.html'
-    var w = null
-
-    afterEach(function () {
-      return closeWindow(w).then(function () { w = null })
-    })
 
     it('is set correctly when window is not shown', function (done) {
       w = new BrowserWindow({
@@ -90,13 +90,7 @@ describe('chromium feature', function () {
   })
 
   describe('navigator.mediaDevices', function () {
-    if (process.env.TRAVIS === 'true') {
-      return
-    }
-    if (isCI && process.platform === 'linux') {
-      return
-    }
-    if (isCI && process.platform === 'win32') {
+    if (isCI) {
       return
     }
 
@@ -107,7 +101,7 @@ describe('chromium feature', function () {
         if (labelFound) {
           done()
         } else {
-          done('No device labels found: ' + JSON.stringify(labels))
+          done(new Error(`No device labels found: ${JSON.stringify(labels)}`))
         }
       }).catch(done)
     })
@@ -119,7 +113,7 @@ describe('chromium feature', function () {
       }
       const deviceIds = []
       const ses = session.fromPartition('persist:media-device-id')
-      let w = new BrowserWindow({
+      w = new BrowserWindow({
         show: false,
         webPreferences: {
           session: ses
@@ -155,11 +149,6 @@ describe('chromium feature', function () {
 
   describe('navigator.serviceWorker', function () {
     var url = 'file://' + fixtures + '/pages/service-worker/index.html'
-    var w = null
-
-    afterEach(function () {
-      return closeWindow(w).then(function () { w = null })
-    })
 
     it('should register for file scheme', function (done) {
       w = new BrowserWindow({
@@ -188,12 +177,6 @@ describe('chromium feature', function () {
       return
     }
 
-    let w = null
-
-    afterEach(() => {
-      return closeWindow(w).then(function () { w = null })
-    })
-
     it('returns a BrowserWindowProxy object', function () {
       var b = window.open('about:blank', '', 'show=no')
       assert.equal(b.closed, false)
@@ -343,11 +326,6 @@ describe('chromium feature', function () {
 
   describe('window.opener', function () {
     let url = 'file://' + fixtures + '/pages/window-opener.html'
-    let w = null
-
-    afterEach(function () {
-      return closeWindow(w).then(function () { w = null })
-    })
 
     it('is null for main window', function (done) {
       w = new BrowserWindow({
@@ -849,7 +827,6 @@ describe('chromium feature', function () {
   })
 
   describe('PDF Viewer', function () {
-    let w = null
     const pdfSource = url.format({
       pathname: path.join(fixtures, 'assets', 'cat.pdf').replace(/\\/g, '/'),
       protocol: 'file',
@@ -865,10 +842,6 @@ describe('chromium feature', function () {
       })
     })
 
-    afterEach(function () {
-      return closeWindow(w).then(function () { w = null })
-    })
-
     it('opens when loading a pdf resource as top level navigation', function (done) {
       ipcMain.once('pdf-loaded', function (event, success) {
         if (success) done()

spec/fixtures/api/crash-restart.html

@@ -3,7 +3,7 @@
 <script type="text/javascript" charset="utf-8">
 
 const {port} = require('url').parse(window.location.href, true).query
-const {crashReporter} = require('electron')
+const {crashReporter, ipcRenderer} = require('electron')
 
 crashReporter.start({
   productName: 'Zombies',
@@ -18,6 +18,10 @@ crashReporter.start({
   }
 })
 
+if (process.platform === 'win32') {
+  ipcRenderer.sendSync('crash-service-pid', crashReporter._crashServiceProcess.pid)
+}
+
 setImmediate(() => {
   if (process.platform === 'darwin') {
     crashReporter.setExtraParameter('extra2', 'extra2')

spec/fixtures/api/crash.html

@@ -16,6 +16,11 @@ crashReporter.start({
     'extra2': 'extra2',
   }
 });
+
+if (process.platform === 'win32') {
+  ipcRenderer.sendSync('crash-service-pid', crashReporter._crashServiceProcess.pid)
+}
+
 if (!uploadToServer) {
   ipcRenderer.sendSync('list-existing-dumps')
 }

spec/static/main.js

@@ -24,7 +24,10 @@ var argv = require('yargs')
   .argv
 
 var window = null
-process.port = 0 // will be used by crash-reporter spec.
+
+ // will be used by crash-reporter spec.
+process.port = 0
+process.crashServicePid = 0
 
 v8.setFlagsFromString('--expose_gc')
 app.commandLine.appendSwitch('js-flags', '--expose_gc')
@@ -329,6 +332,11 @@ ipcMain.on('navigate-with-pending-entry', (event, id) => {
   })
 })
 
+ipcMain.on('crash-service-pid', (event, pid) => {
+  process.crashServicePid = pid
+  event.returnValue = null
+})
+
 // Suspend listeners until the next event and then restore them
 const suspendListeners = (emitter, eventName, callback) => {
   const listeners = emitter.listeners(eventName)