Refactor of the cmd.js - Still a WIP, but looking for comments

bin/cmd.js

@@ -2,17 +2,9 @@
 
 'use strict'
 
-const exec = require('child_process').exec
-const fs = require('fs')
-const http = require('http')
-const https = require('https')
-const url = require('url')
 const nopt = require('nopt')
 const path = require('path')
-const pretty = require('../lib/format-pretty')
-const formatTap = require('../lib/format-tap')
 const Validator = require('../lib')
-const Tap = require('../lib/tap')
 const utils = require('../lib/utils')
 const subsystem = require('../lib/rules/subsystem')
 const knownOpts = { help: Boolean
@@ -35,125 +27,60 @@ const shortHand = { h: ['--help']
 const parsed = nopt(knownOpts, shortHand)
 const usage = require('help')()
 
+// The --help or -h flag was used
 if (parsed.help) {
   return usage()
 }
 
+// The --version or -v flag was used
 if (parsed.version) {
   console.log('core-validate-commit', 'v' + require('../package').version)
   return
 }
 
-const args = parsed.argv.remain
-if (!args.length)
-  args.push('HEAD')
-
-function load(sha, cb) {
-  const parsed = url.parse(sha)
-  if (parsed.protocol) {
-    return loadPatch(parsed, cb)
-  }
-
-  exec(`git show --quiet --format=medium ${sha}`, (err, stdout, stderr) => {
-    if (err) return cb(err)
-    cb(null, stdout.trim())
-  })
+// The --list-subsytems or --ls flag was used
+if (parsed['list-subsystems']) {
+  utils.describeSubsystem(subsystem.defaults.subsystems.sort())
+  return
 }
 
-function loadPatch(uri, cb) {
-  let h = http
-  if (~uri.protocol.indexOf('https')) {
-    h = https
-  }
-  uri.headers = {
-    'user-agent': 'core-validate-commit'
-  }
-  h.get(uri, (res) => {
-    let buf = ''
-    res.on('data', (chunk) => {
-      buf += chunk
-    })
+// any arguments after a --flag will be in the remain array
+const args = parsed.argv.remain
 
-    res.on('end', () => {
-      try {
-        const out = JSON.parse(buf)
-        cb(null, out)
-      } catch (err) {
-        cb(err)
-      }
-    })
-  }).on('error', cb)
-}
+// The argument should be the commit you are validating
+// If there is no args,  then use the HEAD commit
+if (!args.length)
+  args.push('HEAD')
 
+// Create a new Validator
 const v = new Validator(parsed)
 
-if (parsed['list-subsystems']) {
-  utils.describeSubsystem(subsystem.defaults.subsystems.sort())
-  return
-}
-
+// The --list or -l flag was used
 if (parsed.list) {
+  // Get the list of Rule names
   const ruleNames = Array.from(v.rules.keys())
+  // Find the length of the longest Rule names
   const max = ruleNames.reduce((m, item) => {
     if (item.length > m) m = item.length
     return m
   }, 0)
-
+  // Loop through and output the rules
   for (const rule of v.rules.values()) {
     utils.describeRule(rule, max)
   }
   return
 }
 
+// The --tap or -t flag was used
 if (parsed.tap) {
-  const tap = new Tap()
-  tap.pipe(process.stdout)
-  if (parsed.out) tap.pipe(fs.createWriteStream(parsed.out))
-  let count = 0
-  let total = args.length
-
-  v.on('commit', (c) => {
-    count++
-    const test = tap.test(c.commit.sha)
-    formatTap(test, c.commit, c.messages, v)
-    if (count === total) {
-      setImmediate(() => {
-        tap.end()
-        if (tap.status === 'fail')
-          process.exitCode = 1
-      })
-    }
+  utils.parseTap(v, parsed, args, (code) => {
+    process.exitCode = code
+    return
   })
-
-  function run() {
-    if (!args.length) return
-    const sha = args.shift()
-    load(sha, (err, data) => {
-      if (err) throw err
-      v.lint(data)
-      run()
-    })
-  }
-
-  run()
-
 } else {
-  v.on('commit', (c) => {
-    pretty(c.commit, c.messages, v)
-    run()
+  // no --flags used,  defaults to --validate-metadata
+  utils.validateMetadata(v, args, (code) => {
+    process.exitCode = code
+    return
   })
-
-  function run() {
-    if (!args.length) {
-      process.exitCode = v.errors
-      return
-    }
-    const sha = args.shift()
-    load(sha, (err, data) => {
-      if (err) throw err
-      v.lint(data)
-    })
-  }
-
-  run()
 }

lib/utils.js

@@ -1,5 +1,13 @@
 'use strict'
 
+const fs = require('fs')
+const exec = require('child_process').exec
+const http = require('http')
+const https = require('https')
+const url = require('url')
+const formatTap = require('../lib/format-tap')
+const Tap = require('../lib/tap')
+const pretty = require('../lib/format-pretty')
 const chalk = require('chalk')
 const CHECK = chalk.green('✔')
 const X = chalk.red('✖')
@@ -57,3 +65,100 @@ exports.describeSubsystem = function describeSubsystem(subsystems, max = 20) {
     }
   }
 }
+
+// Given a commit hash, load it
+// If a URL is passed in, then load the commit remotely
+// If not, then do a git show
+exports.load = function load(sha, cb) {
+  const parsed = url.parse(sha)
+  if (parsed.protocol) {
+    return exports.loadPatch(parsed, cb)
+  }
+
+  exec(`git show --quiet --format=medium ${sha}`, (err, stdout, stderr) => {
+    if (err) return cb(err)
+    cb(null, stdout.trim())
+  })
+}
+
+// Load the commit from a URL
+exports.loadPatch = function loadPatch(uri, cb) {
+  let h = http
+  if (~uri.protocol.indexOf('https')) {
+    h = https
+  }
+  uri.headers = {
+    'user-agent': 'core-validate-commit'
+  }
+  h.get(uri, (res) => {
+    let buf = ''
+    res.on('data', (chunk) => {
+      buf += chunk
+    })
+
+    res.on('end', () => {
+      try {
+        const out = JSON.parse(buf)
+        cb(null, out)
+      } catch (err) {
+        cb(err)
+      }
+    })
+  }).on('error', cb)
+}
+
+exports.validateMetadata = function(validator, args, cb) {
+  validator.on('commit', (c) => {
+    pretty(c.commit, c.messages, validator)
+    run()
+  })
+
+  function run() {
+    if (!args.length) {
+      return cb(validator.errors)
+    }
+    const sha = args.shift()
+    exports.load(sha, (err, data) => {
+      if (err) throw err
+      validator.lint(data)
+    })
+  }
+
+  run()
+}
+
+exports.parseTap = function(validator, parsed, args, cb) {
+  const tap = new Tap()
+  tap.pipe(process.stdout)
+  if (parsed.out) tap.pipe(fs.createWriteStream(parsed.out))
+  let count = 0
+  let total = args.length
+
+  validator.on('commit', (c) => {
+    count++
+    const test = tap.test(c.commit.sha)
+    formatTap(test, c.commit, c.messages, validator)
+    if (count === total) {
+      setImmediate(() => {
+        tap.end()
+        if (tap.status === 'fail') {
+          return cb(1)
+        }
+
+        return cb(0)
+      })
+    }
+  })
+
+  function run() {
+    if (!args.length) return
+    const sha = args.shift()
+    exports.load(sha, (err, data) => {
+      if (err) throw err
+      validator.lint(data)
+      run()
+    })
+  }
+
+  run()
+}

package.json

@@ -18,6 +18,8 @@
   "devDependencies": {
     "check-pkg": "^2.0.2",
     "lintit": "^6.0.1",
+    "nock": "~10.0.6",
+    "proxyquire": "^2.1.1",
     "tap": "^12.6.1"
   },
   "files": [

test/utils-test.js

@@ -0,0 +1,104 @@
+'use strict'
+
+const { test } = require('tap')
+const proxyquire = require('proxyquire')
+const nock = require('nock')
+
+const commitHash = 'a12b34c56'
+
+test('Test util functions', (t) => {
+  t.test('test sha load function', (tt) => {
+    const commitMessage = 'Commit Message'
+
+    const utils = proxyquire('../lib/utils', {
+      'child_process': {
+        exec: (cmd, cb) => {
+          tt.equals(cmd,
+                    `git show --quiet --format=medium ${commitHash}`,
+                    'cmd should be equal')
+          cb(null, commitMessage)
+        }
+      }
+    })
+
+    utils.load(commitHash, (err, stdout, stderr) => {
+      tt.equals(err, null, 'Should not be an error')
+      tt.equals(commitMessage, stdout, 'should have the commit message')
+      tt.end()
+    })
+  })
+
+  t.test('test sha load function with error', (tt) => {
+    const utils = proxyquire('../lib/utils', {
+      'child_process': {
+        exec: (cmd, cb) => {
+          cb('Error', null)
+        }
+      }
+    })
+
+    utils.load(commitHash, (err, stdout, stderr) => {
+      tt.equals(err, 'Error', 'should have the error message')
+      tt.end()
+    })
+  })
+
+  t.test('test load patch function using http', (tt) => {
+    const commitUrl = `http://api.github.com/repos/nodejs/${commitHash}`
+    const util = require('../lib/utils')
+
+    nock('http://api.github.com', {
+      reqheaders: {
+        'user-agent': 'core-validate-commit'
+      }
+    })
+      .get(`/repos/nodejs/${commitHash}`)
+      .reply(200, {commit: 'message'})
+
+    util.load(commitUrl, (err, commitMessage) => {
+      tt.equals(err, null, 'Should not be an error')
+      tt.pass()
+      tt.end()
+    })
+  })
+
+  t.test('test load patch function using https', (tt) => {
+    const commitUrl = `https://api.github.com/repos/nodejs/${commitHash}`
+    const util = require('../lib/utils')
+
+    nock('https://api.github.com', {
+      reqheaders: {
+        'user-agent': 'core-validate-commit'
+      }
+    })
+      .get(`/repos/nodejs/${commitHash}`)
+      .reply(200, {commit: 'message'})
+
+    util.load(commitUrl, (err, commitMessage) => {
+      tt.equals(err, null, 'Should not be an error')
+      tt.pass()
+      tt.end()
+    })
+  })
+
+  t.test('test load patch function - catch parse error', (tt) => {
+    const commitUrl = `http://api.github.com/repos/nodejs/${commitHash}`
+    const util = require('../lib/utils')
+
+    nock('http://api.github.com', {
+      reqheaders: {
+        'user-agent': 'core-validate-commit'
+      }
+    })
+      .get(`/repos/nodejs/${commitHash}`)
+      .reply(200, '{commit: \'message\'}')
+
+    util.load(commitUrl, (err, commitMessage) => {
+      tt.true(err)
+      tt.pass()
+      tt.end()
+    })
+  })
+
+  t.end()
+})