From 524aff5ed5d1390c12d14be4f3fb02e067f6a94c Mon Sep 17 00:00:00 2001 From: Evan Lucas Date: Wed, 20 Jul 2016 21:36:19 -0500 Subject: [PATCH 01/11] lib: add node-specific rules --- lib/rules/fixes-url.js | 53 +++++++++++++++++++++ lib/rules/index.js | 13 +++++ lib/rules/line-length.js | 38 +++++++++++++++ lib/rules/pr-url.js | 46 ++++++++++++++++++ lib/rules/reviewers.js | 37 +++++++++++++++ lib/rules/subsystem.js | 99 +++++++++++++++++++++++++++++++++++++++ lib/rules/title-length.js | 31 ++++++++++++ 7 files changed, 317 insertions(+) create mode 100644 lib/rules/fixes-url.js create mode 100644 lib/rules/index.js create mode 100644 lib/rules/line-length.js create mode 100644 lib/rules/pr-url.js create mode 100644 lib/rules/reviewers.js create mode 100644 lib/rules/subsystem.js create mode 100644 lib/rules/title-length.js diff --git a/lib/rules/fixes-url.js b/lib/rules/fixes-url.js new file mode 100644 index 0000000..08a4a94 --- /dev/null +++ b/lib/rules/fixes-url.js @@ -0,0 +1,53 @@ +'use strict' + +const id = 'fixes-url' + +module.exports = { + id: id +, meta: { + description: 'enforce format of Fixes URLs' + , recommended: true + } +, defaults: {} +, options: {} +, validate: (context, rule) => { + const parsed = context.toJSON() + if (!Array.isArray(parsed.fixes)) return + if (!parsed.fixes.length) return + for (const url of parsed.fixes) { + if (url[0] === '#') { + // See nodejs/node#2aa376914b621018c5784104b82c13e78ee51307 + // for an example + const { line, column } = findLineAndColumn(context.body, url) + context.report({ + id: id + , message: 'Fixes must be a url, not an issue number.' + , string: url + , line: line + , column: column + , level: 'error' + }) + } + } + } +} + +function findLineAndColumn(body, str) { + for (var i = 0; i < body.length; i++) { + const l = body[i] + if (~l.indexOf('Fixes')) { + const idx = l.indexOf(str) + if (idx !== -1) { + return { + line: i + , column: idx + } + } + } + } + + return { + line: -1 + , column: -1 + } +} diff --git a/lib/rules/index.js b/lib/rules/index.js new file mode 100644 index 0000000..6bdb19d --- /dev/null +++ b/lib/rules/index.js @@ -0,0 +1,13 @@ +'use strict' + +const fs = require('fs') +const path = require('path') +const BaseRule = require('../rule') + +fs.readdirSync(__dirname).forEach((item) => { + const fp = path.join(__dirname, item) + if (path.extname(fp) !== '.js') return + if (fp === __filename) return + const rule = require(fp) + exports[rule.id] = rule +}) diff --git a/lib/rules/line-length.js b/lib/rules/line-length.js new file mode 100644 index 0000000..9257990 --- /dev/null +++ b/lib/rules/line-length.js @@ -0,0 +1,38 @@ +'use strict' + +const id = 'line-length' + +module.exports = { + id: id +, meta: { + description: 'enforce max length of lines in commit body' + , recommended: true + } +, defaults: { + length: 72 + } +, options: { + length: 72 + } +, validate: (context, rule) => { + const len = rule.options.length + const parsed = context.toJSON() + // release commits include the notable changes from the changelog + // in the commit message + if (parsed.release) return + for (let i = 0; i < parsed.body.length; i++) { + const line = parsed.body[i] + if (line.length > len) { + context.report({ + id: id + , message: `Line should be <= ${len} columns.` + , string: line + , maxLength: len + , line: i + , column: len + , level: 'error' + }) + } + } + } +} diff --git a/lib/rules/pr-url.js b/lib/rules/pr-url.js new file mode 100644 index 0000000..cf1772d --- /dev/null +++ b/lib/rules/pr-url.js @@ -0,0 +1,46 @@ +'use strict' + +const id = 'pr-url' + +module.exports = { + id: id +, meta: { + description: 'enforce PR-URL' + , recommended: true + } +, defaults: {} +, options: {} +, validate: (context, rule) => { + if (!context.prUrl) { + context.report({ + id: id + , message: 'Commit must have a PR-URL.' + , string: context.prUrl + , line: 0 + , column: 0 + , level: 'error' + }) + } + if (context.prUrl && context.prUrl[0] === '#') { + // see nodejs/node#7d3a7ea0d7df9b6f11df723dec370f49f4f87e99 + // for an example + var line = -1 + var column = -1 + for (var i = 0; i < context.body.length; i++) { + const l = context.body[i] + if (~l.indexOf('PR-URL') && ~l.indexOf(context.prUrl)) { + line = i + column = l.indexOf(context.prUrl) + } + } + context.report({ + id: id + , message: 'PR-URL must be a url, not a pr number.' + , string: context.prUrl + , line: line + , column: column + , level: 'error' + }) + } + } +} diff --git a/lib/rules/reviewers.js b/lib/rules/reviewers.js new file mode 100644 index 0000000..5ebe123 --- /dev/null +++ b/lib/rules/reviewers.js @@ -0,0 +1,37 @@ +'use strict' + +const id = 'reviewers' + +module.exports = { + id: id +, meta: { + description: 'enforce having reviewers' + , recommended: true + } +, defaults: {} +, options: {} +, validate: (context, rule) => { + const reviewers = rule.options.reviewers + const parsed = context.toJSON() + // release commits generally won't have any reviewers + if (parsed.release) return + + if (!Array.isArray(parsed.reviewers) || !parsed.reviewers.length) { + // See nodejs/node#5aac4c42da104c30d8f701f1042d61c2f06b7e6c + // for an example + return context.report({ + id: id + , message: 'Commit must have at least 1 reviewer.' + , string: null + , line: 0 + , column: 0 + , level: 'error' + }) + } + + // TODO(evanlucas) verify that each reviewer is a collaborator + // This will probably be easier to do once we move gitlint-parser-node + // over to using an array of objects with parsed reviewers vs an array + // of strings + } +} diff --git a/lib/rules/subsystem.js b/lib/rules/subsystem.js new file mode 100644 index 0000000..7007f6b --- /dev/null +++ b/lib/rules/subsystem.js @@ -0,0 +1,99 @@ +'use strict' + +const id = 'subsystem' + +const validSubsystems = [ + 'benchmark' +, 'build' +, 'deps' +, 'doc' +, 'lib' +, 'node' +, 'src' +, 'test' +, 'tools' + +// core libs +, 'assert' +, 'async_wrap' +, 'buffer' +, 'child_process' +, 'cluster' +, 'console' +, 'constants' +, 'crypto' +, 'debugger' +, 'dgram' +, 'dns' +, 'domain' +, 'events' +, 'fs' +, 'http' +, 'https' +, 'module' +, 'net' +, 'os' +, 'path' +, 'process' +, 'punycode' +, 'querystring' +, 'readline' +, 'repl' +, 'stream' +, 'string_decoder' +, 'sys' +, 'timers' +, 'tls' +, 'tty' +, 'url' +, 'util' +, 'v8' +, 'vm' +, 'zlib' +] + +module.exports = { + id: id +, meta: { + description: 'enforce subsystem validity' + , recommended: true + } +, defaults: { + subsystems: validSubsystems + } +, options: { + subsystems: validSubsystems + } +, validate: (context, rule) => { + const subs = rule.options.subsystems + const parsed = context.toJSON() + if (!parsed.subsystems.length) { + if (!parsed.release && !parsed.working) { + // Missing subsystem + context.report({ + id: id + , message: 'Missing subsystem.' + , string: parsed.title + , line: 0 + , column: 0 + , level: 'error' + }) + } + } else { + for (const sub of parsed.subsystems) { + if (!~subs.indexOf(sub)) { + // invalid subsystem + const column = parsed.title.indexOf(sub) + context.report({ + id: id + , message: `Invalid subsystem: "${sub}".` + , string: parsed.title + , line: 0 + , column: column + , level: 'warning' + }) + } + } + } + } +} diff --git a/lib/rules/title-length.js b/lib/rules/title-length.js new file mode 100644 index 0000000..24c946a --- /dev/null +++ b/lib/rules/title-length.js @@ -0,0 +1,31 @@ +'use strict' + +const id = 'title-length' + +module.exports = { + id: id +, meta: { + description: 'enforce max length of commit title' + , recommended: true + } +, defaults: { + length: 50 + } +, options: { + length: 50 + } +, validate: (context, rule) => { + const len = rule.options.length + if (context.title.length > len) { + context.report({ + id: id + , message: `Title must be <= ${len} columns.` + , string: context.title + , maxLength: len + , line: 0 + , column: len + , level: 'error' + }) + } + } +} From 204a279f00d86f14ae8333ea179b4cefe5bf69ce Mon Sep 17 00:00:00 2001 From: Evan Lucas Date: Wed, 20 Jul 2016 21:36:36 -0500 Subject: [PATCH 02/11] lib: add BaseRule --- lib/rule.js | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 lib/rule.js diff --git a/lib/rule.js b/lib/rule.js new file mode 100644 index 0000000..aece7ed --- /dev/null +++ b/lib/rule.js @@ -0,0 +1,30 @@ +'use strict' + +module.exports = class Rule { + constructor(opts) { + opts = Object.assign({ + options: {} + , defaults: {} + , meta: {} + }, opts) + + if (!opts.id) { + throw new Error('Rule must have an id') + } + + if (typeof opts.validate !== 'function') { + throw new TypeError('Rule must have validate function') + } + + this.id = opts.id + this.disabled = opts.disabled === true + this.meta = opts.meta + this.defaults = Object.assign({}, opts.defaults) + this.options = Object.assign({}, opts.defaults, opts.options) + this._validate = opts.validate + } + + validate(commit) { + this._validate(commit, this) + } +} From 48606d13495b3c3025ef8576ace6ab8ed1c02d59 Mon Sep 17 00:00:00 2001 From: Evan Lucas Date: Wed, 20 Jul 2016 21:36:50 -0500 Subject: [PATCH 03/11] lib: add utils --- lib/utils.js | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 lib/utils.js diff --git a/lib/utils.js b/lib/utils.js new file mode 100644 index 0000000..45b5177 --- /dev/null +++ b/lib/utils.js @@ -0,0 +1,29 @@ +'use strict' + +const chalk = require('chalk') +const CHECK = chalk.green('✔') +const X = chalk.red('✖') +const WARN = chalk.yellow('⚠') + +exports.CHECK = CHECK +exports.X = X +exports.WARN = WARN + +exports.rightPad = function rightPad(str, max) { + const diff = max - str.length + 1 + if (diff > 0) { + return `${str}${' '.repeat(diff)}` + } + return str +} + +exports.header = (sha, status) => { + switch (status) { + case 'pass': + return `${CHECK} ${chalk.underline(sha)}` + case 'warn': + return `${WARN} ${chalk.underline(sha)}` + case 'error': + return `${X} ${chalk.underline(sha)}` + } +} From d2d1c5a58a3e5b810645e82ada9a982eefe50c48 Mon Sep 17 00:00:00 2001 From: Evan Lucas Date: Wed, 20 Jul 2016 21:37:05 -0500 Subject: [PATCH 04/11] lib: add pretty formatter --- lib/format-pretty.js | 88 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 lib/format-pretty.js diff --git a/lib/format-pretty.js b/lib/format-pretty.js new file mode 100644 index 0000000..0af5364 --- /dev/null +++ b/lib/format-pretty.js @@ -0,0 +1,88 @@ +'use strict' + +const chalk = require('chalk') +const utils = require('./utils') + +const MAX_LINE_COL_LEN = 6 + +module.exports = function formatPretty(context, msgs, validator, opts) { + opts = Object.assign({ + detailed: false + }, opts) + + if (!msgs.length) { + console.log(' ', utils.header(context.sha, 'pass')) + return + } + + let level = 'warn' + for (const msg of msgs) { + if (msg.level === 'error') level = 'error' + } + + msgs.sort((a, b) => { + if (a.line === b.line) { + return a.column < b.column + ? -1 + : a.column > b.column + ? 1 + : 0 + } + + return a.line < b.line + ? -1 + : a.line > b.line + ? 1 + : 0 + }) + + console.log(' ', utils.header(context.sha, level)) + + for (const msg of msgs) { + const ruleId = msg.id + const rule = validator.rules.get(ruleId) + if (!rule) { + throw new Error(`Invalid rule: "${ruleId}"`) + } + + switch (ruleId) { + case 'title-length': + case 'line-length': + console.log(formatLength(msg, opts)) + break + default: + const m = formatDesc(msg.message) + console.log(formatMessage(msg)) + break + } + } +} + +function formatLength(msg, opts) { + const out = formatMessage(msg) + const str = msg.string + const l = str.length + if (!opts.detailed) return out + const diff = str.slice(0, msg.column) + chalk.red(str.slice(msg.column, l)) + return `${out} + ${diff}` +} + +function formatMessage(msg) { + const pad = utils.rightPad(`${msg.line}:${msg.column}`, MAX_LINE_COL_LEN) + const line = chalk.grey(pad) + const id = formatId(msg.id) + const m = msg.message + const icon = msg.level === 'error' + ? utils.X + : utils.WARN + return ` ${icon} ${line} ${utils.rightPad(m, 40)} ${id}` +} + +function formatId(id) { + return chalk.red(id) +} + +function formatDesc(str) { + return utils.rightPad(str || '', 40) +} From 6dbf7867240b751ff8fe1050e926f4a3f39420f6 Mon Sep 17 00:00:00 2001 From: Evan Lucas Date: Wed, 20 Jul 2016 21:37:56 -0500 Subject: [PATCH 05/11] lib: add new validator This is a pretty big refactor but should make this more maintainable --- lib/index.js | 110 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+) create mode 100644 lib/index.js diff --git a/lib/index.js b/lib/index.js new file mode 100644 index 0000000..40faa65 --- /dev/null +++ b/lib/index.js @@ -0,0 +1,110 @@ +'use strict' + +const EE = require('events') +const fs = require('fs') +const path = require('path') +const Parser = require('gitlint-parser-node') +const BaseRule = require('./rule') +const RULES = require('./rules') + +module.exports = class ValidateCommit extends EE { + constructor(options) { + super() + + this.opts = Object.assign({ + 'validate-metadata': true + }, options) + + this.messages = new Map() + this.errors = 0 + + this.rules = new Map() + this.loadBaseRules() + } + + loadBaseRules() { + const keys = Object.keys(RULES) + for (const key of keys) { + this.rules.set(key, new BaseRule(RULES[key])) + } + if (!this.opts['validate-metadata']) { + this.disableRule('pr-url') + this.disableRule('reviewers') + } + } + + disableRule(id) { + if (!this.rules.has(id)) { + throw new TypeError(`Invalid rule: "${id}"`) + } + + this.rules.get(id).disabled = true + } + + loadCustomRule(config) { + if (!config || typeof config !== 'object') { + throw new TypeError('rule must be an object') + } + + const rule = new BaseRule(config) + this.rules.set(rule.id, rule) + } + + loadRules(rules) { + if (!rules || typeof rules !== 'object') { + throw new TypeError('rules must be an object') + } + + const keys = Object.keys(rules) + for (const id of keys) { + const val = rules[id] + if (this.rules.has(id)) { + this.rules.get(id).options = val + } else { + const err = new Error(`Invalid rule: ${id}`) + err.code = 'ENOTFOUND' + return setImmediate(() => { + this.emit('error', err) + }) + } + } + } + + lint(str) { + if (Array.isArray(str)) { + for (const item of str) { + this.lint(item) + } + } else { + const commit = new Parser(str, this) + for (const rule of this.rules.values()) { + if (rule.disabled) continue + rule.validate(commit) + } + + setImmediate(() => { + this.emit('commit', { + commit: commit + , messages: this.messages.get(commit.sha) || [] + }) + }) + } + } + + report(opts) { + const commit = opts.commit + const sha = commit.sha + if (!sha) { + throw new Error('Invalid report. Missing commit sha') + } + + if (opts.data.level === 'error') + this.errors++ + const ar = this.messages.get(sha) || [] + ar.push(opts.data) + this.messages.set(sha, ar) + setImmediate(() => { + this.emit('message', opts) + }) + } +} From aea79483504fa3ab811b173886946c2b4550b96b Mon Sep 17 00:00:00 2001 From: Evan Lucas Date: Wed, 20 Jul 2016 21:38:25 -0500 Subject: [PATCH 06/11] package: switch bin to use new Validator --- bin/cmd.js | 118 ++++++++++++++++-------- commit.js | 260 ----------------------------------------------------- diff.js | 30 ------- format.js | 99 -------------------- index.js | 55 +----------- info.js | 14 --- 6 files changed, 81 insertions(+), 495 deletions(-) delete mode 100644 commit.js delete mode 100644 diff.js delete mode 100644 format.js delete mode 100644 info.js diff --git a/bin/cmd.js b/bin/cmd.js index 2614c55..e5b29ea 100755 --- a/bin/cmd.js +++ b/bin/cmd.js @@ -3,53 +3,95 @@ 'use strict' const exec = require('child_process').exec -const chalk = require('chalk') -const args = process.argv.splice(2) -const help = require('help')() -const Commit = require('../commit') -const format = require('../format') - -if (!args.length) return help() -for (const arg of args) { - if (arg === 'help' || arg === '--help' || arg === '-h') { - return help() - } +const http = require('http') +const https = require('https') +const url = require('url') +const nopt = require('nopt') - if (arg === 'version' || arg === '--version' || arg === '-v') { - console.log('core-validate-commit', 'v' + require('../package').version) - return - } +const pretty = require('../lib/format-pretty') +const Validator = require('../lib') + +const knownOpts = { help: Boolean + , version: Boolean + , 'validate-metadata': Boolean + } +const shortHand = { h: ['--help'] + , v: ['--version'] + , V: ['--validate-metadata'] + } + +const parsed = nopt(knownOpts, shortHand) +const usage = require('help')() + +if (parsed.help) { + return usage() +} + +if (parsed.version) { + console.log('core-validate-commit', 'v' + require('../package').version) + return } +const args = parsed.argv.remain + function getCommitCommand(sha) { return `git show --quiet ${sha}` } -;(function run() { - if (!args.length) return - const sha = args.shift() - exec(getCommitCommand(sha), (err, stdout, stderr) => { - if (err) throw err - const c = new Commit(stdout) - - if (c.errors.length) { - console.log() - console.log(format.header(c)) - } - - c.errors.forEach((m) => { - if (format[m.code]) { - console.error(format[m.code](m, c)) - } else { - console.error(format.default(m, c)) - } - process.exitCode = 1 +function load(sha, cb) { + const parsed = url.parse(sha) + if (parsed.protocol) { + return loadPatch(parsed, cb) + } + + exec(`git show --quiet ${sha}`, (err, stdout, stderr) => { + if (err) return cb(err) + cb(null, stdout.trim()) + }) +} + +function loadPatch(uri, cb) { + var h = http + if (~uri.protocol.indexOf('https')) { + h = https + } + uri.headers = { + 'user-agent': 'core-validate-commit' + } + h.get(uri, (res) => { + var buf = '' + res.on('data', (chunk) => { + buf += chunk }) - c.warnings.forEach((m) => { - console.error(' ', chalk.yellow(m.code), chalk.grey(m.str)) + res.on('end', () => { + try { + const out = JSON.parse(buf) + cb(null, out) + } catch (err) { + cb(err) + } }) + }).on('error', cb) +} + +const v = new Validator(parsed) - run() +v.on('commit', (c) => { + pretty(c.commit, c.messages, v) + run() +}) + +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() diff --git a/commit.js b/commit.js deleted file mode 100644 index ea23bf6..0000000 --- a/commit.js +++ /dev/null @@ -1,260 +0,0 @@ -'use strict' - -module.exports = Commit - -function Commit(str) { - if (!(this instanceof Commit)) - return new Commit(str) - - this._raw = str - this.sha = null - this.title = null - this.subsystems = [] - this.author = null - this.date = null - this.fixes = [] - this.pr = null - this.reviewers = [] - this.description = null - this.revert = false - this.errors = [] - this.warnings = [] - this.parse() -} - -const headingRE = /([^\:]+):([\s]+)(.*)/ -const revertRE = /Revert "(.*)"$/ -const workingRE = /Working on v([\d]+)\.([\d]+).([\d]+)$/ -const releaseRE = /([\d]{4})-([\d]{2})-([\d]{2}),? Version/ -const reviewedByRE = /Reviewed-By: (.*)/ -const fixesRE = /Fixes: (.*)/ -const prUrlRE = /PR-URL: (.*)/ - -const validSubsystems = [ - 'benchmark' -, 'build' -, 'deps' -, 'doc' -, 'node' -, 'src' -, 'test' -, 'tools' - -// core libs -, 'assert' -, 'async_wrap' -, 'buffer' -, 'child_process' -, 'cluster' -, 'console' -, 'constants' -, 'crypto' -, 'debugger' -, 'dgram' -, 'dns' -, 'domain' -, 'events' -, 'fs' -, 'http' -, 'https' -, 'module' -, 'net' -, 'os' -, 'path' -, 'process' -, 'punycode' -, 'querystring' -, 'readline' -, 'repl' -, 'stream' -, 'string_decoder' -, 'sys' -, 'timers' -, 'tls' -, 'tty' -, 'url' -, 'util' -, 'v8' -, 'vm' -, 'zlib' -] - -Commit.prototype.parse = function parse() { - const splits = this._raw.split('\n') - const commitLine = splits.shift() - let lineNum = 1 - this.sha = commitLine.replace('commit ', '') - - var line - while (line = splits.shift()) { - lineNum++ - line = line.trim() - if (!line) break // stop on the first empty line - const matches = line.match(headingRE) - if (matches) { - const key = matches[1].toLowerCase() - const val = matches[3] - if (key === 'date' || key === 'authordate') { - this.date = val - } else if (key === 'author') { - this.author = val - } - } - } - - const body = splits.map((item) => { - if (item.length) return item.slice(4, item.length) - return '' - }) - - this.title = body.shift() - lineNum++ - - const revert = this.isRevert() - - if (this.title.length > 50) { - if (!revert) { - this.error('ETITLETOOLONG', this.title, lineNum) - } - } - - const release = this.isReleaseCommit() - - if (!revert) { - this.subsystems = getSubsystems(this.title) - } else { - this.revert = true - // on validate, check that original commit sha is included - // in commit message - } - - if (this.subsystems.length) { - for (let i = 0; i < this.subsystems.length; i++) { - const sub = this.subsystems[i] - if (!~validSubsystems.indexOf(sub)) { - this.error('EINVALIDSUBSYSTEM', sub, lineNum) - } - } - } - - const afterTitle = body.shift() - lineNum++ - // check that it is a blank line - if (afterTitle.trim()) { - this.warn('Expected blank line after commit title.', afterTitle, lineNum) - } - - for (let i = 0; i < body.length; i++) { - const line = body[i] - lineNum++ - if (line.length > 72 && !release) { - this.error('ELINETOOLONG', line, lineNum) - } - - const reviewedBy = line.match(reviewedByRE) - if (reviewedBy) { - if (!this.pr) { - this.error('EMETAORDER', 'PR-URL should come before reviewers', lineNum) - } - this.reviewers.push(reviewedBy[1]) - continue - } - - const fixes = line.match(fixesRE) - if (fixes) { - const f = fixes[1] - if (f[0] === '#') { - this.error('EINVALIDFIXESURL', line, lineNum) - } - this.fixes.push(f) - continue - } - - const prUrl = line.match(prUrlRE) - if (prUrl) { - this.pr = prUrl[1] - if (this.pr[0] === '#') { - this.error('EINVALIDPRURL', line, lineNum) - } - continue - } - } - - if (!this.author) { - this.error('EMISSINGAUTHOR', 'Author not found', null) - } - - if (!this.date) { - this.error('EMISSINGDATE', 'Date not found', null) - } - - if (!this.pr && !release) { - this.error('EMISSINGPRURL', 'PR-URL Not found', null) - } - - if (!this.reviewers.length && !release) { - this.error('EMISSINGREVIEWERS', 'No reviewers found', null) - } - - this.description = body.join('\n') -} - -Commit.prototype.getLine = function getLine(idx) { - return this._raw.split('\n')[idx] -} - -Commit.prototype.error = function error(code, str, n) { - this.errors.push({ - code: code - , str: str - , lineNum: n - }) -} - -Commit.prototype.warn = function warn(code, str, n) { - this.warnings.push({ - code: code - , str: str - , lineNum: n - }) -} - -Commit.prototype.isRevert = function isRevert() { - return revertRE.test(this.title) -} - -Commit.prototype.isWorkingCommit = function isWorkingCommit() { - return workingRE.test(this.title) -} - -Commit.prototype.isReleaseCommit = function isReleaseCommit() { - return releaseRE.test(this.title) -} - -function getSubsystems(str) { - const colon = str.indexOf(':') - if (colon === -1) { - return [] - } - - const subStr = str.slice(0, colon) - const subs = subStr.split(',') - return subs.map((item) => { - return item.trim() - }) -} - -Commit.prototype.toJSON = function toJSON() { - return { - sha: this.sha - , title: this.title - , subsystems: this.subsystems - , author: this.author - , date: this.date - , fixes: this.fixes - , pr: this.pr - , reviewers: this.reviewers - , description: this.description - , revert: this.revert - } -} diff --git a/diff.js b/diff.js deleted file mode 100644 index 334c4ad..0000000 --- a/diff.js +++ /dev/null @@ -1,30 +0,0 @@ -'use strict' - -const chalk = require('chalk') - -exports.ETITLETOOLONG = (str) => { - const l = str.length - return str.slice(0, 50) + chalk.red(str.slice(50, l)) -} - -exports.EINVALIDSUBSYSTEM = (sub, title) => { - const l = title.length - const idx = title.indexOf(sub) - const len = sub.length - const str = title - const end = len + idx - return str.slice(0, idx) + chalk.red(str.slice(idx, end)) + str.slice(end, l) -} - -exports.ELINETOOLONG = (str) => { - const l = str.length - return str.slice(0, 72) + chalk.red(str.slice(72, l)) -} - -exports.EINVALIDPRURL = (str) => { - return str.slice(0, 7) + chalk.red(str.slice(7, str.length)) -} - -exports.EINVALIDFIXESURL = (str) => { - return str.slice(0, 6) + chalk.red(str.slice(6, str.length)) -} diff --git a/format.js b/format.js deleted file mode 100644 index 3188dcb..0000000 --- a/format.js +++ /dev/null @@ -1,99 +0,0 @@ -'use strict' - -const chalk = require('chalk') -const diff = require('./diff') -const info = require('./info') - -const fail = chalk.red('✖') - -function line(obj) { - return obj.lineNum ? obj.lineNum : '' -} - -function rpad(str, max) { - const diff = max - str.length + 1 - if (diff > 0) { - return `${str}${' '.repeat(diff)}` - } - return str -} - -function getInfo(s) { - return rpad(info[s] || '', 40) -} - -exports.default = (obj, commit) => { - const t = chalk.red(obj.code) - const lin = obj.lineNum - ? chalk.grey(rpad(`${line(obj)}:0`, 6)) - : rpad('', 6) - - const i = getInfo(obj.code) - - return ` ${fail} ${lin} ${i} ${t}` -} - -exports.header = (commit) => { - return ` ${chalk.underline(commit.sha)}` -} - -exports.ETITLETOOLONG = (obj, commit) => { - const t = chalk.red('ETITLETOOLONG') - const lin = chalk.grey(rpad(`${line(obj)}:50`, 6)) - const str = obj.str - const i = getInfo('ETITLETOOLONG') - return ` ${fail} ${lin} ${i} ${t} - ${diff.ETITLETOOLONG(str)}` -} - -exports.EINVALIDSUBSYSTEM = (obj, commit) => { - const t = chalk.red('EINVALIDSUBSYSTEM') - const title = commit.title - const col = title.indexOf(obj.str) - const lin = chalk.grey(rpad(`${line(obj)}:${col}`, 6)) - const str = obj.str - const i = getInfo('EINVALIDSUBSYSTEM') - return ` ${fail} ${lin} ${i} ${t} - ${diff.EINVALIDSUBSYSTEM(str, title)}` -} - -exports.ELINETOOLONG = (obj, commit) => { - const t = chalk.red('ELINETOOLONG') - const lin = chalk.grey(rpad(`${line(obj)}:72`, 6)) - const str = obj.str - const i = getInfo('ELINETOOLONG') - return ` ${fail} ${lin} ${i} ${t} - ${diff.ELINETOOLONG(str)}` -} - -exports.EMISSINGPRURL = (obj, commit) => { - const t = chalk.red('EMISSINGPRURL') - const lin = chalk.grey(rpad('0:0', 6)) - const i = getInfo('EMISSINGPRURL') - return ` ${fail} ${lin} ${i} ${t}` -} - -exports.EINVALIDPRURL = (obj, commit) => { - const t = chalk.red('EINVALIDPRURL') - const lin = chalk.grey(rpad(`${line(obj)}:7`, 6)) - const i = getInfo('EINVALIDPRURL') - const str = obj.str - return ` ${fail} ${lin} ${i} ${t} - ${diff.EINVALIDPRURL(str)}` -} - -exports.EINVALIDFIXESURL = (obj, commit) => { - const t = chalk.red('EINVALIDFIXESURL') - const lin = chalk.grey(rpad(`${line(obj)}:6`, 6)) - const i = getInfo('EINVALIDFIXESURL') - const str = obj.str - return ` ${fail} ${lin} ${i} ${t} - ${diff.EINVALIDFIXESURL(str)}` -} - -exports.EMETAORDER = (obj, commit) => { - const t = chalk.red('EMETAORDER') - const lin = chalk.grey(rpad(`${line(obj)}:0`, 6)) - const i = getInfo('EMETAORDER') - return ` ${fail} ${lin} ${i} ${t}` -} diff --git a/index.js b/index.js index 8ed8b28..3541ac5 100644 --- a/index.js +++ b/index.js @@ -1,56 +1,3 @@ 'use strict' -const exec = require('child_process').exec -const Commit = require('./commit') - -module.exports = Validator - -function Validator(opts) { - if (!(this instanceof Validator)) - return new Validator(opts) - - this.opts = Object.assign({ - endRef: 'HEAD' - }, opts) - - if (!this.opts.startRef && !this.opts.sha) { - throw new Error('Either startRef or sha are required') - } -} - -Validator.prototype.validate = function validate(shas_, cb) { - const shas = shas_.slice() - const self = this - const out = [] - - ;(function run() { - if (!shas.length) return cb(null, out) - const sha = shas.shift() - self.getCommit(sha, (err, res) => { - if (err) return cb(err) - out.push(res) - run() - }) - })() -} - -Validator.prototype.getCommit = function getCommit(sha, cb) { - exec(`git show --quiet ${sha}`, (err, stdout, stderr) => { - if (err) return cb(err) - const commit = new Commit(stdout) - cb(null, commit) - }) -} - -Validator.prototype.getShas = function getShas(cb) { - if (this.opts.sha) { - return setImmediate(() => { - cb(null, [this.opts.sha]) - }) - } - const cmd = `git rev-list ${this.opts.startRef}..${this.opts.endRef}` - exec(cmd, (err, stdout, stderr) => { - if (err) return cb(err) - cb(null, stdout.trim().split('\n')) - }) -} +module.exports = require('./lib') diff --git a/info.js b/info.js deleted file mode 100644 index c7b3d30..0000000 --- a/info.js +++ /dev/null @@ -1,14 +0,0 @@ -'use strict' - -module.exports = { - ETITLETOOLONG: 'Title must be <= 50 columns.' -, EINVALIDSUBSYSTEM: 'Invalid subsystem.' -, ELINETOOLONG: 'Line must be <= 72 columns.' -, EINVALIDFIXESURL: 'Fixes must be a url, not an issue number.' -, EINVALIDPRURL: 'PR-URL must be a url, not a pr number.' -, EMISSINGAUTHOR: 'Commit must have an Author.' -, EMISSINGDATE: 'Commit must have a Date.' -, EMISSINGPRURL: 'Commit must have a PR-URL.' -, EMISSINGREVIEWERS: 'Commit must have at least 1 reviewer.' -, EMETAORDER: 'PR-URL should be first line of metadata.' -} From e3d0e42b754d98e393fcd39db26b6cf468c400de Mon Sep 17 00:00:00 2001 From: Evan Lucas Date: Thu, 21 Jul 2016 07:20:13 -0500 Subject: [PATCH 07/11] package: fix lint errors --- bin/cmd.js | 4 ---- lib/format-pretty.js | 5 ----- lib/index.js | 2 -- lib/rules/index.js | 1 - lib/rules/reviewers.js | 1 - 5 files changed, 13 deletions(-) diff --git a/bin/cmd.js b/bin/cmd.js index e5b29ea..793e55b 100755 --- a/bin/cmd.js +++ b/bin/cmd.js @@ -34,10 +34,6 @@ if (parsed.version) { const args = parsed.argv.remain -function getCommitCommand(sha) { - return `git show --quiet ${sha}` -} - function load(sha, cb) { const parsed = url.parse(sha) if (parsed.protocol) { diff --git a/lib/format-pretty.js b/lib/format-pretty.js index 0af5364..de88e8e 100644 --- a/lib/format-pretty.js +++ b/lib/format-pretty.js @@ -51,7 +51,6 @@ module.exports = function formatPretty(context, msgs, validator, opts) { console.log(formatLength(msg, opts)) break default: - const m = formatDesc(msg.message) console.log(formatMessage(msg)) break } @@ -82,7 +81,3 @@ function formatMessage(msg) { function formatId(id) { return chalk.red(id) } - -function formatDesc(str) { - return utils.rightPad(str || '', 40) -} diff --git a/lib/index.js b/lib/index.js index 40faa65..25be1c9 100644 --- a/lib/index.js +++ b/lib/index.js @@ -1,8 +1,6 @@ 'use strict' const EE = require('events') -const fs = require('fs') -const path = require('path') const Parser = require('gitlint-parser-node') const BaseRule = require('./rule') const RULES = require('./rules') diff --git a/lib/rules/index.js b/lib/rules/index.js index 6bdb19d..05ec7ac 100644 --- a/lib/rules/index.js +++ b/lib/rules/index.js @@ -2,7 +2,6 @@ const fs = require('fs') const path = require('path') -const BaseRule = require('../rule') fs.readdirSync(__dirname).forEach((item) => { const fp = path.join(__dirname, item) diff --git a/lib/rules/reviewers.js b/lib/rules/reviewers.js index 5ebe123..bff9cd9 100644 --- a/lib/rules/reviewers.js +++ b/lib/rules/reviewers.js @@ -11,7 +11,6 @@ module.exports = { , defaults: {} , options: {} , validate: (context, rule) => { - const reviewers = rule.options.reviewers const parsed = context.toJSON() // release commits generally won't have any reviewers if (parsed.release) return From 93ea175451d0a252777bd23e577c815ca1a30f5f Mon Sep 17 00:00:00 2001 From: Evan Lucas Date: Thu, 21 Jul 2016 07:20:24 -0500 Subject: [PATCH 08/11] test: add basic validator tests --- package.json | 2 +- test.js | 85 --------------------------------- test/validator.js | 116 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+), 86 deletions(-) delete mode 100644 test.js create mode 100644 test/validator.js diff --git a/package.json b/package.json index 8ac9c9a..36107c3 100644 --- a/package.json +++ b/package.json @@ -5,7 +5,7 @@ "main": "commit.js", "scripts": { "pretest": "lintit", - "test": "tap test.js --cov" + "test": "tap test --cov" }, "dependencies": { "chalk": "~1.1.1", diff --git a/test.js b/test.js deleted file mode 100644 index 8b686a6..0000000 --- a/test.js +++ /dev/null @@ -1,85 +0,0 @@ -'use strict' - -const test = require('tap').test -const Commit = require('./commit') - -const str = `commit e7c077c610afa371430180fbd447bfef60ebc5ea -Author: Calvin Metcalf -AuthorDate: Tue Apr 12 15:42:23 2016 -0400 -Commit: James M Snell -CommitDate: Wed Apr 20 13:28:35 2016 -0700 - - stream: make null an invalid chunk to write in object mode - - this harmonizes behavior between readable, writable, and transform - streams so that they all handle nulls in object mode the same way by - considering them invalid chunks. - - PR-URL: https://github.com/nodejs/node/pull/6170 - Reviewed-By: James M Snell - Reviewed-By: Matteo Collina -` - -const str2 = `commit b6475b9a9d0da0971eec7eb5559dff4d18a0e721 -Author: Evan Lucas -Date: Tue Mar 29 08:09:37 2016 -0500 - - Revert "tty: don't read from console stream upon creation" - - This reverts commit 461138929498f31bd35bea61aa4375a2f56cceb7. - - The offending commit broke certain usages of piping from stdin. - - Fixes: https://github.com/nodejs/node/issues/5927 - PR-URL: https://github.com/nodejs/node/pull/5947 - Reviewed-By: Matteo Collina - Reviewed-By: Alexis Campailla - Reviewed-By: Colin Ihrig -` - -/* eslint-disable */ -const str3 = `commit 75487f0db80e70a3e27fabfe323a33258dfbbea8 -Author: Michaël Zasso -Date: Fri Apr 15 13:32:36 2016 +0200 - - module: fix resolution of filename with trailing slash - - A recent optimization of module loading performance [1] forgot to check that - extensions were set in a certain code path. - - [1] https://github.com/nodejs/node/pull/5172/commits/ae18bbef48d87d9c641df85369f62cfd5ed8c250 - - Fixes: https://github.com/nodejs/node/issues/6214 - PR-URL: https://github.com/nodejs/node/pull/6215 - Reviewed-By: James M Snell - Reviewed-By: Brian White ` -/* eslint-enable */ - -test('commit', (t) => { - let c = new Commit(str) - t.equal(c.sha, 'e7c077c610afa371430180fbd447bfef60ebc5ea', 'sha') - t.equal(c.date, 'Tue Apr 12 15:42:23 2016 -0400', 'date') - t.deepEqual(c.subsystems, ['stream'], 'subsystems') - t.equal(c.pr, 'https://github.com/nodejs/node/pull/6170', 'pr') - t.equal(c.errors.length, 1, 'errors.length') - t.equal(c.errors[0].code, 'ETITLETOOLONG', 'code') - t.equal(c.revert, false, 'revert') - - c = new Commit(str2) - t.equal(c.sha, 'b6475b9a9d0da0971eec7eb5559dff4d18a0e721', 'sha') - t.equal(c.date, 'Tue Mar 29 08:09:37 2016 -0500', 'date') - t.deepEqual(c.subsystems, [], 'subsystems') - t.equal(c.pr, 'https://github.com/nodejs/node/pull/5947', 'pr') - t.equal(c.warnings.length, 0, 'warnings.length') - t.equal(c.revert, true, 'revert') - - c = new Commit(str3) - t.equal(c.sha, '75487f0db80e70a3e27fabfe323a33258dfbbea8', 'sha') - t.equal(c.date, 'Fri Apr 15 13:32:36 2016 +0200', 'date') - t.deepEqual(c.subsystems, ['module'], 'subsystems') - t.equal(c.pr, 'https://github.com/nodejs/node/pull/6215', 'pr') - t.equal(c.errors.length, 3, 'errors.length') - t.equal(c.revert, false, 'revert') - - t.end() -}) diff --git a/test/validator.js b/test/validator.js new file mode 100644 index 0000000..67552b2 --- /dev/null +++ b/test/validator.js @@ -0,0 +1,116 @@ +'use strict' + +const test = require('tap').test +const Validator = require('../') + +const str = `commit e7c077c610afa371430180fbd447bfef60ebc5ea +Author: Calvin Metcalf +AuthorDate: Tue Apr 12 15:42:23 2016 -0400 +Commit: James M Snell +CommitDate: Wed Apr 20 13:28:35 2016 -0700 + + stream: make null an invalid chunk to write in object mode + + this harmonizes behavior between readable, writable, and transform + streams so that they all handle nulls in object mode the same way by + considering them invalid chunks. + + PR-URL: https://github.com/nodejs/node/pull/6170 + Reviewed-By: James M Snell + Reviewed-By: Matteo Collina +` + +const str2 = `commit b6475b9a9d0da0971eec7eb5559dff4d18a0e721 +Author: Evan Lucas +Date: Tue Mar 29 08:09:37 2016 -0500 + + Revert "tty: don't read from console stream upon creation" + + This reverts commit 461138929498f31bd35bea61aa4375a2f56cceb7. + + The offending commit broke certain usages of piping from stdin. + + Fixes: https://github.com/nodejs/node/issues/5927 + PR-URL: https://github.com/nodejs/node/pull/5947 + Reviewed-By: Matteo Collina + Reviewed-By: Alexis Campailla + Reviewed-By: Colin Ihrig +` + +/* eslint-disable */ +const str3 = `commit 75487f0db80e70a3e27fabfe323a33258dfbbea8 +Author: Michaël Zasso +Date: Fri Apr 15 13:32:36 2016 +0200 + + module: fix resolution of filename with trailing slash + + A recent optimization of module loading performance [1] forgot to check that + extensions were set in a certain code path. + + [1] https://github.com/nodejs/node/pull/5172/commits/ae18bbef48d87d9c641df85369f62cfd5ed8c250 + + Fixes: https://github.com/nodejs/node/issues/6214 + PR-URL: https://github.com/nodejs/node/pull/6215 + Reviewed-By: James M Snell + Reviewed-By: Brian White ` +/* eslint-enable */ + +test('Validator', (t) => { + t.test('basic', (tt) => { + const v = new Validator() + v.lint(str) + v.on('commit', (data) => { + const c = data.commit + tt.equal(c.sha, 'e7c077c610afa371430180fbd447bfef60ebc5ea', 'sha') + tt.equal(c.date, 'Tue Apr 12 15:42:23 2016 -0400', 'date') + tt.deepEqual(c.subsystems, ['stream'], 'subsystems') + tt.equal(c.prUrl, 'https://github.com/nodejs/node/pull/6170', 'pr') + const msgs = data.messages + tt.equal(msgs.length, 1, 'messages.length') + tt.equal(msgs[0].level, 'error') + tt.equal(msgs[0].id, 'title-length') + tt.end() + }) + }) + + t.test('basic revert', (tt) => { + const v = new Validator() + v.lint(str2) + v.on('commit', (data) => { + const c = data.commit.toJSON() + tt.equal(c.sha, 'b6475b9a9d0da0971eec7eb5559dff4d18a0e721', 'sha') + tt.equal(c.date, 'Tue Mar 29 08:09:37 2016 -0500', 'date') + tt.deepEqual(c.subsystems, ['tty'], 'subsystems') + tt.equal(c.prUrl, 'https://github.com/nodejs/node/pull/5947', 'pr') + tt.equal(c.revert, true, 'revert') + const msgs = data.messages + tt.equal(msgs.length, 1, 'messages.length') + tt.equal(msgs[0].level, 'error') + tt.equal(msgs[0].id, 'title-length') + tt.end() + }) + }) + + t.test('more basic', (tt) => { + const v = new Validator() + v.lint(str3) + v.on('commit', (data) => { + const c = data.commit.toJSON() + tt.equal(c.sha, '75487f0db80e70a3e27fabfe323a33258dfbbea8', 'sha') + tt.equal(c.date, 'Fri Apr 15 13:32:36 2016 +0200', 'date') + tt.deepEqual(c.subsystems, ['module'], 'subsystems') + tt.equal(c.prUrl, 'https://github.com/nodejs/node/pull/6215', 'pr') + tt.equal(c.revert, false, 'revert') + const msgs = data.messages + tt.equal(msgs.length, 3, 'messages.length') + const ids = msgs.map((item) => { + return item.id + }) + const exp = ['line-length', 'line-length', 'title-length'] + tt.deepEqual(ids.sort(), exp.sort(), 'message ids') + tt.end() + }) + }) + + t.end() +}) From 42d9da311cfc83bdfd2dad12fbb14abdb362b518 Mon Sep 17 00:00:00 2001 From: Evan Lucas Date: Thu, 21 Jul 2016 08:12:17 -0500 Subject: [PATCH 09/11] validator: remove unused functions --- lib/index.js | 29 ----------------------------- 1 file changed, 29 deletions(-) diff --git a/lib/index.js b/lib/index.js index 25be1c9..3724521 100644 --- a/lib/index.js +++ b/lib/index.js @@ -39,35 +39,6 @@ module.exports = class ValidateCommit extends EE { this.rules.get(id).disabled = true } - loadCustomRule(config) { - if (!config || typeof config !== 'object') { - throw new TypeError('rule must be an object') - } - - const rule = new BaseRule(config) - this.rules.set(rule.id, rule) - } - - loadRules(rules) { - if (!rules || typeof rules !== 'object') { - throw new TypeError('rules must be an object') - } - - const keys = Object.keys(rules) - for (const id of keys) { - const val = rules[id] - if (this.rules.has(id)) { - this.rules.get(id).options = val - } else { - const err = new Error(`Invalid rule: ${id}`) - err.code = 'ENOTFOUND' - return setImmediate(() => { - this.emit('error', err) - }) - } - } - } - lint(str) { if (Array.isArray(str)) { for (const item of str) { From e5825a26870367cb182b2b50b254c64db545187d Mon Sep 17 00:00:00 2001 From: Evan Lucas Date: Thu, 21 Jul 2016 08:12:49 -0500 Subject: [PATCH 10/11] test: add rules tests Adds tests for rules and more validator tests --- test/fixtures/commit.json | 27 +++++++++++ test/fixtures/pr.json | 72 ++++++++++++++++++++++++++++++ test/rules/fixes-url.js | 39 ++++++++++++++++ test/rules/line-length.js | 68 ++++++++++++++++++++++++++++ test/rules/pr-url.js | 50 +++++++++++++++++++++ test/rules/reviewers.js | 39 ++++++++++++++++ test/validator.js | 94 +++++++++++++++++++++++++++++++++++++-- 7 files changed, 385 insertions(+), 4 deletions(-) create mode 100644 test/fixtures/commit.json create mode 100644 test/fixtures/pr.json create mode 100644 test/rules/fixes-url.js create mode 100644 test/rules/line-length.js create mode 100644 test/rules/pr-url.js create mode 100644 test/rules/reviewers.js diff --git a/test/fixtures/commit.json b/test/fixtures/commit.json new file mode 100644 index 0000000..08bec52 --- /dev/null +++ b/test/fixtures/commit.json @@ -0,0 +1,27 @@ +{ + "sha": "e7c077c610afa371430180fbd447bfef60ebc5ea", + "url": "https://api.github.com/repos/nodejs/node/git/commits/e7c077c610afa371430180fbd447bfef60ebc5ea", + "html_url": "https://github.com/nodejs/node/commit/e7c077c610afa371430180fbd447bfef60ebc5ea", + "author": { + "name": "Calvin Metcalf", + "email": "cmetcalf@appgeo.com", + "date": "2016-04-12T19:42:23Z" + }, + "committer": { + "name": "James M Snell", + "email": "jasnell@gmail.com", + "date": "2016-04-20T20:28:35Z" + }, + "tree": { + "sha": "d3f20ccfaa7b0919a7c5a472e344b7de8829b30c", + "url": "https://api.github.com/repos/nodejs/node/git/trees/d3f20ccfaa7b0919a7c5a472e344b7de8829b30c" + }, + "message": "stream: make null an invalid chunk to write in object mode\n\nthis harmonizes behavior between readable, writable, and transform\nstreams so that they all handle nulls in object mode the same way by\nconsidering them invalid chunks.\n\nPR-URL: https://github.com/nodejs/node/pull/6170\nReviewed-By: James M Snell \nReviewed-By: Matteo Collina ", + "parents": [ + { + "sha": "ec2822adaad76b126b5cccdeaa1addf2376c9aa6", + "url": "https://api.github.com/repos/nodejs/node/git/commits/ec2822adaad76b126b5cccdeaa1addf2376c9aa6", + "html_url": "https://github.com/nodejs/node/commit/ec2822adaad76b126b5cccdeaa1addf2376c9aa6" + } + ] +} diff --git a/test/fixtures/pr.json b/test/fixtures/pr.json new file mode 100644 index 0000000..3ad5078 --- /dev/null +++ b/test/fixtures/pr.json @@ -0,0 +1,72 @@ +[ + { + "sha": "e7c077c610afa371430180fbd447bfef60ebc5ea", + "commit": { + "author": { + "name": "Calvin Metcalf", + "email": "cmetcalf@appgeo.com", + "date": "2016-04-12T19:42:23Z" + }, + "committer": { + "name": "Calvin Metcalf", + "email": "cmetcalf@appgeo.com", + "date": "2016-04-13T16:33:55Z" + }, + "message": "stream: make null an invalid chunk to write in object mode\n\nthis harmonizes behavior between readable, writable, and transform\nstreams so that they all handle nulls in object mode the same way by\nconsidering them invalid chunks.\n\nPR-URL: https://github.com/nodejs/node/pull/6170\nReviewed-By: James M Snell \nReviewed-By: Matteo Collina ", + "tree": { + "sha": "e4f9381fdd77d1fd38fe27a80dc43486ac732d48", + "url": "https://api.github.com/repos/nodejs/node/git/trees/e4f9381fdd77d1fd38fe27a80dc43486ac732d48" + }, + "url": "https://api.github.com/repos/nodejs/node/git/commits/401ff75945d39b28b26c4e54863f312b19c0a2dd", + "comment_count": 0 + }, + "url": "https://api.github.com/repos/nodejs/node/commits/401ff75945d39b28b26c4e54863f312b19c0a2dd", + "html_url": "https://github.com/nodejs/node/commit/401ff75945d39b28b26c4e54863f312b19c0a2dd", + "comments_url": "https://api.github.com/repos/nodejs/node/commits/401ff75945d39b28b26c4e54863f312b19c0a2dd/comments", + "author": { + "login": "calvinmetcalf", + "id": 1128607, + "avatar_url": "https://avatars.githubusercontent.com/u/1128607?v=3", + "gravatar_id": "", + "url": "https://api.github.com/users/calvinmetcalf", + "html_url": "https://github.com/calvinmetcalf", + "followers_url": "https://api.github.com/users/calvinmetcalf/followers", + "following_url": "https://api.github.com/users/calvinmetcalf/following{/other_user}", + "gists_url": "https://api.github.com/users/calvinmetcalf/gists{/gist_id}", + "starred_url": "https://api.github.com/users/calvinmetcalf/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/calvinmetcalf/subscriptions", + "organizations_url": "https://api.github.com/users/calvinmetcalf/orgs", + "repos_url": "https://api.github.com/users/calvinmetcalf/repos", + "events_url": "https://api.github.com/users/calvinmetcalf/events{/privacy}", + "received_events_url": "https://api.github.com/users/calvinmetcalf/received_events", + "type": "User", + "site_admin": false + }, + "committer": { + "login": "calvinmetcalf", + "id": 1128607, + "avatar_url": "https://avatars.githubusercontent.com/u/1128607?v=3", + "gravatar_id": "", + "url": "https://api.github.com/users/calvinmetcalf", + "html_url": "https://github.com/calvinmetcalf", + "followers_url": "https://api.github.com/users/calvinmetcalf/followers", + "following_url": "https://api.github.com/users/calvinmetcalf/following{/other_user}", + "gists_url": "https://api.github.com/users/calvinmetcalf/gists{/gist_id}", + "starred_url": "https://api.github.com/users/calvinmetcalf/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/calvinmetcalf/subscriptions", + "organizations_url": "https://api.github.com/users/calvinmetcalf/orgs", + "repos_url": "https://api.github.com/users/calvinmetcalf/repos", + "events_url": "https://api.github.com/users/calvinmetcalf/events{/privacy}", + "received_events_url": "https://api.github.com/users/calvinmetcalf/received_events", + "type": "User", + "site_admin": false + }, + "parents": [ + { + "sha": "aba035fb27b14fe561c45540818be6a2bbb9dc9e", + "url": "https://api.github.com/repos/nodejs/node/commits/aba035fb27b14fe561c45540818be6a2bbb9dc9e", + "html_url": "https://github.com/nodejs/node/commit/aba035fb27b14fe561c45540818be6a2bbb9dc9e" + } + ] + } +] diff --git a/test/rules/fixes-url.js b/test/rules/fixes-url.js new file mode 100644 index 0000000..a67a1f6 --- /dev/null +++ b/test/rules/fixes-url.js @@ -0,0 +1,39 @@ +'use strict' + +const test = require('tap').test +const Rule = require('../../lib/rules/fixes-url') +const Commit = require('gitlint-parser-node') +const Validator = require('../../') +const INVALID_FIXES_URL = 'Fixes must be a url, not an issue number.' + +test('rule: fixes-url', (t) => { + t.test('invalid', (tt) => { + tt.plan(7) + const v = new Validator() + const context = new Commit({ + sha: 'e7c077c610afa371430180fbd447bfef60ebc5ea' + , author: { + name: 'Evan Lucas' + , email: 'evanlucas@me.com' + , date: '2016-04-12T19:42:23Z' + } + , message: `test: fix something + +Fixes: #1234` + }, v) + + context.report = (opts) => { + tt.pass('called report') + tt.equal(opts.id, 'fixes-url', 'id') + tt.equal(opts.message, INVALID_FIXES_URL, 'message') + tt.equal(opts.string, '#1234', 'string') + tt.equal(opts.line, 1, 'line') + tt.equal(opts.column, 7, 'column') + tt.equal(opts.level, 'error', 'level') + } + + Rule.validate(context) + }) + + t.end() +}) diff --git a/test/rules/line-length.js b/test/rules/line-length.js new file mode 100644 index 0000000..0651463 --- /dev/null +++ b/test/rules/line-length.js @@ -0,0 +1,68 @@ +'use strict' + +const test = require('tap').test +const Rule = require('../../lib/rules/line-length') +const Commit = require('gitlint-parser-node') +const Validator = require('../../') + +test('rule: line-length', (t) => { + t.test('line too long', (tt) => { + tt.plan(7) + const v = new Validator() + const context = new Commit({ + sha: 'e7c077c610afa371430180fbd447bfef60ebc5ea' + , author: { + name: 'Evan Lucas' + , email: 'evanlucas@me.com' + , date: '2016-04-12T19:42:23Z' + } + , message: `test: fix something + +${'aaa'.repeat(30)}` + }, v) + + context.report = (opts) => { + tt.pass('called report') + tt.equal(opts.id, 'line-length', 'id') + tt.equal(opts.message, 'Line should be <= 72 columns.', 'message') + tt.equal(opts.string, 'aaa'.repeat(30), 'string') + tt.equal(opts.line, 1, 'line') + tt.equal(opts.column, 72, 'column') + tt.equal(opts.level, 'error', 'level') + } + + Rule.validate(context, { + options: { + length: 72 + } + }) + }) + + t.test('release commit', (tt) => { + const v = new Validator() + const context = new Commit({ + sha: 'e7c077c610afa371430180fbd447bfef60ebc5ea' + , author: { + name: 'Evan Lucas' + , email: 'evanlucas@me.com' + , date: '2016-04-12T19:42:23Z' + } + , message: `2016-01-01, Version 1.0.0 + +${'aaa'.repeat(30)}` + }, v) + + context.report = (opts) => { + tt.fail('should not call report()') + } + + Rule.validate(context, { + options: { + length: 72 + } + }) + tt.end() + }) + + t.end() +}) diff --git a/test/rules/pr-url.js b/test/rules/pr-url.js new file mode 100644 index 0000000..5ffca2b --- /dev/null +++ b/test/rules/pr-url.js @@ -0,0 +1,50 @@ +'use strict' + +const test = require('tap').test +const Rule = require('../../lib/rules/pr-url') +const MISSING_PR_URL = 'Commit must have a PR-URL.' +const INVALID_PR_URL = 'PR-URL must be a url, not a pr number.' + +test('rule: pr-url', (t) => { + t.test('missing', (tt) => { + tt.plan(7) + const context = { + prUrl: null + , report: (opts) => { + tt.pass('called report') + tt.equal(opts.id, 'pr-url', 'id') + tt.equal(opts.message, MISSING_PR_URL, 'message') + tt.equal(opts.string, null, 'string') + tt.equal(opts.line, 0, 'line') + tt.equal(opts.column, 0, 'column') + tt.equal(opts.level, 'error', 'level') + } + } + + Rule.validate(context) + }) + + t.test('invalid', (tt) => { + tt.plan(7) + const context = { + prUrl: '#1234' + , body: [ + '' + , 'PR-URL: #1234' + ] + , report: (opts) => { + tt.pass('called report') + tt.equal(opts.id, 'pr-url', 'id') + tt.equal(opts.message, INVALID_PR_URL, 'message') + tt.equal(opts.string, '#1234', 'string') + tt.equal(opts.line, 1, 'line') + tt.equal(opts.column, 8, 'column') + tt.equal(opts.level, 'error', 'level') + } + } + + Rule.validate(context) + }) + + t.end() +}) diff --git a/test/rules/reviewers.js b/test/rules/reviewers.js new file mode 100644 index 0000000..29695c3 --- /dev/null +++ b/test/rules/reviewers.js @@ -0,0 +1,39 @@ +'use strict' + +const test = require('tap').test +const Rule = require('../../lib/rules/reviewers') +const Commit = require('gitlint-parser-node') +const Validator = require('../../') +const MSG = 'Commit must have at least 1 reviewer.' + +test('rule: reviewers', (t) => { + t.test('missing', (tt) => { + tt.plan(7) + const v = new Validator() + const context = new Commit({ + sha: 'e7c077c610afa371430180fbd447bfef60ebc5ea' + , author: { + name: 'Evan Lucas' + , email: 'evanlucas@me.com' + , date: '2016-04-12T19:42:23Z' + } + , message: `test: fix something + +This is a test` + }, v) + + context.report = (opts) => { + tt.pass('called report') + tt.equal(opts.id, 'reviewers', 'id') + tt.equal(opts.message, MSG, 'message') + tt.equal(opts.string, null, 'string') + tt.equal(opts.line, 0, 'line') + tt.equal(opts.column, 0, 'column') + tt.equal(opts.level, 'error', 'level') + } + + Rule.validate(context) + }) + + t.end() +}) diff --git a/test/validator.js b/test/validator.js index 67552b2..8ff35a8 100644 --- a/test/validator.js +++ b/test/validator.js @@ -3,6 +3,7 @@ const test = require('tap').test const Validator = require('../') +// Note, these are not necessarily all real commit messages const str = `commit e7c077c610afa371430180fbd447bfef60ebc5ea Author: Calvin Metcalf AuthorDate: Tue Apr 12 15:42:23 2016 -0400 @@ -55,21 +56,60 @@ Date: Fri Apr 15 13:32:36 2016 +0200 Reviewed-By: Brian White ` /* eslint-enable */ -test('Validator', (t) => { +const str4 = `commit 7d3a7ea0d7df9b6f11df723dec370f49f4f87e99 +Author: Wyatt Preul +Date: Thu Mar 3 10:10:46 2016 -0600 + + check memoryUsage properties + The properties on memoryUsage were not checked before, + this commit checks them. + + PR-URL: #5546 + Reviewed-By: Colin Ihrig ` + +const str5 = `commit 7d3a7ea0d7df9b6f11df723dec370f49f4f87e99 +Author: Wyatt Preul +Date: Thu Mar 3 10:10:46 2016 -0600 + + test: check memoryUsage properties + + The properties on memoryUsage were not checked before, + this commit checks them.` + + +test('Validator - misc', (t) => { + const v = new Validator() + + t.throws(() => { + v.disableRule('biscuits') + }, /Invalid rule: "biscuits"/) + + v.disableRule('line-length') + t.equal(v.rules.get('line-length').disabled, true, 'disabled') + v.rules.get('line-length').disabled = false + + t.end() +}) + +test('Validator - real commits', (t) => { t.test('basic', (tt) => { + tt.plan(18) const v = new Validator() + // run against the output of git show --quiet + // run against the output of github's get commit api request + // run against the output of github's list commits for pr api request v.lint(str) + v.lint(require('./fixtures/commit')) + v.lint(require('./fixtures/pr')) v.on('commit', (data) => { const c = data.commit tt.equal(c.sha, 'e7c077c610afa371430180fbd447bfef60ebc5ea', 'sha') - tt.equal(c.date, 'Tue Apr 12 15:42:23 2016 -0400', 'date') tt.deepEqual(c.subsystems, ['stream'], 'subsystems') tt.equal(c.prUrl, 'https://github.com/nodejs/node/pull/6170', 'pr') const msgs = data.messages - tt.equal(msgs.length, 1, 'messages.length') + tt.equal(msgs.length, 3, 'messages.length') tt.equal(msgs[0].level, 'error') tt.equal(msgs[0].id, 'title-length') - tt.end() }) }) @@ -112,5 +152,51 @@ test('Validator', (t) => { }) }) + t.test('invalid pr-url, missing subsystem', (tt) => { + const v = new Validator() + v.lint(str4) + v.on('commit', (data) => { + const c = data.commit.toJSON() + tt.equal(c.sha, '7d3a7ea0d7df9b6f11df723dec370f49f4f87e99', 'sha') + tt.equal(c.date, 'Thu Mar 3 10:10:46 2016 -0600', 'date') + tt.deepEqual(c.subsystems, [], 'subsystems') + tt.equal(c.prUrl, '#5546', 'pr') + tt.equal(c.revert, false, 'revert') + const msgs = data.messages + msgs.sort((a, b) => { + return a.id < b.id + ? -1 + : a.id > b.id + ? 1 + : 0 + }) + tt.equal(msgs.length, 2, 'messages.length') + tt.equal(msgs[0].id, 'pr-url', 'message id') + tt.equal(msgs[0].string, '#5546', 'message string') + tt.equal(msgs[1].id, 'subsystem', 'message id') + tt.equal(msgs[1].line, 0, 'line') + tt.equal(msgs[1].column, 0, 'column') + tt.end() + }) + }) + + t.test('invalid pr-url, missing subsystem no meta', (tt) => { + const v = new Validator({ + 'validate-metadata': false + }) + v.lint(str5) + v.on('commit', (data) => { + const c = data.commit.toJSON() + tt.equal(c.sha, '7d3a7ea0d7df9b6f11df723dec370f49f4f87e99', 'sha') + tt.equal(c.date, 'Thu Mar 3 10:10:46 2016 -0600', 'date') + tt.deepEqual(c.subsystems, ['test'], 'subsystems') + tt.equal(c.prUrl, null, 'pr') + tt.equal(c.revert, false, 'revert') + const msgs = data.messages + tt.equal(msgs.length, 0, 'messages.length') + tt.end() + }) + }) + t.end() }) From e703622e757e02bbf5e09f443c4ee2b55dd22970 Mon Sep 17 00:00:00 2001 From: Evan Lucas Date: Thu, 21 Jul 2016 08:16:33 -0500 Subject: [PATCH 11/11] ci: only support v6 --- .travis.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 0dc49c0..db8b926 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,5 +1,4 @@ language: node_js node_js: - - "4" - - "5" + - "6" sudo: false