feat(postinstall): warn when no hooks were installed

various scenarios like no scripts field, no githook scripts, no valid githook scripts

Author: PixnBits

.eslintrc

@@ -7,6 +7,8 @@
   },
   rules: {
     // think in function scope
-    'vars-on-top': 0
+    'vars-on-top': 0,
+    // command line tool
+    'no-console': 0
   }
 }

postinstall.js

@@ -70,23 +70,45 @@ if (!fs.statSync(gitHooksDirectoryPath).isDirectory()) {
   return;
 }
 
-Object.keys(parentModulePackage.scripts)
-  .forEach(function (scriptName) {
+var githookNames = Object
+  .keys(parentModulePackage.scripts)
+  .map(function (scriptName) {
     var scriptParts = /^githook:(.+)$/.exec(scriptName);
     if (!scriptParts) {
       debug(scriptName + ' ignored');
-      return;
+      return false;
     }
-
     var hookName = scriptParts[1];
-    if (VALID_HOOK_NAMES.indexOf(hookName) === -1) {
-      console.warn('githook-scripts: "' + hookName + '" is not a valid git hook, ignoring');
-      return;
-    }
-
-    var hookPath = path.join(gitHooksDirectoryPath, hookName);
-    fs.writeFileSync(hookPath, '#!/bin/bash\n npm run ' + scriptName + ' "$@"');
-    fs.chmodSync(hookPath, '755');
-    debug('wrote ', hookPath);
-    console.log('githook-scripts: added hook ' + hookName);
+    return hookName;
+  })
+  .filter(function (hookName) {
+    return !!hookName;
   });
+
+if (githookNames.length <= 0) {
+  console.warn('githook-scripts: no githook scripts found, add one (ex: "githook:pre-commit") to package.json and run "npm rebuild githook-scripts" to activate');
+  return;
+}
+
+var validGithookNames = githookNames.filter(function (hookName) {
+  if (VALID_HOOK_NAMES.indexOf(hookName) >= 0) {
+    return true;
+  }
+  console.warn('githook-scripts: "' + hookName + '" is not a valid git hook, ignoring');
+  return false;
+});
+
+if (validGithookNames.length <= 0) {
+  console.warn('githook-scripts: no valid githook scripts found, add one (ex: "githook:pre-commit") to package.json and run "npm rebuild githook-scripts" to activate');
+  console.warn('githook-scripts: valid githooks are ' + VALID_HOOK_NAMES.join(', '));
+  return;
+}
+
+validGithookNames.forEach(function (hookName) {
+  var hookPath = path.join(gitHooksDirectoryPath, hookName);
+  var scriptName = 'githook:' + hookName;
+  fs.writeFileSync(hookPath, '#!/bin/bash\n npm run ' + scriptName + ' "$@"');
+  fs.chmodSync(hookPath, '755');
+  debug('wrote ', hookPath);
+  console.log('githook-scripts: added hook ' + hookName);
+});

test/_utils.js

@@ -1,6 +1,7 @@
 var fs = require('fs');
 var path = require('path');
 var execSync = require('child_process').execSync;
+var spawnSync = require('child_process').spawnSync;
 
 var debug = require('debug')('githook-scripts:test-utils');
 var rimraf = require('rimraf');
@@ -54,8 +55,17 @@ function setup(scripts, opts) {
       { PATH: BROKEN_GIT_LOCATION + (process.platform === 'win32' ? ';' : ':') + process.env.PATH }
     );
   }
-  debug('npm install: ' + execSync('npm --loglevel=silent install', npmExecOpts).toString());
+  var npmInstallResults = spawnSync('npm', ['--loglevel=silent', 'install'], npmExecOpts);
+  var rawLogs = '' + npmInstallResults.stdout + '\n' + npmInstallResults.stderr;
+  debug('npm install: ' + rawLogs);
 
   debug('setup finished');
+
+  return rawLogs
+    .split('\n')
+    .filter(function (line) {
+      debug(line.startsWith('githook-scripts:') + ' ' + line);
+      return line.startsWith('githook-scripts:');
+    });
 }
 module.exports.setup = setup;

test/no-scripts.spec.js

@@ -7,9 +7,21 @@ var expect = chai.expect;
 chai.use(chaiFiles);
 
 describe('no scripts', function () {
-  after(utils.teardown);
+  afterEach(utils.teardown);
 
   it('should not fail if a package.json does not have a scripts field', function () {
     expect(function () { utils.setup(undefined); }).not.to.throw();
   });
+
+  it('should warn if a package.json does not have a scripts field', function () {
+    var logLines = utils.setup(undefined);
+    expect(logLines).to.have.length(1);
+    expect(logLines[0]).to.equal('githook-scripts: no "scripts" field in package.json, skipping');
+  });
+
+  it('should warn if a package.json does not have any scripts defined', function () {
+    var logLines = utils.setup({ });
+    expect(logLines).to.have.length(1);
+    expect(logLines[0]).to.equal('githook-scripts: no githook scripts found, add one (ex: "githook:pre-commit") to package.json and run "npm rebuild githook-scripts" to activate');
+  });
 });

test/no-valid-hook-scripts.spec.js

@@ -0,0 +1,27 @@
+var utils = require('./_utils.js');
+
+var chai = require('chai');
+var expect = chai.expect;
+
+describe('no valid hook scripts', function () {
+  afterEach(utils.teardown);
+
+  it('should warn that no hooks were valid', function () {
+    var logLines = utils.setup({ 'githook:not-a-valid-hook': 'exit 1' });
+    expect(logLines).to.have.length(3);
+    expect(logLines[0]).to.equal('githook-scripts: "not-a-valid-hook" is not a valid git hook, ignoring');
+  });
+
+  it('should say what valid hooks are and how to activate them when added', function () {
+    var logLines = utils.setup({ 'githook:not-a-valid-hook': 'exit 1' });
+    expect(logLines).to.have.length(3);
+    expect(logLines[1]).to.equal('githook-scripts: no valid githook scripts found, add one (ex: "githook:pre-commit") to package.json and run "npm rebuild githook-scripts" to activate');
+    expect(logLines[2]).to.equal('githook-scripts: valid githooks are applypatch-msg, pre-applypatch, post-applypatch, pre-commit, prepare-commit-msg, commit-msg, post-commit, pre-rebase, post-checkout, post-merge, pre-receive, update, post-receive, post-update, pre-auto-gc, post-rewrite, pre-push');
+  });
+
+  it('should warn if a package.json does not have any githook scripts defined', function () {
+    var logLines = utils.setup({ test: 'jest' });
+    expect(logLines).to.have.length(1);
+    expect(logLines[0]).to.equal('githook-scripts: no githook scripts found, add one (ex: "githook:pre-commit") to package.json and run "npm rebuild githook-scripts" to activate');
+  });
+});