Don't handle EH/NP Helpers if they don't exist

mac.js

@@ -147,25 +147,22 @@ class MacApp extends App {
 
   determinePlistFilesToUpdate () {
     const appPlistFilename = path.join(this.contentsPath, 'Info.plist')
-    const helperPlistFilename = this.ehPlistFilename('Electron Helper.app')
-    const helperEHPlistFilename = this.ehPlistFilename('Electron Helper EH.app')
-    const helperNPPlistFilename = this.ehPlistFilename('Electron Helper NP.app')
-    const loginHelperPlistFilename = this.helperPlistFilename(this.loginHelperPath)
 
     const plists = [
       [appPlistFilename, 'appPlist'],
-      [helperPlistFilename, 'helperPlist'],
-      [helperEHPlistFilename, 'helperEHPlist'],
-      [helperNPPlistFilename, 'helperNPPlist']
+      [this.ehPlistFilename('Electron Helper.app'), 'helperPlist']
     ]
 
-    return fs.pathExists(loginHelperPlistFilename)
-      .then(exists => {
-        if (exists) {
-          plists.push([loginHelperPlistFilename, 'loginHelperPlist'])
-        }
-        return plists
-      })
+    const possiblePlists = [
+      [this.ehPlistFilename('Electron Helper EH.app'), 'helperEHPlist'],
+      [this.ehPlistFilename('Electron Helper NP.app'), 'helperNPPlist'],
+      [this.helperPlistFilename(this.loginHelperPath), 'loginHelperPlist']
+    ]
+
+    return Promise.all(possiblePlists.map(item =>
+      fs.pathExists(item[0])
+        .then(exists => exists ? item : null)
+    )).then(optional => plists.concat(optional.filter(item => item)))
   }
 
   updatePlistFiles () {
@@ -182,8 +179,12 @@ class MacApp extends App {
       .then(() => {
         this.appPlist = this.updatePlist(this.appPlist, this.executableName, appBundleIdentifier, this.appName)
         this.helperPlist = this.updateHelperPlist(this.helperPlist)
-        this.helperEHPlist = this.updateHelperPlist(this.helperEHPlist, 'EH')
-        this.helperNPPlist = this.updateHelperPlist(this.helperNPPlist, 'NP')
+        if (this.helperEHPlist) {
+          this.helperEHPlist = this.updateHelperPlist(this.helperEHPlist, 'EH')
+        }
+        if (this.helperNPPlist) {
+          this.helperNPPlist = this.updateHelperPlist(this.helperNPPlist, 'NP')
+        }
 
         if (this.loginHelperPlist) {
           const loginHelperName = common.sanitizeAppName(`${this.appName} Login Helper`)
@@ -229,10 +230,24 @@ class MacApp extends App {
 
   moveHelper (helperDirectory, suffix) {
     const originalBasename = `Electron${suffix}`
-    const newBasename = `${common.sanitizeAppName(this.appName)}${suffix}`
+
+    return fs.pathExists(path.join(helperDirectory, `${originalBasename}.app`))
+      .then(exists => {
+        if (exists) {
+          return this.renameHelperAndExecutable(
+            helperDirectory,
+            originalBasename,
+            `${common.sanitizeAppName(this.appName)}${suffix}`
+          )
+        } else {
+          return Promise.resolve()
+        }
+      })
+  }
+
+  renameHelperAndExecutable (helperDirectory, originalBasename, newBasename) {
     const originalAppname = `${originalBasename}.app`
     const executableBasePath = path.join(helperDirectory, originalAppname, 'Contents', 'MacOS')
-
     return this.relativeRename(executableBasePath, originalBasename, newBasename)
       .then(() => this.relativeRename(helperDirectory, originalAppname, `${newBasename}.app`))
   }

test/_util.js

@@ -57,6 +57,10 @@ module.exports = {
         return bufferEqual(buffer1, buffer2)
       })
   },
+  assertPathNotExists: function assertPathNotExists (t, pathToCheck, message) {
+    return fs.pathExists(pathToCheck)
+      .then(exists => t.false(exists))
+  },
   fixtureSubdir: setup.fixtureSubdir,
   generateResourcesPath: function generateResourcesPath (opts) {
     return common.isPlatformMac(opts.platform)

test/darwin.js

@@ -44,8 +44,16 @@ function electron0374Test (testName, testFunction) {
   return testWrapper.apply(null, [testName, el0374Opts, testFunction].concat(extraArgs))
 }
 
-function getHelperExecutablePath (helperName) {
-  return path.join(`${helperName}.app`, 'Contents', 'MacOS', helperName)
+function getFrameworksPath (prefix, appName) {
+  return path.join(prefix, `${appName}.app`, 'Contents', 'Frameworks')
+}
+
+function getHelperAppPath (prefix, appName, helperSuffix) {
+  return path.join(getFrameworksPath(prefix, appName), `${appName} ${helperSuffix}.app`)
+}
+
+function getHelperExecutablePath (prefix, appName, helperSuffix) {
+  return path.join(getHelperAppPath(prefix, appName, helperSuffix), 'Contents', 'MacOS', `${appName} ${helperSuffix}`)
 }
 
 function parseInfoPlist (t, opts, basePath) {
@@ -63,32 +71,30 @@ function packageAndParseInfoPlist (t, opts) {
     .then(paths => parseInfoPlist(t, opts, paths[0]))
 }
 
+function assertHelper (t, prefix, appName, helperSuffix) {
+  return fs.stat(getHelperAppPath(prefix, appName, helperSuffix))
+    .then(stats => {
+      t.true(stats.isDirectory(), `The ${helperSuffix}.app should reflect sanitized opts.name`)
+      return fs.stat(getHelperExecutablePath(prefix, appName, helperSuffix))
+    }).then(stats => t.true(stats.isFile(), `The ${helperSuffix}.app executable should reflect sanitized opts.name`))
+}
+
 function helperAppPathsTest (t, baseOpts, extraOpts, expectedName) {
   const opts = Object.assign(baseOpts, extraOpts)
-  let frameworksPath
 
   if (!expectedName) {
     expectedName = opts.name
   }
 
   return packager(opts)
     .then(paths => {
-      frameworksPath = path.join(paths[0], `${expectedName}.app`, 'Contents', 'Frameworks')
-      // main Helper.app is already tested in basic test suite; test its executable and the other helpers
-      return fs.stat(path.join(frameworksPath, getHelperExecutablePath(`${expectedName} Helper`)))
-    }).then(stats => {
-      t.true(stats.isFile(), 'The Helper.app executable should reflect sanitized opts.name')
-      return fs.stat(path.join(frameworksPath, `${expectedName} Helper EH.app`))
-    }).then(stats => {
-      t.true(stats.isDirectory(), 'The Helper EH.app should reflect sanitized opts.name')
-      return fs.stat(path.join(frameworksPath, getHelperExecutablePath(`${expectedName} Helper EH`)))
-    }).then(stats => {
-      t.true(stats.isFile(), 'The Helper EH.app executable should reflect sanitized opts.name')
-      return fs.stat(path.join(frameworksPath, `${expectedName} Helper NP.app`))
-    }).then(stats => {
-      t.true(stats.isDirectory(), 'The Helper NP.app should reflect sanitized opts.name')
-      return fs.stat(path.join(frameworksPath, getHelperExecutablePath(`${expectedName} Helper NP`)))
-    }).then(stats => t.true(stats.isFile(), 'The Helper NP.app executable should reflect sanitized opts.name'))
+      const helpers = [
+        'Helper',
+        'Helper EH',
+        'Helper NP'
+      ]
+      return Promise.all(helpers.map(helper => assertHelper(t, paths[0], expectedName, helper)))
+    })
 }
 
 function iconTest (t, opts, icon, iconPath) {
@@ -421,6 +427,23 @@ if (!(process.env.CI && process.platform === 'win32')) {
   darwinTest('app helpers bundle helper-bundle-id fallback to app-bundle-id (w/ special characters) test', appHelpersBundleTest, null, 'com.electron."bãśè tëßt!!@#$%^&*()?\'')
   darwinTest('app helpers bundle helper-bundle-id & app-bundle-id fallback test', appHelpersBundleTest)
 
+  darwinTest('EH/NP helpers do not exist', (t, baseOpts) => {
+    const helpers = [
+      'Helper EH',
+      'Helper NP'
+    ]
+    const opts = Object.assign({}, baseOpts, {
+      afterExtract: [(buildPath, electronVersion, platform, arch, cb) => {
+        return Promise.all(helpers.map(helper => fs.remove(getHelperAppPath(buildPath, 'Electron', helper)))).then(() => cb()) // eslint-disable-line promise/no-callback-in-promise
+      }]
+    })
+
+    return packager(opts)
+      .then(paths => {
+        return Promise.all(helpers.map(helper => util.assertPathNotExists(t, getHelperAppPath(paths[0], opts.name, helper), `${helper} should not exist`)))
+      })
+  })
+
   darwinTest('appCopyright/NSHumanReadableCopyright test', (t, baseOpts) => {
     const copyright = 'Copyright © 2003–2015 Organization. All rights reserved.'
     const opts = Object.assign({}, baseOpts, {appCopyright: copyright})