From 8760413fd218fa865070375f0a5ac3b8895a9cf8 Mon Sep 17 00:00:00 2001 From: Emelia Smith Date: Wed, 29 Jul 2020 03:39:03 +0200 Subject: [PATCH 1/5] refactor: lift inquirer up to the initialization of engine from the prompter This allows us to use methods from inquirer, such as the Separator class, before actually calling prompter() --- engine.js | 16 ++++++------- engine.test.js | 64 ++++++++++++++++++++++++++++++++------------------ index.js | 6 ++++- 3 files changed, 53 insertions(+), 33 deletions(-) diff --git a/engine.js b/engine.js index 75dac053..510ba154 100644 --- a/engine.js +++ b/engine.js @@ -36,7 +36,7 @@ var filterSubject = function(subject) { // This can be any kind of SystemJS compatible module. // We use Commonjs here, but ES6 or AMD would do just // fine. -module.exports = function(options) { +module.exports = function(options, inquirer) { var types = options.types; var length = longest(Object.keys(types)).length + 1; @@ -49,9 +49,7 @@ module.exports = function(options) { return { // When a user runs `git cz`, prompter will - // be executed. We pass you cz, which currently - // is just an instance of inquirer.js. Using - // this you can ask questions and get answers. + // be executed. // // The commit callback should be executed when // you're ready to send back a commit template @@ -59,14 +57,14 @@ module.exports = function(options) { // // By default, we'll de-indent your commit // template and will keep empty lines. - prompter: function(cz, commit) { + prompter: function(commit) { // Let's ask some questions of the user // so that we can populate our commit // template. - // - // See inquirer.js docs for specifics. - // You can also opt to use another input - // collection library if you prefer. + + // FIXME: Avoid changing the entire file because of the longer variable of inquirer vs cz + var cz = inquirer; + cz.prompt([ { type: 'list', diff --git a/engine.test.js b/engine.test.js index 4a42becd..8fac8f8e 100644 --- a/engine.test.js +++ b/engine.test.js @@ -387,6 +387,7 @@ describe('commitlint config header-max-length', function() { var options = undefined; mock('./engine', function(opts) { options = opts; + return { prompter: function() {} }; }); if (headerMaxLength) { mock('cosmiconfig', function() { @@ -406,6 +407,9 @@ describe('commitlint config header-max-length', function() { } mock.reRequire('./index'); + + require('./index').prompter(); + try { return mock .reRequire('@commitlint/load')() @@ -460,28 +464,43 @@ describe('commitlint config header-max-length', function() { }); } }); + +// processor accepts questions and returns an array of answers: +function createMockInquirer(processor) { + function Separator(line) { + if (!(this instanceof Separator)) { + throw new Error('inquirer.Separator must be invoked with `new` keyword'); + } + return { type: 'separator', line: line }; + } + return { + Separator: Separator, + prompt: function(questions) { + return { + then: function(finalizer) { + finalizer(processor(questions)); + } + }; + } + }; +} + function commitMessage(answers, options) { options = options || defaultOptions; + var result = null; - engine(options).prompter( - { - prompt: function(questions) { - return { - then: function(finalizer) { - processQuestions(questions, answers, options); - finalizer(answers); - } - }; - } - }, - function(message) { - result = message; - } - ); + var mockInquirer = createMockInquirer(questions => { + processQuestions(questions, answers); + return answers; + }); + + engine(options, mockInquirer).prompter(function(message) { + result = message; + }); return result; } -function processQuestions(questions, answers, options) { +function processQuestions(questions, answers) { for (var i in questions) { var question = questions[i]; var answer = answers[question.name]; @@ -504,14 +523,13 @@ function processQuestions(questions, answers, options) { function getQuestions(options) { options = options || defaultOptions; var result = null; - engine(options).prompter({ - prompt: function(questions) { - result = questions; - return { - then: function() {} - }; - } + + var mockInquirer = createMockInquirer(questions => { + result = questions; + return []; }); + + engine(options, mockInquirer).prompter(message => {}); return result; } diff --git a/index.js b/index.js index d86155bc..878f7da6 100644 --- a/index.js +++ b/index.js @@ -45,4 +45,8 @@ var options = { } catch (err) {} })(options); -module.exports = engine(options); +module.exports = { + prompter: function czConventionalChangelogAdapter(inquirer, commit) { + engine(options, inquirer).prompter(commit); + } +}; From 9997ea9c36f9d5380df751ab3157a7b1b9ab5c7f Mon Sep 17 00:00:00 2001 From: Emelia Smith Date: Wed, 29 Jul 2020 02:49:01 +0200 Subject: [PATCH 2/5] refactor: improve loading of configuration Previously configuration was tested by mocking our own code, as well as code that we don't own (other modules), this was because configuration took place at require-time of the cz-conventional-commit module, rather than at execution-time. By moving configuration evaluation to execution-time we can better test the logic involved in loading and evaluating configuration. The removal of the dependency on cosmiconfig comes because we no longer need to mock this module which is a 2nd degree dependency: it's used by commitlint, which we can just mock directly. The previous mocking was needed to override the max-header-length option in a really round-about manner where all tests effectively depended on commitlint, even though that wasn't what the functionality actually was. This changeset all means that we can test loading and evaluating configuration in isolation, without involving the engine. --- engine.test.js | 87 ------------------------ index.js | 84 +++++++++++++++-------- index.test.js | 165 ++++++++++++++++++++++++++++++++++++++++++++++ package-lock.json | 157 ++++++++++++++++--------------------------- package.json | 6 +- 5 files changed, 277 insertions(+), 222 deletions(-) create mode 100644 index.test.js diff --git a/engine.test.js b/engine.test.js index 8fac8f8e..719e3c4a 100644 --- a/engine.test.js +++ b/engine.test.js @@ -1,8 +1,6 @@ var chai = require('chai'); var chalk = require('chalk'); var engine = require('./engine'); -var mock = require('mock-require'); -var semver = require('semver'); var types = require('conventional-commit-types').types; @@ -380,91 +378,6 @@ describe('when', function() { ).to.be.true); }); -describe('commitlint config header-max-length', function() { - //commitlint config parser only supports Node 6.0.0 and higher - if (semver.gte(process.version, '6.0.0')) { - function mockOptions(headerMaxLength) { - var options = undefined; - mock('./engine', function(opts) { - options = opts; - return { prompter: function() {} }; - }); - if (headerMaxLength) { - mock('cosmiconfig', function() { - return { - load: function(cwd) { - return { - filepath: cwd + '/.commitlintrc.js', - config: { - rules: { - 'header-max-length': [2, 'always', headerMaxLength] - } - } - }; - } - }; - }); - } - - mock.reRequire('./index'); - - require('./index').prompter(); - - try { - return mock - .reRequire('@commitlint/load')() - .then(function() { - return options; - }); - } catch (err) { - return Promise.resolve(options); - } - } - - afterEach(function() { - delete require.cache[require.resolve('./index')]; - delete require.cache[require.resolve('@commitlint/load')]; - delete process.env.CZ_MAX_HEADER_WIDTH; - mock.stopAll(); - }); - - it('with no environment or commitizen config override', function() { - return mockOptions(72).then(function(options) { - expect(options).to.have.property('maxHeaderWidth', 72); - }); - }); - - it('with environment variable override', function() { - process.env.CZ_MAX_HEADER_WIDTH = '105'; - return mockOptions(72).then(function(options) { - expect(options).to.have.property('maxHeaderWidth', 105); - }); - }); - - it('with commitizen config override', function() { - mock('commitizen', { - configLoader: { - load: function() { - return { - maxHeaderWidth: 103 - }; - } - } - }); - return mockOptions(72).then(function(options) { - expect(options).to.have.property('maxHeaderWidth', 103); - }); - }); - } else { - //Node 4 doesn't support commitlint so the config value should remain the same - it('default value for Node 4', function() { - return mockOptions(72).then(function(options) { - expect(options).to.have.property('maxHeaderWidth', 100); - }); - }); - } -}); - // processor accepts questions and returns an array of answers: function createMockInquirer(processor) { function Separator(line) { diff --git a/index.js b/index.js index 878f7da6..e8fbc847 100644 --- a/index.js +++ b/index.js @@ -4,49 +4,75 @@ var engine = require('./engine'); var conventionalCommitTypes = require('conventional-commit-types'); var configLoader = require('commitizen').configLoader; -var config = configLoader.load() || {}; -var options = { - types: config.types || conventionalCommitTypes.types, - defaultType: process.env.CZ_TYPE || config.defaultType, - defaultScope: process.env.CZ_SCOPE || config.defaultScope, - defaultSubject: process.env.CZ_SUBJECT || config.defaultSubject, - defaultBody: process.env.CZ_BODY || config.defaultBody, - defaultIssues: process.env.CZ_ISSUES || config.defaultIssues, - disableScopeLowerCase: - process.env.DISABLE_SCOPE_LOWERCASE || config.disableScopeLowerCase, - maxHeaderWidth: - (process.env.CZ_MAX_HEADER_WIDTH && - parseInt(process.env.CZ_MAX_HEADER_WIDTH)) || - config.maxHeaderWidth || - 100, - maxLineWidth: - (process.env.CZ_MAX_LINE_WIDTH && - parseInt(process.env.CZ_MAX_LINE_WIDTH)) || - config.maxLineWidth || - 100 -}; +function isValidCommitlintRule(rule) { + return Array.isArray(rule) && rule.length >= 3; +} + +function loadOptions(params) { + params = params || {}; + + var env = params.env || {}; + var config = params.config || {}; + + // maxHeaderWidth can come from environment, commitizen config, commitlint config, or default; + // If the environment variable isn't an integer, then we fall through to + // commitizen config, then to the default. Commitlint config is detected later. + var CZ_MAX_HEADER_WIDTH = + env.CZ_MAX_HEADER_WIDTH && parseInt(env.CZ_MAX_HEADER_WIDTH, 10); + var maxHeaderWidth = CZ_MAX_HEADER_WIDTH || config.maxHeaderWidth || 100; + + // maxLineWidth can come from environment, commitizen config or default: + // If the environment variable isn't an integer, then we fall through to + // commitizen config, then to the default: + var CZ_MAX_LINE_WIDTH = + env.CZ_MAX_LINE_WIDTH && parseInt(env.CZ_MAX_LINE_WIDTH, 10); + var maxLineWidth = CZ_MAX_LINE_WIDTH || config.maxLineWidth || 100; + + var options = { + types: config.types || conventionalCommitTypes.types, + defaultType: env.CZ_TYPE || config.defaultType, + defaultScope: env.CZ_SCOPE || config.defaultScope, + defaultSubject: env.CZ_SUBJECT || config.defaultSubject, + defaultBody: env.CZ_BODY || config.defaultBody, + defaultIssues: env.CZ_ISSUES || config.defaultIssues, + disableScopeLowerCase: + env.DISABLE_SCOPE_LOWERCASE || config.disableScopeLowerCase, + maxHeaderWidth: maxHeaderWidth, + maxLineWidth: maxLineWidth + }; -(function(options) { try { var commitlintLoad = require('@commitlint/load'); - commitlintLoad().then(function(clConfig) { + return commitlintLoad().then(function(clConfig) { if (clConfig.rules) { var maxHeaderLengthRule = clConfig.rules['header-max-length']; if ( - typeof maxHeaderLengthRule === 'object' && - maxHeaderLengthRule.length >= 3 && - !process.env.CZ_MAX_HEADER_WIDTH && + isValidCommitlintRule(maxHeaderLengthRule) && + !env.CZ_MAX_HEADER_WIDTH && !config.maxHeaderWidth ) { options.maxHeaderWidth = maxHeaderLengthRule[2]; } + + return options; } }); - } catch (err) {} -})(options); + } catch (err) { + return Promise.resolve(options); + } +} module.exports = { + internals: { + loadOptions: loadOptions + }, + prompter: function czConventionalChangelogAdapter(inquirer, commit) { - engine(options, inquirer).prompter(commit); + loadOptions({ + env: process.env, + config: configLoader.load() + }).then(options => { + engine(options, inquirer).prompter(commit); + }); } }; diff --git a/index.test.js b/index.test.js new file mode 100644 index 00000000..a4a5f485 --- /dev/null +++ b/index.test.js @@ -0,0 +1,165 @@ +var chai = require('chai'); +var proxyquire = require('proxyquire'); + +var conventionalCommitTypes = require('conventional-commit-types'); + +var expect = chai.expect; +chai.should(); + +var defaultConfig = { + types: conventionalCommitTypes.types, + defaultType: undefined, + defaultScope: undefined, + defaultSubject: undefined, + defaultBody: undefined, + defaultIssues: undefined, + disableScopeLowerCase: undefined, + maxHeaderWidth: 100, + maxLineWidth: 100 +}; + +var invalidCommitlintConfig = { + rules: { + // [ level, applicability, ...options ] + 'header-max-length': [] + } +}; + +var validCommitlintConfig = { + rules: { + // [ level, applicability, ...options ] + 'header-max-length': [0, 'always', 123] + } +}; + +var loadOptions; + +function setupCommitlintStub(stub) { + var entrypoint = proxyquire('./index.js', { + '@commitlint/load': stub + }); + + loadOptions = entrypoint.internals.loadOptions; +} + +describe('loading of options', function() { + describe('when commitlint is not installed', function() { + before(function() { + setupCommitlintStub(null); + }); + + it('should correctly use the defaults when not passed any arguments', async function() { + const options = await loadOptions(); + expect(options).to.deep.equal(defaultConfig); + }); + + it('should correctly use the defaults when not passed an environment', async function() { + const options = await loadOptions({}); + expect(options).to.deep.equal(defaultConfig); + }); + + describe('maxHeaderWidth option', function() { + it('should use the CZ_MAX_HEADER_WIDTH environment variable if parseable as an integer', async function() { + const options = await loadOptions({ + env: { + CZ_MAX_HEADER_WIDTH: '60' + } + }); + expect(options).to.have.property('maxHeaderWidth', 60); + }); + + it('should not use the CZ_MAX_HEADER_WIDTH environment variable if not parseable as an integer', async function() { + const options = await loadOptions({ + env: { + CZ_MAX_HEADER_WIDTH: 'not-a-number' + } + }); + + expect(options).to.have.property('maxHeaderWidth', 100); + }); + }); + + describe('maxLineWidth option', function() { + it('should use the CZ_MAX_LINE_WIDTH environment variable if parseable as an integer', async function() { + const options = await loadOptions({ + env: { + CZ_MAX_LINE_WIDTH: '60' + } + }); + expect(options).to.have.property('maxLineWidth', 60); + }); + + it('should not use the CZ_MAX_LINE_WIDTH environment variable if not parseable as an integer', async function() { + const options = await loadOptions({ + env: { + CZ_MAX_LINE_WIDTH: 'not-a-number' + } + }); + + expect(options).to.have.property('maxLineWidth', 100); + }); + }); + }); + + describe('when commitlint is installed', function() { + before(function() { + setupCommitlintStub(function() { + return Promise.resolve(validCommitlintConfig); + }); + }); + + it('should override max-header-width if not otherwise set', async function() { + const options = await loadOptions(); + expect(options).to.deep.equal({ ...defaultConfig, maxHeaderWidth: 123 }); + }); + + it('should not override max-header-width if set by environment', async function() { + const options = await loadOptions({ + env: { + CZ_MAX_HEADER_WIDTH: 60 + } + }); + expect(options).to.deep.equal({ ...defaultConfig, maxHeaderWidth: 60 }); + }); + + it('should not override max-header-width if set by commitizen configuration', async function() { + // by passing config, we're setting commitizen config + const options = await loadOptions({ + config: { + maxHeaderWidth: 70 + } + }); + expect(options).to.deep.equal({ ...defaultConfig, maxHeaderWidth: 70 }); + }); + + describe('and the commitlint configuration is invalid', function() { + before(function() { + setupCommitlintStub(function() { + return Promise.resolve(invalidCommitlintConfig); + }); + }); + + it('should not override max-header-width', async function() { + const options = await loadOptions({}); + expect(options).to.deep.equal({ + ...defaultConfig, + maxHeaderWidth: 100 + }); + }); + }); + }); +}); + +describe('initialization of prompter', function() { + // The loadOptions tests above assume that commitizen configuration is passed in, + // in order to test the loading of commitizen configuration, we would need to mock + // cosmicconfig and the filesystem. + // + // We should be able to assume that `require('commitizen').configLoader.load()` + // works as expected, though there was previously a test that mocked the + // commitizen module in order to provide testing, we may not need that due to + // the clearer separation of concerns now present. + // + // For details of the prior test, see: https://github.com/commitizen/cz-conventional-changelog/blob/e7bd5462966d00acb03aca394836b5427513681c/engine.test.js#L391-L406 + it('should correctly load configuration from commitizen'); +}); diff --git a/package-lock.json b/package-lock.json index 8173160f..2cf35d28 100644 --- a/package-lock.json +++ b/package-lock.json @@ -268,30 +268,6 @@ "resolved": "https://registry.npmjs.org/cachedir/-/cachedir-2.2.0.tgz", "integrity": "sha512-VvxA0xhNqIIfg0V9AmJkDg91DaJwryutH5rVEZAhcNi4iJFj9f+QxmAjgK1LT9I8OgToX27fypX6/MeCXVbBjQ==" }, - "caller-callsite": { - "version": "2.0.0", - "resolved": "https://registry.npmjs.org/caller-callsite/-/caller-callsite-2.0.0.tgz", - "integrity": "sha1-hH4PzgoiN1CpoCfFSzNzGtMVQTQ=", - "dev": true, - "requires": { - "callsites": "^2.0.0" - } - }, - "caller-path": { - "version": "2.0.0", - "resolved": "https://registry.npmjs.org/caller-path/-/caller-path-2.0.0.tgz", - "integrity": "sha1-Ro+DBE42mrIBD6xfBs7uFbsssfQ=", - "dev": true, - "requires": { - "caller-callsite": "^2.0.0" - } - }, - "callsites": { - "version": "2.0.0", - "resolved": "https://registry.npmjs.org/callsites/-/callsites-2.0.0.tgz", - "integrity": "sha1-BuuE8A7qQT2oav/vrL/7Ngk7PFA=", - "dev": true - }, "camelcase": { "version": "5.3.1", "resolved": "https://registry.npmjs.org/camelcase/-/camelcase-5.3.1.tgz", @@ -511,18 +487,6 @@ "integrity": "sha512-l00tmFFZOBHtYhN4Cz7k32VM7vTn3rE2ANjQDxdEN6zmXZ/xq1jQuutnmHvMG1ZJ7xd72+TA5YpUK8wz3rWsfQ==", "optional": true }, - "cosmiconfig": { - "version": "5.2.1", - "resolved": "https://registry.npmjs.org/cosmiconfig/-/cosmiconfig-5.2.1.tgz", - "integrity": "sha512-H65gsXo1SKjf8zmrJ67eJk8aIRKV5ff2D4uKZIBZShbhGSpEmsQOPW/SKMKYhSTrqR7ufy6RP69rPogdaPh/kA==", - "dev": true, - "requires": { - "import-fresh": "^2.0.0", - "is-directory": "^0.3.1", - "js-yaml": "^3.13.1", - "parse-json": "^4.0.0" - } - }, "cross-spawn": { "version": "6.0.5", "resolved": "https://registry.npmjs.org/cross-spawn/-/cross-spawn-6.0.5.tgz", @@ -565,23 +529,6 @@ } } }, - "debug": { - "version": "3.2.6", - "resolved": "https://registry.npmjs.org/debug/-/debug-3.2.6.tgz", - "integrity": "sha512-mel+jf7nrtEl5Pn1Qx46zARXKDpBbvzezse7p7LqINmdoIk8PYP5SySaxEmYv6TZ0JyEKA1hsCId6DIhgITtWQ==", - "dev": true, - "requires": { - "ms": "^2.1.1" - }, - "dependencies": { - "ms": { - "version": "2.1.2", - "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.2.tgz", - "integrity": "sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w==", - "dev": true - } - } - }, "decamelize": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/decamelize/-/decamelize-1.2.0.tgz", @@ -688,6 +635,7 @@ "version": "1.3.2", "resolved": "https://registry.npmjs.org/error-ex/-/error-ex-1.3.2.tgz", "integrity": "sha512-7dFHNmqeFSEt2ZBsCriorKnn3Z2pj+fd9kmI6QoWw4//DL+icEBfc0U7qJCisqrTsKTjw4fNFy2pW9OqStD84g==", + "optional": true, "requires": { "is-arrayish": "^0.2.1" } @@ -859,6 +807,16 @@ "escape-string-regexp": "^1.0.5" } }, + "fill-keys": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/fill-keys/-/fill-keys-1.0.2.tgz", + "integrity": "sha1-mo+jb06K1jTjv2tPPIiCVRRS6yA=", + "dev": true, + "requires": { + "is-object": "~1.0.1", + "merge-descriptors": "~1.0.0" + } + }, "fill-range": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/fill-range/-/fill-range-4.0.0.tgz", @@ -1120,16 +1078,6 @@ "safer-buffer": ">= 2.1.2 < 3" } }, - "import-fresh": { - "version": "2.0.0", - "resolved": "https://registry.npmjs.org/import-fresh/-/import-fresh-2.0.0.tgz", - "integrity": "sha1-2BNVwVYS04bGH53dOSLUMEgipUY=", - "dev": true, - "requires": { - "caller-path": "^2.0.0", - "resolve-from": "^3.0.0" - } - }, "inflight": { "version": "1.0.6", "resolved": "https://registry.npmjs.org/inflight/-/inflight-1.0.6.tgz", @@ -1208,7 +1156,8 @@ "is-arrayish": { "version": "0.2.1", "resolved": "https://registry.npmjs.org/is-arrayish/-/is-arrayish-0.2.1.tgz", - "integrity": "sha1-d8mYQFJ6qOyxqLppe4BkWnqSap0=" + "integrity": "sha1-d8mYQFJ6qOyxqLppe4BkWnqSap0=", + "optional": true }, "is-buffer": { "version": "1.1.6", @@ -1265,7 +1214,8 @@ "is-directory": { "version": "0.3.1", "resolved": "https://registry.npmjs.org/is-directory/-/is-directory-0.3.1.tgz", - "integrity": "sha1-YTObbyR1/Hcv2cnYP1yFddwVSuE=" + "integrity": "sha1-YTObbyR1/Hcv2cnYP1yFddwVSuE=", + "optional": true }, "is-extendable": { "version": "0.1.1", @@ -1308,6 +1258,12 @@ } } }, + "is-object": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/is-object/-/is-object-1.0.1.tgz", + "integrity": "sha1-iVJojF7C/9awPsyF52ngKQMINHA=", + "dev": true + }, "is-plain-object": { "version": "2.0.4", "resolved": "https://registry.npmjs.org/is-plain-object/-/is-plain-object-2.0.4.tgz", @@ -1382,7 +1338,8 @@ "json-parse-better-errors": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/json-parse-better-errors/-/json-parse-better-errors-1.0.2.tgz", - "integrity": "sha512-mrqyZKfX5EhL7hvqcV6WG1yYjnjeuYDzDhhcAAUrq8Po85NBQBJP+ZDUT75qZQ98IkUoBqdkExkukOU7Ts2wrw==" + "integrity": "sha512-mrqyZKfX5EhL7hvqcV6WG1yYjnjeuYDzDhhcAAUrq8Po85NBQBJP+ZDUT75qZQ98IkUoBqdkExkukOU7Ts2wrw==", + "optional": true }, "jsonfile": { "version": "4.0.0", @@ -1492,6 +1449,12 @@ "resolved": "https://registry.npmjs.org/merge/-/merge-1.2.1.tgz", "integrity": "sha512-VjFo4P5Whtj4vsLzsYBu5ayHhoHJ0UqNm7ibvShmbmoz7tGi0vXaoJbGdB+GmDMLUdg8DpQXEIeVDAe8MaABvQ==" }, + "merge-descriptors": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/merge-descriptors/-/merge-descriptors-1.0.1.tgz", + "integrity": "sha1-sAqqVW3YtEVoFQ7J0blT8/kMu2E=", + "dev": true + }, "micromatch": { "version": "3.1.10", "resolved": "https://registry.npmjs.org/micromatch/-/micromatch-3.1.10.tgz", @@ -1609,6 +1572,15 @@ "yargs-unparser": "1.5.0" }, "dependencies": { + "debug": { + "version": "3.2.6", + "resolved": "https://registry.npmjs.org/debug/-/debug-3.2.6.tgz", + "integrity": "sha512-mel+jf7nrtEl5Pn1Qx46zARXKDpBbvzezse7p7LqINmdoIk8PYP5SySaxEmYv6TZ0JyEKA1hsCId6DIhgITtWQ==", + "dev": true, + "requires": { + "ms": "^2.1.1" + } + }, "glob": { "version": "7.1.3", "resolved": "https://registry.npmjs.org/glob/-/glob-7.1.3.tgz", @@ -1637,15 +1609,11 @@ } } }, - "mock-require": { - "version": "3.0.3", - "resolved": "https://registry.npmjs.org/mock-require/-/mock-require-3.0.3.tgz", - "integrity": "sha512-lLzfLHcyc10MKQnNUCv7dMcoY/2Qxd6wJfbqCcVk3LDb8An4hF6ohk5AztrvgKhJCqj36uyzi/p5se+tvyD+Wg==", - "dev": true, - "requires": { - "get-caller-file": "^1.0.2", - "normalize-path": "^2.1.1" - } + "module-not-found-error": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/module-not-found-error/-/module-not-found-error-1.0.1.tgz", + "integrity": "sha1-z4tP9PKWQGdNbN0CsOO8UjwrvcA=", + "dev": true }, "ms": { "version": "2.0.0", @@ -1724,15 +1692,6 @@ } } }, - "normalize-path": { - "version": "2.1.1", - "resolved": "https://registry.npmjs.org/normalize-path/-/normalize-path-2.1.1.tgz", - "integrity": "sha1-GrKLVW4Zg2Oowab35vogE3/mrtk=", - "dev": true, - "requires": { - "remove-trailing-separator": "^1.0.1" - } - }, "npm-run-path": { "version": "2.0.2", "resolved": "https://registry.npmjs.org/npm-run-path/-/npm-run-path-2.0.2.tgz", @@ -1925,6 +1884,7 @@ "version": "4.0.0", "resolved": "https://registry.npmjs.org/parse-json/-/parse-json-4.0.0.tgz", "integrity": "sha1-vjX1Qlvh9/bHRxhPmKeIy5lHfuA=", + "optional": true, "requires": { "error-ex": "^1.3.1", "json-parse-better-errors": "^1.0.1" @@ -1979,6 +1939,17 @@ "integrity": "sha1-H+qsW90YEje1Tb5l2HTgKhRyeGo=", "dev": true }, + "proxyquire": { + "version": "2.1.3", + "resolved": "https://registry.npmjs.org/proxyquire/-/proxyquire-2.1.3.tgz", + "integrity": "sha512-BQWfCqYM+QINd+yawJz23tbBM40VIGXOdDw3X344KcclI/gtBbdWF6SlQ4nK/bYhF9d27KYug9WzljHC6B9Ysg==", + "dev": true, + "requires": { + "fill-keys": "^1.0.2", + "module-not-found-error": "^1.0.1", + "resolve": "^1.11.1" + } + }, "pump": { "version": "3.0.0", "resolved": "https://registry.npmjs.org/pump/-/pump-3.0.0.tgz", @@ -2031,12 +2002,6 @@ } } }, - "remove-trailing-separator": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/remove-trailing-separator/-/remove-trailing-separator-1.1.0.tgz", - "integrity": "sha1-wkvOKig62tW8P1jg1IJJuSN52O8=", - "dev": true - }, "repeat-element": { "version": "1.1.3", "resolved": "https://registry.npmjs.org/repeat-element/-/repeat-element-1.1.3.tgz", @@ -2082,12 +2047,6 @@ "global-modules": "^1.0.0" } }, - "resolve-from": { - "version": "3.0.0", - "resolved": "https://registry.npmjs.org/resolve-from/-/resolve-from-3.0.0.tgz", - "integrity": "sha1-six699nWiBvItuZTM17rywoYh0g=", - "dev": true - }, "resolve-global": { "version": "0.1.0", "resolved": "https://registry.npmjs.org/resolve-global/-/resolve-global-0.1.0.tgz", @@ -11643,12 +11602,6 @@ } } }, - "semver": { - "version": "6.2.0", - "resolved": "https://registry.npmjs.org/semver/-/semver-6.2.0.tgz", - "integrity": "sha512-jdFC1VdUGT/2Scgbimf7FSx9iJLXoqfglSF+gJeuNWVpiE37OIbc1jywR/GJyFdz3mnkz2/id0L0J/cr0izR5A==", - "dev": true - }, "set-blocking": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/set-blocking/-/set-blocking-2.0.0.tgz", diff --git a/package.json b/package.json index 87ffbd5e..54f9b041 100644 --- a/package.json +++ b/package.json @@ -32,12 +32,10 @@ "@types/chai": "^4.1.7", "@types/mocha": "^5.2.7", "chai": "^4.2.0", - "cosmiconfig": "^5.2.1", "mocha": "^6.2.0", - "mock-require": "^3.0.3", "prettier": "^1.15.3", - "semantic-release": "^15.13.3", - "semver": "^6.2.0" + "proxyquire": "^2.1.3", + "semantic-release": "^15.13.3" }, "optionalDependencies": { "@commitlint/load": ">6.1.1" From 19d41d77551b7d8abfeac5e3a6c33e025b156626 Mon Sep 17 00:00:00 2001 From: Emelia Smith Date: Wed, 29 Jul 2020 03:13:30 +0200 Subject: [PATCH 3/5] feat: improve debuggability of tests --- engine.test.js | 26 ++++++++++++++++++++++---- package-lock.json | 17 +++++++++++++++++ package.json | 1 + 3 files changed, 40 insertions(+), 4 deletions(-) diff --git a/engine.test.js b/engine.test.js index 719e3c4a..3a6783a6 100644 --- a/engine.test.js +++ b/engine.test.js @@ -2,6 +2,8 @@ var chai = require('chai'); var chalk = require('chalk'); var engine = require('./engine'); +var debug = require('debug')('tests'); + var types = require('conventional-commit-types').types; var expect = chai.expect; @@ -409,6 +411,7 @@ function commitMessage(answers, options) { engine(options, mockInquirer).prompter(function(message) { result = message; + debug('Commit Message:\n\n' + message); }); return result; } @@ -416,16 +419,29 @@ function commitMessage(answers, options) { function processQuestions(questions, answers) { for (var i in questions) { var question = questions[i]; + + var message = + typeof question.message === 'function' + ? question.message(answers) + : question.message; + + debug('Asked: ' + message); + var answer = answers[question.name]; + + debug('Answer: ' + answer); + var validation = answer === undefined || !question.validate ? true : question.validate(answer, answers); if (validation !== true) { - throw new Error( + var errorMessage = validation || - `Answer '${answer}' to question '${question.name}' was invalid` - ); + `Answer '${answer}' to question '${question.name}' was invalid`; + + debug('Threw Error: ' + errorMessage); + throw new Error(errorMessage); } if (question.filter && answer) { answers[question.name] = question.filter(answer); @@ -442,7 +458,9 @@ function getQuestions(options) { return []; }); - engine(options, mockInquirer).prompter(message => {}); + engine(options, mockInquirer).prompter(message => { + debug('Commit Message:\n\n' + message); + }); return result; } diff --git a/package-lock.json b/package-lock.json index 2cf35d28..155cc135 100644 --- a/package-lock.json +++ b/package-lock.json @@ -529,6 +529,23 @@ } } }, + "debug": { + "version": "4.1.1", + "resolved": "https://registry.npmjs.org/debug/-/debug-4.1.1.tgz", + "integrity": "sha512-pYAIzeRo8J6KPEaJ0VWOh5Pzkbw/RetuzehGM7QRRX5he4fPHx2rdKMB256ehJCkX+XRQm16eZLqLNS8RSZXZw==", + "dev": true, + "requires": { + "ms": "^2.1.1" + }, + "dependencies": { + "ms": { + "version": "2.1.2", + "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.2.tgz", + "integrity": "sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w==", + "dev": true + } + } + }, "decamelize": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/decamelize/-/decamelize-1.2.0.tgz", diff --git a/package.json b/package.json index 54f9b041..49bc4719 100644 --- a/package.json +++ b/package.json @@ -32,6 +32,7 @@ "@types/chai": "^4.1.7", "@types/mocha": "^5.2.7", "chai": "^4.2.0", + "debug": "^4.1.1", "mocha": "^6.2.0", "prettier": "^1.15.3", "proxyquire": "^2.1.3", From 23e0356a180bbf6b6431a78eb69cc9a8389bc422 Mon Sep 17 00:00:00 2001 From: Emelia Smith Date: Wed, 29 Jul 2020 03:17:58 +0200 Subject: [PATCH 4/5] feat(engine): implement scope choices Allow users to configure a predefined list of scopes that can be selected from, with the option to type their own scope if it's not on the list. When passing CZ_SCOPE, we attempt to pre-select the scope from the list of scopes, or auto-select "something else" which allows the user to confirm the CZ_SCOPE value. --- engine.js | 85 +++++++++++++++++++++++++++++++++++++++++++++----- engine.test.js | 20 ++++++++++-- index.js | 19 +++++++++++ index.test.js | 1 + 4 files changed, 116 insertions(+), 9 deletions(-) diff --git a/engine.js b/engine.js index 510ba154..44f7e1c3 100644 --- a/engine.js +++ b/engine.js @@ -38,15 +38,49 @@ var filterSubject = function(subject) { // fine. module.exports = function(options, inquirer) { var types = options.types; - - var length = longest(Object.keys(types)).length + 1; - var choices = map(types, function(type, key) { + var longestTypeChoice = longest(Object.keys(types)).length + 1; + var typeChoices = map(types, function(type, key) { return { - name: (key + ':').padEnd(length) + ' ' + type.description, + name: (key + ':').padEnd(longestTypeChoice) + ' ' + type.description, value: key }; }); + var predefinedScopes = options.scopes || []; + var hasPredefinedScopes = predefinedScopes.length > 0; + + var otherScopeChoice = 'something else...'; + var scopeChoices = [{ name: chalk.dim(chalk.white('(skip)')), value: '' }] + .concat( + map(predefinedScopes, function(scope) { + return { + name: scope, + value: scope + }; + }) + ) + .concat([ + new inquirer.Separator(''), + { name: otherScopeChoice, value: otherScopeChoice } + ]); + + var defaultScopeChoice; + + if (options.defaultScope) { + var foundDefault = scopeChoices.findIndex(function(choice) { + if (choice.type === 'separator') { + return false; + } + return ( + choice.value === options.defaultScope || + choice.name === options.defaultScope + ); + }); + + defaultScopeChoice = + foundDefault === -1 ? scopeChoices.length - 1 : foundDefault; + } + return { // When a user runs `git cz`, prompter will // be executed. @@ -70,19 +104,51 @@ module.exports = function(options, inquirer) { type: 'list', name: 'type', message: "Select the type of change that you're committing:", - choices: choices, + choices: typeChoices, default: options.defaultType }, + { + type: 'list', + name: 'scopeChoices', + message: + 'What is the scope of this change' + + (defaultScopeChoice > 0 ? '' : ' (press enter to skip)') + + ':', + choices: scopeChoices, + default: function() { + return ( + scopeChoices[defaultScopeChoice] && + scopeChoices[defaultScopeChoice].value + ); + }, + filter: function(value) { + return options.disableScopeLowerCase + ? value.trim() + : value.trim().toLowerCase(); + }, + when: function() { + return hasPredefinedScopes; + } + }, { type: 'input', name: 'scope', message: - 'What is the scope of this change (e.g. component or file name): (press enter to skip)', + 'What is the scope of this change (e.g. component or file name): ' + + chalk.grey('(press enter to skip)') + + '\n> ', default: options.defaultScope, filter: function(value) { return options.disableScopeLowerCase ? value.trim() : value.trim().toLowerCase(); + }, + when: function(answers) { + return ( + (answers.scopeChoices && + answers.scopeChoices === otherScopeChoice) || + !hasPredefinedScopes + ); } }, { @@ -195,7 +261,12 @@ module.exports = function(options, inquirer) { }; // parentheses are only needed when a scope is present - var scope = answers.scope ? '(' + answers.scope + ')' : ''; + var scope; + if (answers.scopeChoices && answers.scopeChoices !== otherScopeChoice) { + scope = '(' + answers.scopeChoices + ')'; + } else { + scope = answers.scope ? '(' + answers.scope + ')' : ''; + } // Hard limit this line in the validate var head = answers.type + scope + ': ' + answers.subject; diff --git a/engine.test.js b/engine.test.js index 3a6783a6..9ff66102 100644 --- a/engine.test.js +++ b/engine.test.js @@ -419,13 +419,27 @@ function commitMessage(answers, options) { function processQuestions(questions, answers) { for (var i in questions) { var question = questions[i]; + var skipQuestion = false; var message = typeof question.message === 'function' ? question.message(answers) : question.message; - debug('Asked: ' + message); + if (question.when) { + if (typeof question.when === 'function') { + skipQuestion = !question.when(answers); + } else { + skipQuestion = !question.when; + } + } + + if (skipQuestion) { + debug('Skipped: ' + message); + continue; + } else { + debug('Asked: ' + message); + } var answer = answers[question.name]; @@ -504,7 +518,9 @@ function questionFilter(name, answer, options) { function questionDefault(name, options) { options = options || defaultOptions; var question = getQuestion(name, options); - return question.default; + return typeof question.default === 'function' + ? question.default() + : question.default; } function questionWhen(name, answers, options) { diff --git a/index.js b/index.js index e8fbc847..ccd3483d 100644 --- a/index.js +++ b/index.js @@ -4,6 +4,8 @@ var engine = require('./engine'); var conventionalCommitTypes = require('conventional-commit-types'); var configLoader = require('commitizen').configLoader; +var IS_VALID_SCOPE_REGEX = /^[a-zA-Z0-9]+$/; + function isValidCommitlintRule(rule) { return Array.isArray(rule) && rule.length >= 3; } @@ -28,7 +30,16 @@ function loadOptions(params) { env.CZ_MAX_LINE_WIDTH && parseInt(env.CZ_MAX_LINE_WIDTH, 10); var maxLineWidth = CZ_MAX_LINE_WIDTH || config.maxLineWidth || 100; + // scopes can come from environment, commitizen config, commitlint config or the default of free choice: + var scopes = config.scopes || []; + if (env.CZ_SCOPES) { + scopes = env.CZ_SCOPES.split(',').filter(function(scope) { + return IS_VALID_SCOPE_REGEX.test(scope); + }); + } + var options = { + scopes: scopes, types: config.types || conventionalCommitTypes.types, defaultType: env.CZ_TYPE || config.defaultType, defaultScope: env.CZ_SCOPE || config.defaultScope, @@ -54,6 +65,14 @@ function loadOptions(params) { options.maxHeaderWidth = maxHeaderLengthRule[2]; } + var scopesEnumRule = clConfig.rules['scope-enum']; + if ( + isValidCommitlintRule(scopesEnumRule) && + Array.isArray(scopesEnumRule[2]) + ) { + options.scopes = scopes.concat(scopesEnumRule[2]); + } + return options; } }); diff --git a/index.test.js b/index.test.js index a4a5f485..253f3262 100644 --- a/index.test.js +++ b/index.test.js @@ -7,6 +7,7 @@ var expect = chai.expect; chai.should(); var defaultConfig = { + scopes: [], types: conventionalCommitTypes.types, defaultType: undefined, defaultScope: undefined, From 19649407cbad1b45051b593870bd1804834628f8 Mon Sep 17 00:00:00 2001 From: Emelia Smith Date: Wed, 29 Jul 2020 03:20:41 +0200 Subject: [PATCH 5/5] test: improve coverage for subject max header length Previously specifying a custom max header width wasn't actually tested for, only the configuration loading was tested. This ensures that we have coverage on this functionality. --- engine.test.js | 46 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/engine.test.js b/engine.test.js index 9ff66102..f73cd4c8 100644 --- a/engine.test.js +++ b/engine.test.js @@ -241,18 +241,42 @@ describe('commit message', function() { }); describe('validation', function() { - it('subject exceeds max length', function() { - expect(() => - commitMessage({ - type, - scope, - subject: longBody - }) - ).to.throw( - 'length must be less than or equal to ' + - `${defaultOptions.maxLineWidth - type.length - scope.length - 4}` - ); + describe('subject exceeds max length', function() { + it('using the default max length', function() { + expect(() => + commitMessage({ + type, + scope, + subject: longBody + }) + ).to.throw( + 'length must be less than or equal to ' + + `${defaultOptions.maxLineWidth - type.length - scope.length - 4}` + ); + }); + + it('using a custom max length', function() { + var customMaxHeaderWidth = 30; + + expect(() => + commitMessage( + { + type, + scope, + subject: longBody + }, + { + ...defaultOptions, + maxHeaderWidth: customMaxHeaderWidth + } + ) + ).to.throw( + 'length must be less than or equal to ' + + `${customMaxHeaderWidth - type.length - scope.length - 4}` + ); + }); }); + it('empty subject', function() { expect(() => commitMessage({