Merge pull request #6718 from electron/coverage

Add JavaScript code coverage reporting

package.json

@@ -3,6 +3,7 @@
   "version": "1.3.2",
   "devDependencies": {
     "asar": "^0.11.0",
+    "electabul": "~0.0.4",
     "request": "*",
     "standard": "^7.1.2",
     "standard-markdown": "^1.1.1"
@@ -24,6 +25,8 @@
   "scripts": {
     "bootstrap": "python ./script/bootstrap.py",
     "build": "python ./script/build.py -c D",
+    "coverage": "npm run instrument-code-coverage && npm test -- --use-instrumented-asar",
+    "instrument-code-coverage": "electabul instrument --input-path ./lib --output-path ./out/coverage/electron.asar",
     "lint": "npm run lint-js && npm run lint-cpp && npm run lint-docs",
     "lint-js": "standard && cd spec && standard",
     "lint-cpp": "python ./script/cpplint.py",

script/test.py

@@ -1,6 +1,7 @@
 #!/usr/bin/env python
 
 import os
+import shutil
 import subprocess
 import sys
 
@@ -24,17 +25,30 @@ def main():
     electron = os.path.join(SOURCE_ROOT, 'out', config,
                               '{0}.app'.format(PRODUCT_NAME), 'Contents',
                               'MacOS', PRODUCT_NAME)
+    resources_path = os.path.join(SOURCE_ROOT, 'out', config,
+                                   '{0}.app'.format(PRODUCT_NAME), 'Contents',
+                                   'Resources')
   elif sys.platform == 'win32':
     electron = os.path.join(SOURCE_ROOT, 'out', config,
                               '{0}.exe'.format(PROJECT_NAME))
+    resources_path = os.path.join(SOURCE_ROOT, 'out', config)
   else:
     electron = os.path.join(SOURCE_ROOT, 'out', config, PROJECT_NAME)
+    resources_path = os.path.join(SOURCE_ROOT, 'out', config)
 
+  use_instrumented_asar = '--use-instrumented-asar' in sys.argv
   returncode = 0
   try:
+    if use_instrumented_asar:
+      install_instrumented_asar_file(resources_path)
     subprocess.check_call([electron, 'spec'] + sys.argv[1:])
   except subprocess.CalledProcessError as e:
     returncode = e.returncode
+  except KeyboardInterrupt:
+    returncode = 0
+
+  if use_instrumented_asar:
+    restore_uninstrumented_asar_file(resources_path)
 
   if os.environ.has_key('OUTPUT_TO_FILE'):
     output_to_file = os.environ['OUTPUT_TO_FILE']
@@ -46,5 +60,23 @@ def main():
   return returncode
 
 
+def install_instrumented_asar_file(resources_path):
+  asar_path = os.path.join(resources_path, '{0}.asar'.format(PROJECT_NAME))
+  uninstrumented_path = os.path.join(resources_path,
+                                      '{0}-original.asar'.format(PROJECT_NAME))
+  instrumented_path = os.path.join(SOURCE_ROOT, 'out', 'coverage',
+                                      '{0}.asar'.format(PROJECT_NAME))
+  shutil.move(asar_path, uninstrumented_path)
+  shutil.move(instrumented_path, asar_path)
+
+
+def restore_uninstrumented_asar_file(resources_path):
+  asar_path = os.path.join(resources_path, '{0}.asar'.format(PROJECT_NAME))
+  uninstrumented_path = os.path.join(resources_path,
+                                      '{0}-original.asar'.format(PROJECT_NAME))
+  os.remove(asar_path)
+  shutil.move(uninstrumented_path, asar_path)
+
+
 if __name__ == '__main__':
   sys.exit(main())

spec/api-app-spec.js

@@ -5,6 +5,7 @@ const net = require('net')
 const fs = require('fs')
 const path = require('path')
 const {remote} = require('electron')
+const {closeWindow} = require('./window-helpers')
 
 const {app, BrowserWindow, ipcMain} = remote
 
@@ -19,9 +20,6 @@ describe('electron module', function () {
     let window = null
 
     beforeEach(function () {
-      if (window != null) {
-        window.destroy()
-      }
       window = new BrowserWindow({
         show: false,
         width: 400,
@@ -30,10 +28,7 @@ describe('electron module', function () {
     })
 
     afterEach(function () {
-      if (window != null) {
+      return closeWindow(window).then(function () { window = null })
-        window.destroy()
-      }
-      window = null
     })
 
     it('always returns the internal electron module', function (done) {
@@ -191,10 +186,7 @@ describe('app module', function () {
     })
 
     afterEach(function () {
-      if (w != null) {
+      return closeWindow(w).then(function () { w = null })
-        w.destroy()
-      }
-      w = null
     })
 
     it('can import certificate into platform cert store', function (done) {
@@ -232,10 +224,7 @@ describe('app module', function () {
     var w = null
 
     afterEach(function () {
-      if (w != null) {
+      return closeWindow(w).then(function () { w = null })
-        w.destroy()
-      }
-      w = null
     })
 
     it('should emit browser-window-focus event when window is focused', function (done) {

spec/api-browser-window-spec.js

@@ -5,6 +5,7 @@ const fs = require('fs')
 const path = require('path')
 const os = require('os')
 const http = require('http')
+const {closeWindow} = require('./window-helpers')
 
 const remote = require('electron').remote
 const screen = require('electron').screen
@@ -38,9 +39,6 @@ describe('browser-window module', function () {
   })
 
   beforeEach(function () {
-    if (w != null) {
-      w.destroy()
-    }
     w = new BrowserWindow({
       show: false,
       width: 400,
@@ -52,10 +50,7 @@ describe('browser-window module', function () {
   })
 
   afterEach(function () {
-    if (w != null) {
+    return closeWindow(w).then(function () { w = null })
-      w.destroy()
-    }
-    w = null
   })
 
   describe('BrowserWindow.close()', function () {
@@ -63,7 +58,7 @@ describe('browser-window module', function () {
       w.webContents.on('did-finish-load', function () {
         w.close()
       })
-      w.on('closed', function () {
+      w.once('closed', function () {
         var test = path.join(fixtures, 'api', 'unload')
         var content = fs.readFileSync(test)
         fs.unlinkSync(test)
@@ -74,7 +69,7 @@ describe('browser-window module', function () {
     })
 
     it('should emit beforeunload handler', function (done) {
-      w.on('onbeforeunload', function () {
+      w.once('onbeforeunload', function () {
         done()
       })
       w.webContents.on('did-finish-load', function () {
@@ -86,7 +81,7 @@ describe('browser-window module', function () {
 
   describe('window.close()', function () {
     it('should emit unload handler', function (done) {
-      w.on('closed', function () {
+      w.once('closed', function () {
         var test = path.join(fixtures, 'api', 'close')
         var content = fs.readFileSync(test)
         fs.unlinkSync(test)
@@ -97,7 +92,7 @@ describe('browser-window module', function () {
     })
 
     it('should emit beforeunload handler', function (done) {
-      w.on('onbeforeunload', function () {
+      w.once('onbeforeunload', function () {
         done()
       })
       w.loadURL('file://' + path.join(fixtures, 'api', 'close-beforeunload-false.html'))
@@ -526,21 +521,21 @@ describe('browser-window module', function () {
 
   describe('beforeunload handler', function () {
     it('returning undefined would not prevent close', function (done) {
-      w.on('closed', function () {
+      w.once('closed', function () {
         done()
       })
       w.loadURL('file://' + path.join(fixtures, 'api', 'close-beforeunload-undefined.html'))
     })
 
     it('returning false would prevent close', function (done) {
-      w.on('onbeforeunload', function () {
+      w.once('onbeforeunload', function () {
         done()
       })
       w.loadURL('file://' + path.join(fixtures, 'api', 'close-beforeunload-false.html'))
     })
 
     it('returning empty string would prevent close', function (done) {
-      w.on('onbeforeunload', function () {
+      w.once('onbeforeunload', function () {
         done()
       })
       w.loadURL('file://' + path.join(fixtures, 'api', 'close-beforeunload-empty-string.html'))
@@ -1002,6 +997,7 @@ describe('browser-window module', function () {
 
   describe('dev tool extensions', function () {
     describe('BrowserWindow.addDevToolsExtension', function () {
+      let showPanelIntevalId
       this.timeout(10000)
 
       beforeEach(function () {
@@ -1013,7 +1009,7 @@ describe('browser-window module', function () {
         assert.equal(BrowserWindow.getDevToolsExtensions().hasOwnProperty('foo'), true)
 
         w.webContents.on('devtools-opened', function () {
-          var showPanelIntevalId = setInterval(function () {
+          showPanelIntevalId = setInterval(function () {
             if (w && w.devToolsWebContents) {
               var showLastPanel = function () {
                 var lastPanelId = WebInspector.inspectorView._tabbedPane._tabs.peekLast().id
@@ -1029,6 +1025,10 @@ describe('browser-window module', function () {
         w.loadURL('about:blank')
       })
 
+      afterEach(function () {
+        clearInterval(showPanelIntevalId)
+      })
+
       it('throws errors for missing manifest.json files', function () {
         assert.throws(function () {
           BrowserWindow.addDevToolsExtension(path.join(__dirname, 'does-not-exist'))

spec/api-crash-reporter-spec.js

@@ -3,6 +3,7 @@ const http = require('http')
 const multiparty = require('multiparty')
 const path = require('path')
 const url = require('url')
+const {closeWindow} = require('./window-helpers')
 
 const remote = require('electron').remote
 const app = remote.require('electron').app
@@ -20,7 +21,7 @@ describe('crash-reporter module', function () {
   })
 
   afterEach(function () {
-    w.destroy()
+    return closeWindow(w).then(function () { w = null })
   })
 
   if (process.mas) {

spec/api-debugger-spec.js

@@ -1,5 +1,6 @@
 const assert = require('assert')
 const path = require('path')
+const {closeWindow} = require('./window-helpers')
 const BrowserWindow = require('electron').remote.BrowserWindow
 
 describe('debugger module', function () {
@@ -7,9 +8,6 @@ describe('debugger module', function () {
   var w = null
 
   beforeEach(function () {
-    if (w != null) {
-      w.destroy()
-    }
     w = new BrowserWindow({
       show: false,
       width: 400,
@@ -18,10 +16,7 @@ describe('debugger module', function () {
   })
 
   afterEach(function () {
-    if (w != null) {
+    return closeWindow(w).then(function () { w = null })
-      w.destroy()
-    }
-    w = null
   })
 
   describe('debugger.attach', function () {

spec/api-ipc-spec.js

@@ -2,6 +2,7 @@
 
 const assert = require('assert')
 const path = require('path')
+const {closeWindow} = require('./window-helpers')
 
 const {ipcRenderer, remote} = require('electron')
 const {ipcMain, webContents, BrowserWindow} = remote
@@ -17,6 +18,12 @@ const comparePaths = function (path1, path2) {
 describe('ipc module', function () {
   var fixtures = path.join(__dirname, 'fixtures')
 
+  var w = null
+
+  afterEach(function () {
+    return closeWindow(w).then(function () { w = null })
+  })
+
   describe('remote.require', function () {
     it('should returns same object for the same module', function () {
       var dialog1 = remote.require('electron')
@@ -302,19 +309,18 @@ describe('ipc module', function () {
     it('does not crash when reply is not sent and browser is destroyed', function (done) {
       this.timeout(10000)
 
-      var w = new BrowserWindow({
+      w = new BrowserWindow({
         show: false
       })
       ipcMain.once('send-sync-message', function (event) {
         event.returnValue = null
-        w.destroy()
         done()
       })
       w.loadURL('file://' + path.join(fixtures, 'api', 'send-sync-message.html'))
     })
 
     it('does not crash when reply is sent by multiple listeners', function (done) {
-      var w = new BrowserWindow({
+      w = new BrowserWindow({
         show: false
       })
       ipcMain.on('send-sync-message', function (event) {
@@ -322,7 +328,6 @@ describe('ipc module', function () {
       })
       ipcMain.on('send-sync-message', function (event) {
         event.returnValue = null
-        w.destroy()
         done()
       })
       w.loadURL('file://' + path.join(fixtures, 'api', 'send-sync-message.html'))
@@ -354,12 +359,6 @@ describe('ipc module', function () {
   })
 
   describe('remote listeners', function () {
-    var w = null
-
-    afterEach(function () {
-      w.destroy()
-    })
-
     it('can be added and removed correctly', function () {
       w = new BrowserWindow({
         show: false

spec/api-protocol-spec.js

@@ -2,6 +2,7 @@ const assert = require('assert')
 const http = require('http')
 const path = require('path')
 const qs = require('querystring')
+const {closeWindow} = require('./window-helpers')
 const remote = require('electron').remote
 const {BrowserWindow, protocol, webContents} = remote
 
@@ -896,11 +897,10 @@ describe('protocol module', function () {
 
     afterEach(function (done) {
       protocol.unregisterProtocol(standardScheme, function () {
-        if (w != null) {
+        closeWindow(w).then(function () {
-          w.destroy()
+          w = null
-        }
+          done()
-        w = null
+        })
-        done()
       })
     })
 

spec/api-session-spec.js

@@ -2,6 +2,7 @@ const assert = require('assert')
 const http = require('http')
 const path = require('path')
 const fs = require('fs')
+const {closeWindow} = require('./window-helpers')
 
 const {ipcRenderer, remote} = require('electron')
 const {ipcMain, session, BrowserWindow} = remote
@@ -14,9 +15,6 @@ describe('session module', function () {
   var url = 'http://127.0.0.1'
 
   beforeEach(function () {
-    if (w != null) {
-      w.destroy()
-    }
     w = new BrowserWindow({
       show: false,
       width: 400,
@@ -25,10 +23,7 @@ describe('session module', function () {
   })
 
   afterEach(function () {
-    if (w != null) {
+    return closeWindow(w).then(function () { w = null })
-      w.destroy()
-    }
-    w = null
   })
 
   describe('session.defaultSession', function () {
@@ -194,7 +189,7 @@ describe('session module', function () {
     })
 
     afterEach(function () {
-      w.destroy()
+      return closeWindow(w).then(function () { w = null })
     })
 
     it('can cancel default download behavior', function (done) {

spec/api-web-contents-spec.js

@@ -2,6 +2,7 @@
 
 const assert = require('assert')
 const path = require('path')
+const {closeWindow} = require('./window-helpers')
 
 const {remote} = require('electron')
 const {BrowserWindow, webContents} = remote
@@ -13,9 +14,6 @@ describe('webContents module', function () {
   let w
 
   beforeEach(function () {
-    if (w != null) {
-      w.destroy()
-    }
     w = new BrowserWindow({
       show: false,
       width: 400,
@@ -27,10 +25,7 @@ describe('webContents module', function () {
   })
 
   afterEach(function () {
-    if (w != null) {
+    return closeWindow(w).then(function () { w = null })
-      w.destroy()
-    }
-    w = null
   })
 
   describe('getAllWebContents() API', function () {

spec/asar-spec.js

@@ -2,6 +2,7 @@ const assert = require('assert')
 const ChildProcess = require('child_process')
 const fs = require('fs')
 const path = require('path')
+const {closeWindow} = require('./window-helpers')
 
 const nativeImage = require('electron').nativeImage
 const remote = require('electron').remote
@@ -794,8 +795,8 @@ describe('asar package', function () {
 
     it('sets __dirname correctly', function (done) {
       after(function () {
-        w.destroy()
         ipcMain.removeAllListeners('dirname')
+        return closeWindow(w).then(function () { w = null })
       })
 
       var w = new BrowserWindow({
@@ -818,8 +819,8 @@ describe('asar package', function () {
 
     it('loads script tag in html', function (done) {
       after(function () {
-        w.destroy()
         ipcMain.removeAllListeners('ping')
+        return closeWindow(w).then(function () { w = null })
       })
 
       var w = new BrowserWindow({

spec/chromium-spec.js

@@ -54,7 +54,7 @@ describe('chromium feature', function () {
       w = new BrowserWindow({
         show: false
       })
-      w.webContents.on('ipc-message', function (event, args) {
+      w.webContents.once('ipc-message', function (event, args) {
         assert.deepEqual(args, ['hidden', true])
         done()
       })
@@ -69,7 +69,7 @@ describe('chromium feature', function () {
       w = new BrowserWindow({
         show: false
       })
-      w.webContents.on('ipc-message', function (event, args) {
+      w.webContents.once('ipc-message', function (event, args) {
         assert.deepEqual(args, ['hidden', false])
         done()
       })
@@ -278,7 +278,7 @@ describe('chromium feature', function () {
       w = new BrowserWindow({
         show: false
       })
-      w.webContents.on('ipc-message', function (event, args) {
+      w.webContents.once('ipc-message', function (event, args) {
         assert.deepEqual(args, ['opener', null])
         done()
       })

spec/fixtures/api/beforeunload-false.html

@@ -1,11 +1,16 @@
 <html>
 <body>
 <script type="text/javascript" charset="utf-8">
+  // Only prevent unload on the first window close
+  var unloadPrevented = false;
   window.onbeforeunload = function() {
     setTimeout(function() {
       require('electron').remote.getCurrentWindow().emit('onbeforeunload');
     }, 0);
-    return false;
+    if (!unloadPrevented) {
+      unloadPrevented = true;
+      return false;
+    }
   }
 </script>
 </body>

spec/fixtures/api/close-beforeunload-empty-string.html

@@ -1,11 +1,17 @@
 <html>
 <body>
 <script type="text/javascript" charset="utf-8">
+  // Only prevent unload on the first window close
+  var unloadPrevented = false;
   window.onbeforeunload = function() {
     setTimeout(function() {
       require('electron').remote.getCurrentWindow().emit('onbeforeunload');
     }, 0);
-    return '';
+
+    if (!unloadPrevented) {
+      unloadPrevented = true;
+      return '';
+    }
   }
   window.close();
 </script>

spec/fixtures/api/close-beforeunload-false.html

@@ -1,11 +1,16 @@
 <html>
 <body>
 <script type="text/javascript" charset="utf-8">
+  // Only prevent unload on the first window close
+  var unloadPrevented = false;
   window.onbeforeunload = function() {
     setTimeout(function() {
       require('electron').remote.getCurrentWindow().emit('onbeforeunload');
     }, 0);
-    return false;
+    if (!unloadPrevented) {
+      unloadPrevented = true;
+      return false;
+    }
   }
   window.close();
 </script>

spec/fixtures/devtools-extensions/foo/index.html

@@ -16,6 +16,15 @@
         })
       }
 
+      function reportCoverage () {
+        var message = JSON.stringify({
+          pid: chrome.runtime.id,
+          coverage: window.__coverage__
+        })
+        var coverageMessage = `require('electron').ipcRenderer.send('report-coverage', ${message})`
+        window.chrome.devtools.inspectedWindow.eval(coverageMessage, function () {})
+      }
+
       testStorage(function (syncItems, localItems) {
         var message = JSON.stringify({
           runtimeId: chrome.runtime.id,
@@ -26,6 +35,8 @@
             sync: syncItems
           }
         })
+
+        reportCoverage()
         var sendMessage = `require('electron').ipcRenderer.send('answer', ${message})`
         window.chrome.devtools.inspectedWindow.eval(sendMessage, function () {})
       })

spec/static/index.html

@@ -13,11 +13,12 @@
   // Disable use of deprecated functions.
   process.throwDeprecation = true;
 
-  // Check if we are running in CI.
+  var path = require('path');
   var electron = require ('electron');
   var remote = electron.remote;
   var ipcRenderer = electron.ipcRenderer;
 
+  // Check if we are running in CI.
   var isCi = remote.getGlobal('isCi')
 
   if (!isCi) {
@@ -47,6 +48,8 @@
     });
   }
 
+  var Coverage = require('electabul').Coverage;
+
   var Mocha = require('mocha');
 
   var mocha = new Mocha();
@@ -81,6 +84,15 @@
   walker.on('end', function() {
     var runner = mocha.run(function() {
       Mocha.utils.highlightTags('code');
+
+      var coverage = new Coverage({
+        libPath: path.join(__dirname, '..', '..', 'lib'),
+        outputPath: path.join(__dirname, '..', '..', 'out', 'coverage'),
+        formats: ['text', 'lcov']
+      });
+      coverage.addCoverage(ipcRenderer.sendSync('get-main-process-coverage'))
+      coverage.generateReport()
+
       if (isCi)
         ipcRenderer.send('process.exit', runner.failures);
     });

spec/static/main.js

@@ -8,6 +8,7 @@ const dialog = electron.dialog
 const BrowserWindow = electron.BrowserWindow
 const protocol = electron.protocol
 
+const Coverage = require('electabul').Coverage
 const fs = require('fs')
 const path = require('path')
 const url = require('url')
@@ -63,6 +64,15 @@ ipcMain.on('echo', function (event, msg) {
   event.returnValue = msg
 })
 
+const coverage = new Coverage({
+  outputPath: path.join(__dirname, '..', '..', 'out', 'coverage')
+})
+coverage.setup()
+
+ipcMain.on('get-main-process-coverage', function (event) {
+  event.returnValue = global.__coverage__ || null
+})
+
 global.isCi = !!argv.ci
 if (global.isCi) {
   process.removeAllListeners('uncaughtException')

spec/window-helpers.js

@@ -0,0 +1,13 @@
+exports.closeWindow = (window) => {
+  if (window == null || window.isDestroyed()) {
+    return Promise.resolve()
+  } else {
+    return new Promise((resolve, reject) => {
+      window.once('closed', () => {
+        resolve()
+      })
+      window.setClosable(true)
+      window.close()
+    })
+  }
+}