From 98954852ebd9c8ee322a25f5e066c56e519a3c05 Mon Sep 17 00:00:00 2001 From: Kelvin Jin Date: Fri, 13 Oct 2017 14:47:40 -0700 Subject: [PATCH 1/6] warn when clang-format fails --- src/format.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/format.ts b/src/format.ts index 386eeae9..662e5113 100644 --- a/src/format.ts +++ b/src/format.ts @@ -27,7 +27,7 @@ const baseArgs = * @param fix whether to automatically fix the format * @param files files to format */ -export function format( +export async function format( options: Options, files: string[] = [], fix = false): Promise { const program = createProgram(options); const srcFiles = files.length > 0 ? @@ -36,7 +36,16 @@ export function format( .map(sourceFile => sourceFile.fileName) .filter(f => !f.endsWith('.d.ts')); - return fix ? fixFormat(srcFiles) : checkFormat(srcFiles); + if (fix) { + return fixFormat(srcFiles); + } else { + const result = await checkFormat(srcFiles); + if (!result) { + options.logger.error( + 'clang-format reported errors... run `gts fix` to address.'); + } + return result; + } } /** From 738125a24c56f83aebe7662fb189bdd1f96cc277 Mon Sep 17 00:00:00 2001 From: Kelvin Jin Date: Tue, 17 Oct 2017 11:16:03 -0700 Subject: [PATCH 2/6] make travis green --- package.json | 2 +- src/format.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 0b324523..83675e25 100644 --- a/package.json +++ b/package.json @@ -26,7 +26,7 @@ "format-check": "./bin/format-check.sh", "format": "clang-format -i -style='{Language: JavaScript, BasedOnStyle: Google, ColumnLimit: 80}' src/*.ts test/*.ts", "lint": "tslint -c tslint.json --project . --type-check -t codeFrame", - "lint-fix": "tslint -c tslint.json --project . --type-check -t codeFrame --fix", + "lint-fix": "tslint -c tslint.json --project . --type-check -t codeFrame --fix", "prepare": "npm run compile", "test": "npm run compile && nyc ava build/test/test*.js", "posttest": "npm run lint && npm run format-check" diff --git a/src/format.ts b/src/format.ts index 662e5113..cb1a1010 100644 --- a/src/format.ts +++ b/src/format.ts @@ -42,7 +42,7 @@ export async function format( const result = await checkFormat(srcFiles); if (!result) { options.logger.error( - 'clang-format reported errors... run `gts fix` to address.'); + 'clang-format reported errors... run `gts fix` to address.'); } return result; } From f732709963e93f92434b569e2048ccb18a419d6c Mon Sep 17 00:00:00 2001 From: Kelvin Jin Date: Tue, 17 Oct 2017 15:51:47 -0700 Subject: [PATCH 3/6] add test --- src/cli.ts | 4 ++- src/format.ts | 2 +- test/fixtures/kitchen/src/server.ts | 2 +- test/test-kitchen.ts | 40 +++++++++++++++++++++-------- 4 files changed, 35 insertions(+), 13 deletions(-) diff --git a/src/cli.ts b/src/cli.ts index 88a09f1c..f333fb9b 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -86,7 +86,9 @@ async function run(verb: string, files: string[]): Promise { const format: VerbFilesFunction = require('./format').format; switch (verb) { case 'check': - return (await lint(options, files) && await format(options, files)); + const passLint = await lint(options, files); + const passFormat = await format(options, files); + return passLint && passFormat; case 'fix': return ( await lint(options, files, true) && diff --git a/src/format.ts b/src/format.ts index cb1a1010..796c244a 100644 --- a/src/format.ts +++ b/src/format.ts @@ -41,7 +41,7 @@ export async function format( } else { const result = await checkFormat(srcFiles); if (!result) { - options.logger.error( + options.logger.log( 'clang-format reported errors... run `gts fix` to address.'); } return result; diff --git a/test/fixtures/kitchen/src/server.ts b/test/fixtures/kitchen/src/server.ts index f463fff9..da0e69a3 100644 --- a/test/fixtures/kitchen/src/server.ts +++ b/test/fixtures/kitchen/src/server.ts @@ -1 +1 @@ -const isThisTypeScript = true + const isThisTypeScript = true diff --git a/test/test-kitchen.ts b/test/test-kitchen.ts index 01ef0508..e3ca1df2 100644 --- a/test/test-kitchen.ts +++ b/test/test-kitchen.ts @@ -8,14 +8,31 @@ import * as pify from 'pify'; import * as rimraf from 'rimraf'; import * as tmp from 'tmp'; +interface ExecResult { + exitCode: number; + stdout: string; + stderr: string; +} + const pkg = require('../../package.json'); const rimrafp = pify(rimraf); const mkdirp = pify(fs.mkdir); -const execp = pify(cp.exec); +const simpleExecp = pify(cp.exec); const renamep = pify(fs.rename); const ncpp = pify(ncp.ncp); +const execp = + (command: string, execOptions?: cp.ExecOptions): Promise => { + return new Promise((resolve) => { + cp.exec( + command, execOptions || {}, + (err: Error&{code: number}, stdout, stderr) => { + resolve({exitCode: err ? err.code : 0, stdout, stderr}); + }); + }); + }; + const keep = !!process.env.GTS_KEEP_TEMPDIRS; const stagingDir = tmp.dirSync({keep: keep, unsafeCleanup: true}); const stagingPath = stagingDir.name; @@ -30,43 +47,46 @@ console.log(`${chalk.blue(`${__filename} staging area: ${stagingPath}`)}`); * to test on a fresh application. */ test.before(async () => { - await execp('npm pack'); + await simpleExecp('npm pack'); const tarball = `${pkg.name}-${pkg.version}.tgz`; await renamep(tarball, `${stagingPath}/gts.tgz`); await ncpp('test/fixtures', `${stagingPath}/`); - await execp('npm install', execOpts); + await simpleExecp('npm install', execOpts); }); test.serial('init', async t => { - await execp('./node_modules/.bin/gts init -y', execOpts); + await simpleExecp('./node_modules/.bin/gts init -y', execOpts); fs.accessSync(`${stagingPath}/kitchen/tsconfig.json`); t.pass(); }); test.serial('check before fix', async t => { - await t.throws(execp('npm run check', execOpts)); + const {exitCode, stdout} = await execp('npm run check', execOpts); + t.deepEqual(exitCode, 1); + t.notDeepEqual(stdout.indexOf('clang-format reported errors'), -1); t.pass(); }); test.serial('fix', async t => { const preFix = fs.readFileSync(`${stagingPath}/kitchen/src/server.ts`, 'utf8') .split('\n'); - await execp('npm run fix', execOpts); + await simpleExecp('npm run fix', execOpts); const postFix = fs.readFileSync(`${stagingPath}/kitchen/src/server.ts`, 'utf8') .split('\n'); t.deepEqual( - preFix[0] + ';', postFix[0]); // fix should have added a semi-colon + preFix[0].trim() + ';', + postFix[0]); // fix should have added a semi-colon t.pass(); }); test.serial('check after fix', async t => { - await execp('npm run check', execOpts); + await simpleExecp('npm run check', execOpts); t.pass(); }); test.serial('build', async t => { - await execp('npm run compile', execOpts); + await simpleExecp('npm run compile', execOpts); fs.accessSync(`${stagingPath}/kitchen/build/src/server.js`); fs.accessSync(`${stagingPath}/kitchen/build/src/server.js.map`); fs.accessSync(`${stagingPath}/kitchen/build/src/server.d.ts`); @@ -78,7 +98,7 @@ test.serial('build', async t => { * output dir */ test.serial('clean', async t => { - await execp('npm run clean', execOpts); + await simpleExecp('npm run clean', execOpts); t.throws(() => { fs.accessSync(`${stagingPath}/kitchen/build`); }); From 43b1edcd81e4d64382230ab3be81548331d21034 Mon Sep 17 00:00:00 2001 From: Kelvin Jin Date: Wed, 18 Oct 2017 10:13:38 -0700 Subject: [PATCH 4/6] fix clang-format version --- package-lock.json | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/package-lock.json b/package-lock.json index 555fb971..655cdf33 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1411,9 +1411,9 @@ "dev": true }, "clang-format": { - "version": "1.0.53", - "resolved": "https://registry.npmjs.org/clang-format/-/clang-format-1.0.53.tgz", - "integrity": "sha1-/zZoxA7M7u6qPpd7YdUBBtCz/UY=", + "version": "1.1.0", + "resolved": "https://registry.npmjs.org/clang-format/-/clang-format-1.1.0.tgz", + "integrity": "sha512-0Aru49uTLoROl4sPTyO3fvF/NZ+fnGEy5i7bsRwA/bAYT+eWaAxK3sDxRvm/AW7Nwq83BvARH/npeF8OIAaGVQ==", "requires": { "async": "1.5.2", "glob": "7.1.2", @@ -2773,22 +2773,22 @@ } } }, - "string-width": { - "version": "1.0.2", + "string_decoder": { + "version": "1.0.1", "bundled": true, "dev": true, "requires": { - "code-point-at": "1.1.0", - "is-fullwidth-code-point": "1.0.0", - "strip-ansi": "3.0.1" + "safe-buffer": "5.0.1" } }, - "string_decoder": { - "version": "1.0.1", + "string-width": { + "version": "1.0.2", "bundled": true, "dev": true, "requires": { - "safe-buffer": "5.0.1" + "code-point-at": "1.1.0", + "is-fullwidth-code-point": "1.0.0", + "strip-ansi": "3.0.1" } }, "stringstream": { @@ -6361,6 +6361,15 @@ "integrity": "sha1-1PM6tU6OOHeLDKXP07OvsS22hiA=", "dev": true }, + "string_decoder": { + "version": "1.0.3", + "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-1.0.3.tgz", + "integrity": "sha512-4AH6Z5fzNNBcH+6XDMfA/BTt87skxqJlO0lAh3Dker5zThcAxG6mKz+iGu308UKoPPQ8Dcqx/4JhujzltRa+hQ==", + "dev": true, + "requires": { + "safe-buffer": "5.1.1" + } + }, "string-width": { "version": "2.1.1", "resolved": "https://registry.npmjs.org/string-width/-/string-width-2.1.1.tgz", @@ -6385,15 +6394,6 @@ } } }, - "string_decoder": { - "version": "1.0.3", - "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-1.0.3.tgz", - "integrity": "sha512-4AH6Z5fzNNBcH+6XDMfA/BTt87skxqJlO0lAh3Dker5zThcAxG6mKz+iGu308UKoPPQ8Dcqx/4JhujzltRa+hQ==", - "dev": true, - "requires": { - "safe-buffer": "5.1.1" - } - }, "stringstream": { "version": "0.0.5", "resolved": "https://registry.npmjs.org/stringstream/-/stringstream-0.0.5.tgz", From 2e4711aaa509c8e8643b0f69842d79a5c15392ca Mon Sep 17 00:00:00 2001 From: Kelvin Jin Date: Wed, 18 Oct 2017 12:55:14 -0700 Subject: [PATCH 5/6] bump chalk to 2.2 --- package-lock.json | 48 +++++++++++++++++++------------------------- package.json | 3 +-- src/clean.ts | 2 +- src/init.ts | 2 +- test/test-kitchen.ts | 2 +- 5 files changed, 25 insertions(+), 32 deletions(-) diff --git a/package-lock.json b/package-lock.json index 655cdf33..08523cd0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -80,12 +80,6 @@ "arrify": "1.0.1" } }, - "@types/chalk": { - "version": "0.4.31", - "resolved": "https://registry.npmjs.org/@types/chalk/-/chalk-0.4.31.tgz", - "integrity": "sha1-ox10JBprHtu5c8822XooloNKUfk=", - "dev": true - }, "@types/glob": { "version": "5.0.30", "resolved": "https://registry.npmjs.org/@types/glob/-/glob-5.0.30.tgz", @@ -490,7 +484,7 @@ "babel-core": "6.26.0", "bluebird": "3.5.0", "caching-transform": "1.0.1", - "chalk": "2.0.1", + "chalk": "2.2.0", "chokidar": "1.7.0", "clean-stack": "1.3.0", "clean-yaml-object": "0.1.0", @@ -1378,9 +1372,9 @@ "dev": true }, "chalk": { - "version": "2.0.1", - "resolved": "https://registry.npmjs.org/chalk/-/chalk-2.0.1.tgz", - "integrity": "sha512-Mp+FXEI+FrwY/XYV45b2YD3E8i3HwnEAoFcM0qlZzq/RZ9RwWitt2Y/c7cqRAz70U7hfekqx6qNYthuKFO6K0g==", + "version": "2.2.0", + "resolved": "https://registry.npmjs.org/chalk/-/chalk-2.2.0.tgz", + "integrity": "sha512-0BMM/2hG3ZaoPfR6F+h/oWpZtsh3b/s62TjSM6MGCJWEbJDN1acqCXvyhhZsDSVFklpebUoQ5O1kKC7lOzrn9g==", "requires": { "ansi-styles": "3.2.0", "escape-string-regexp": "1.0.5", @@ -2773,14 +2767,6 @@ } } }, - "string_decoder": { - "version": "1.0.1", - "bundled": true, - "dev": true, - "requires": { - "safe-buffer": "5.0.1" - } - }, "string-width": { "version": "1.0.2", "bundled": true, @@ -2791,6 +2777,14 @@ "strip-ansi": "3.0.1" } }, + "string_decoder": { + "version": "1.0.1", + "bundled": true, + "dev": true, + "requires": { + "safe-buffer": "5.0.1" + } + }, "stringstream": { "version": "0.0.5", "bundled": true, @@ -6361,15 +6355,6 @@ "integrity": "sha1-1PM6tU6OOHeLDKXP07OvsS22hiA=", "dev": true }, - "string_decoder": { - "version": "1.0.3", - "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-1.0.3.tgz", - "integrity": "sha512-4AH6Z5fzNNBcH+6XDMfA/BTt87skxqJlO0lAh3Dker5zThcAxG6mKz+iGu308UKoPPQ8Dcqx/4JhujzltRa+hQ==", - "dev": true, - "requires": { - "safe-buffer": "5.1.1" - } - }, "string-width": { "version": "2.1.1", "resolved": "https://registry.npmjs.org/string-width/-/string-width-2.1.1.tgz", @@ -6394,6 +6379,15 @@ } } }, + "string_decoder": { + "version": "1.0.3", + "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-1.0.3.tgz", + "integrity": "sha512-4AH6Z5fzNNBcH+6XDMfA/BTt87skxqJlO0lAh3Dker5zThcAxG6mKz+iGu308UKoPPQ8Dcqx/4JhujzltRa+hQ==", + "dev": true, + "requires": { + "safe-buffer": "5.1.1" + } + }, "stringstream": { "version": "0.0.5", "resolved": "https://registry.npmjs.org/stringstream/-/stringstream-0.0.5.tgz", diff --git a/package.json b/package.json index 83675e25..140758cd 100644 --- a/package.json +++ b/package.json @@ -38,7 +38,7 @@ "author": "Google Inc.", "license": "Apache-2.0", "dependencies": { - "chalk": "^2.0.1", + "chalk": "^2.2.0", "clang-format": "^1.0.53", "inquirer": "^3.2.1", "meow": "^3.7.0", @@ -49,7 +49,6 @@ "write-file-atomic": "^2.1.0" }, "devDependencies": { - "@types/chalk": "^0.4.31", "@types/glob": "^5.0.30", "@types/inquirer": "0.0.35", "@types/make-dir": "^1.0.1", diff --git a/src/clean.ts b/src/clean.ts index dbbba7f6..801ebcfe 100644 --- a/src/clean.ts +++ b/src/clean.ts @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import * as chalk from 'chalk'; +import chalk from 'chalk'; import {Options} from './cli'; import {getTSConfig, rimrafp} from './util'; diff --git a/src/init.ts b/src/init.ts index e9a73a39..17fa1ba1 100644 --- a/src/init.ts +++ b/src/init.ts @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import * as chalk from 'chalk'; +import chalk from 'chalk'; import * as cp from 'child_process'; import * as inquirer from 'inquirer'; import * as path from 'path'; diff --git a/test/test-kitchen.ts b/test/test-kitchen.ts index e3ca1df2..b456cad5 100644 --- a/test/test-kitchen.ts +++ b/test/test-kitchen.ts @@ -1,5 +1,5 @@ import test from 'ava'; -import * as chalk from 'chalk'; +import chalk from 'chalk'; import * as cp from 'child_process'; import * as fs from 'fs'; import * as ncp from 'ncp'; From dbb66303af7f124706920e9487d27ab9a705c718 Mon Sep 17 00:00:00 2001 From: Kelvin Jin Date: Wed, 18 Oct 2017 18:00:40 -0700 Subject: [PATCH 6/6] add comment about execp --- test/test-kitchen.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/test-kitchen.ts b/test/test-kitchen.ts index b456cad5..251e82c0 100644 --- a/test/test-kitchen.ts +++ b/test/test-kitchen.ts @@ -22,6 +22,11 @@ const simpleExecp = pify(cp.exec); const renamep = pify(fs.rename); const ncpp = pify(ncp.ncp); +// cp.exec doesn't fit the (err ^ result) pattern because a process can write +// to stdout/stderr and still exit with error code != 0. +// In most cases simply promisifying cp.exec is adequate, but it's not if we +// need to see console output for a process that exited with a non-zero exit +// code, so we define a more exhaustive promsified cp.exec here. const execp = (command: string, execOptions?: cp.ExecOptions): Promise => { return new Promise((resolve) => {