From d69c65b8b5f7fa00dfecf52633fa6edd6bad6e29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iiro=20J=C3=A4ppinen?= Date: Wed, 22 Apr 2020 17:13:12 +0300 Subject: [PATCH] fix: log task output after running listr to keep everything --- lib/gitWorkflow.js | 2 ++ lib/index.js | 7 ++-- lib/resolveTaskFn.js | 45 ++++++++++++----------- lib/runAll.js | 23 ++++++------ lib/state.js | 8 +++++ test/__snapshots__/index.spec.js.snap | 6 +++- test/index2.spec.js | 6 ++-- test/resolveTaskFn.spec.js | 51 ++++++++++++++++++--------- test/resolveTaskFn.unmocked.spec.js | 2 +- test/runAll.spec.js | 12 +++---- test/runAll.unmocked.2.spec.js | 6 ++-- test/runAll.unmocked.spec.js | 24 +++++++++---- 12 files changed, 115 insertions(+), 77 deletions(-) diff --git a/lib/gitWorkflow.js b/lib/gitWorkflow.js index e79578b1b..d02144503 100644 --- a/lib/gitWorkflow.js +++ b/lib/gitWorkflow.js @@ -189,6 +189,8 @@ class GitWorkflow { const unstagedPatch = this.getHiddenFilepath(PATCH_UNSTAGED) const files = processRenames(this.partiallyStagedFiles) await this.execGit(['diff', ...GIT_DIFF_ARGS, '--output', unstagedPatch, '--', ...files]) + } else { + ctx.hasPartiallyStagedFiles = false } /** diff --git a/lib/index.js b/lib/index.js index b46332787..217cb66f8 100644 --- a/lib/index.js +++ b/lib/index.js @@ -2,12 +2,12 @@ const dedent = require('dedent') const { cosmiconfig } = require('cosmiconfig') +const debugLog = require('debug')('lint-staged') const stringifyObject = require('stringify-object') + const runAll = require('./runAll') const validateConfig = require('./validateConfig') -const debugLog = require('debug')('lint-staged') - const errConfigNotFound = new Error('Config could not be found') function resolveConfig(configPath) { @@ -104,6 +104,9 @@ module.exports = async function lintStaged( debugLog('tasks were executed successfully!') return true } catch (runAllError) { + if (runAllError && runAllError.ctx && runAllError.ctx.output) { + logger.error(...runAllError.ctx.output) + } return false } } catch (lintStagedError) { diff --git a/lib/resolveTaskFn.js b/lib/resolveTaskFn.js index 4c2e6b174..70d14d8f0 100644 --- a/lib/resolveTaskFn.js +++ b/lib/resolveTaskFn.js @@ -1,16 +1,14 @@ 'use strict' -const chalk = require('chalk') -const dedent = require('dedent') +const { redBright, dim } = require('chalk') const execa = require('execa') const debug = require('debug')('lint-staged:task') -const symbols = require('log-symbols') const { parseArgsStringToArgv } = require('string-argv') +const { error } = require('log-symbols') +const { getInitialState } = require('./state') const { TaskError } = require('./symbols') -const successMsg = (linter) => `${symbols.success} ${linter} passed!` - /** * Create a failure message dependding on process result. * @@ -21,23 +19,27 @@ const successMsg = (linter) => `${symbols.success} ${linter} passed!` * @param {boolean} result.failed * @param {boolean} result.killed * @param {string} result.signal - * @param {Object} ctx (see https://github.com/SamVerschueren/listr#context) + * @param {Object} ctx * @returns {Error} */ -const makeErr = (linter, result, ctx) => { +const makeErr = (command, result, ctx) => { ctx.errors.add(TaskError) - const { stdout, stderr, killed, signal } = result + const { code, killed, signal, stderr, stdout } = result + + const tag = signal || (killed && 'KILLED') || code || 'FAILED' - if (killed || (signal && signal !== '')) { - throw new Error(`${chalk.yellow(`${linter} was terminated with ${signal}`)}`) + const hasOutput = !!stderr || !!stdout + if (hasOutput) { + const errorTitle = redBright(`${error} ${command}:`) + const output = ['', errorTitle].concat(stderr ? [stderr] : []).concat(stdout ? [stdout] : []) + ctx.output.push(output.join('\n')) + } else { + // Show generic error when task had no output + const message = redBright(`\n${error} ${command} failed without output (${tag}).`) + ctx.output.push(message) } - throw new Error(dedent`${chalk.redBright( - `${linter} found some errors. Please fix them and try committing again.` - )} - ${stdout} - ${stderr} - `) + return new Error(`${redBright(command)} ${dim(`[${tag}]`)}`) } /** @@ -67,16 +69,13 @@ module.exports = function resolveTaskFn({ command, files, gitDir, isFn, relative } debug('execaOptions:', execaOptions) - return async (ctx) => { - const promise = shell + return async (ctx = getInitialState()) => { + const result = await (shell ? execa.command(isFn ? command : `${command} ${files.join(' ')}`, execaOptions) - : execa(cmd, isFn ? args : args.concat(files), execaOptions) - const result = await promise + : execa(cmd, isFn ? args : args.concat(files), execaOptions)) if (result.failed || result.killed || result.signal != null) { - throw makeErr(cmd, result, ctx) + throw makeErr(command, result, ctx) } - - return successMsg(cmd) } } diff --git a/lib/runAll.js b/lib/runAll.js index c6ee040dc..c0796c3f6 100644 --- a/lib/runAll.js +++ b/lib/runAll.js @@ -24,6 +24,7 @@ const { ApplyEmptyCommitError, GetBackupStashError, GitError } = require('./symb const { applyModificationsSkipped, cleanupSkipped, + getInitialState, hasPartiallyStagedFiles, restoreOriginalStateEnabled, restoreOriginalStateSkipped, @@ -106,14 +107,11 @@ const runAll = async ( // Warn user when their command includes `git add` let hasDeprecatedGitAdd = false - const listrCtx = { - hasPartiallyStagedFiles: false, - shouldBackup, - errors: new Set([]) - } + const ctx = getInitialState() + ctx.shouldBackup = shouldBackup const listrOptions = { - ctx: listrCtx, + ctx, dateFormat: false, exitOnError: false, renderer: getRenderer({ debug, quiet }) @@ -150,7 +148,6 @@ const runAll = async ( // In sub-tasks we don't want to run concurrently // and we want to abort on errors ...listrOptions, - collapse: false, concurrent: false, exitOnError: true }), @@ -171,7 +168,7 @@ const runAll = async ( task: () => new Listr(chunkListrTasks, { ...listrOptions, concurrent }), skip: () => { // Skip if the first step (backup) failed - if (listrCtx.errors.has(GitError)) return SKIPPED_GIT_ERROR + if (ctx.errors.has(GitError)) return SKIPPED_GIT_ERROR // Skip chunk when no every task is skipped (due to no matches) if (chunkListrTasks.every((task) => task.skip())) return 'No tasks to run.' return false @@ -231,12 +228,12 @@ const runAll = async ( listrOptions ) - const context = await runner.run() + await runner.run() - if (context.errors.size > 0) { - if (context.errors.has(ApplyEmptyCommitError)) { + if (ctx.errors.size > 0) { + if (ctx.errors.has(ApplyEmptyCommitError)) { logger.warn(PREVENTED_EMPTY_COMMIT) - } else if (context.errors.has(GitError) && !context.errors.has(GetBackupStashError)) { + } else if (ctx.errors.has(GitError) && !ctx.errors.has(GetBackupStashError)) { logger.error(GIT_ERROR) if (shouldBackup) { // No sense to show this if the backup stash itself is missing. @@ -244,7 +241,7 @@ const runAll = async ( } } const error = new Error('lint-staged failed') - throw Object.assign(error, context) + throw Object.assign(error, { ctx }) } } diff --git a/lib/state.js b/lib/state.js index 0edefd8a8..6fc36c856 100644 --- a/lib/state.js +++ b/lib/state.js @@ -9,6 +9,13 @@ const { RestoreUnstagedChangesError } = require('./symbols') +const getInitialState = () => ({ + hasPartiallyStagedFiles: null, + shouldBackup: null, + errors: new Set([]), + output: [] +}) + const hasPartiallyStagedFiles = (ctx) => ctx.hasPartiallyStagedFiles const applyModificationsSkipped = (ctx) => { @@ -68,6 +75,7 @@ const cleanupSkipped = (ctx) => { } module.exports = { + getInitialState, hasPartiallyStagedFiles, applyModificationsSkipped, restoreUnstagedChangesSkipped, diff --git a/test/__snapshots__/index.spec.js.snap b/test/__snapshots__/index.spec.js.snap index 8ec97131c..34cc1f06a 100644 --- a/test/__snapshots__/index.spec.js.snap +++ b/test/__snapshots__/index.spec.js.snap @@ -1,6 +1,10 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`lintStaged should exit with code 1 on linter errors 1`] = `""`; +exports[`lintStaged should exit with code 1 on linter errors 1`] = ` +" +ERROR +× node -e \\"process.exit(1)\\" failed without output (FAILED)." +`; exports[`lintStaged should load an npm config package when specified 1`] = ` " diff --git a/test/index2.spec.js b/test/index2.spec.js index 71eaa271e..06951df85 100644 --- a/test/index2.spec.js +++ b/test/index2.spec.js @@ -26,7 +26,8 @@ describe('lintStaged', () => { Object { "ctx": Object { "errors": Set {}, - "hasPartiallyStagedFiles": false, + "hasPartiallyStagedFiles": null, + "output": Array [], "shouldBackup": true, }, "dateFormat": false, @@ -49,7 +50,8 @@ describe('lintStaged', () => { Object { "ctx": Object { "errors": Set {}, - "hasPartiallyStagedFiles": false, + "hasPartiallyStagedFiles": null, + "output": Array [], "shouldBackup": true, }, "dateFormat": false, diff --git a/test/resolveTaskFn.spec.js b/test/resolveTaskFn.spec.js index fe863f77b..b72073cba 100644 --- a/test/resolveTaskFn.spec.js +++ b/test/resolveTaskFn.spec.js @@ -1,12 +1,11 @@ import execa from 'execa' import resolveTaskFn from '../lib/resolveTaskFn' +import { getInitialState } from '../lib/state' import { TaskError } from '../lib/symbols' const defaultOpts = { files: ['test.js'] } -const defaultCtx = { errors: new Set() } - describe('resolveTaskFn', () => { beforeEach(() => { execa.mockClear() @@ -19,7 +18,7 @@ describe('resolveTaskFn', () => { command: 'node --arg=true ./myscript.js' }) - await taskFn(defaultCtx) + await taskFn() expect(execa).toHaveBeenCalledTimes(1) expect(execa).lastCalledWith('node', ['--arg=true', './myscript.js', 'test.js'], { preferLocal: true, @@ -36,7 +35,7 @@ describe('resolveTaskFn', () => { command: 'node --arg=true ./myscript.js test.js' }) - await taskFn(defaultCtx) + await taskFn() expect(execa).toHaveBeenCalledTimes(1) expect(execa).lastCalledWith('node', ['--arg=true', './myscript.js', 'test.js'], { preferLocal: true, @@ -54,7 +53,7 @@ describe('resolveTaskFn', () => { command: 'node --arg=true ./myscript.js test.js' }) - await taskFn(defaultCtx) + await taskFn() expect(execa).toHaveBeenCalledTimes(1) expect(execa).lastCalledWith('node --arg=true ./myscript.js test.js', { preferLocal: true, @@ -71,7 +70,7 @@ describe('resolveTaskFn', () => { command: 'node --arg=true ./myscript.js' }) - await taskFn(defaultCtx) + await taskFn() expect(execa).toHaveBeenCalledTimes(1) expect(execa).lastCalledWith('node --arg=true ./myscript.js test.js', { preferLocal: true, @@ -88,7 +87,7 @@ describe('resolveTaskFn', () => { gitDir: '../' }) - await taskFn(defaultCtx) + await taskFn() expect(execa).toHaveBeenCalledTimes(1) expect(execa).lastCalledWith('git', ['diff', 'test.js'], { cwd: '../', @@ -102,7 +101,7 @@ describe('resolveTaskFn', () => { expect.assertions(2) const taskFn = resolveTaskFn({ ...defaultOpts, command: 'jest', gitDir: '../' }) - await taskFn(defaultCtx) + await taskFn() expect(execa).toHaveBeenCalledTimes(1) expect(execa).lastCalledWith('jest', ['test.js'], { preferLocal: true, @@ -119,7 +118,7 @@ describe('resolveTaskFn', () => { relative: true }) - await taskFn(defaultCtx) + await taskFn() expect(execa).toHaveBeenCalledTimes(1) expect(execa).lastCalledWith('git', ['diff', 'test.js'], { cwd: process.cwd(), @@ -140,10 +139,10 @@ describe('resolveTaskFn', () => { }) const taskFn = resolveTaskFn({ ...defaultOpts, command: 'mock-fail-linter' }) - await expect(taskFn(defaultCtx)).rejects.toThrow('mock-fail-linter found some errors') + await expect(taskFn()).rejects.toThrowErrorMatchingInlineSnapshot(`"mock-fail-linter [FAILED]"`) }) - it('should throw error for killed processes', async () => { + it('should throw error for interrupted processes', async () => { expect.assertions(1) execa.mockResolvedValueOnce({ stdout: 'Mock error', @@ -156,14 +155,32 @@ describe('resolveTaskFn', () => { }) const taskFn = resolveTaskFn({ ...defaultOpts, command: 'mock-killed-linter' }) - await expect(taskFn(defaultCtx)).rejects.toThrow( - 'mock-killed-linter was terminated with SIGINT' + await expect(taskFn()).rejects.toThrowErrorMatchingInlineSnapshot( + `"mock-killed-linter [SIGINT]"` + ) + }) + + it('should throw error for killed processes without signal', async () => { + expect.assertions(1) + execa.mockResolvedValueOnce({ + stdout: 'Mock error', + stderr: '', + code: 0, + failed: false, + killed: true, + signal: undefined, + cmd: 'mock cmd' + }) + + const taskFn = resolveTaskFn({ ...defaultOpts, command: 'mock-killed-linter' }) + await expect(taskFn()).rejects.toThrowErrorMatchingInlineSnapshot( + `"mock-killed-linter [KILLED]"` ) }) it('should not add TaskError if no error occur', async () => { expect.assertions(1) - const context = { errors: new Set() } + const context = getInitialState() const taskFn = resolveTaskFn({ ...defaultOpts, command: 'jest', gitDir: '../' }) await taskFn(context) expect(context.errors.has(TaskError)).toEqual(false) @@ -177,10 +194,12 @@ describe('resolveTaskFn', () => { failed: true, cmd: 'mock cmd' }) - const context = { errors: new Set() } + const context = getInitialState() const taskFn = resolveTaskFn({ ...defaultOpts, command: 'mock-fail-linter' }) expect.assertions(2) - await expect(taskFn(context)).rejects.toThrow('mock-fail-linter found some errors') + await expect(taskFn(context)).rejects.toThrowErrorMatchingInlineSnapshot( + `"mock-fail-linter [FAILED]"` + ) expect(context.errors.has(TaskError)).toEqual(true) }) }) diff --git a/test/resolveTaskFn.unmocked.spec.js b/test/resolveTaskFn.unmocked.spec.js index c2141a471..35ab545d7 100644 --- a/test/resolveTaskFn.unmocked.spec.js +++ b/test/resolveTaskFn.unmocked.spec.js @@ -11,6 +11,6 @@ describe('resolveTaskFn', () => { shell: true }) - await expect(taskFn()).resolves.toMatchInlineSnapshot(`"√ node passed!"`) + await expect(taskFn()).resolves.toMatchInlineSnapshot(`undefined`) }) }) diff --git a/test/runAll.spec.js b/test/runAll.spec.js index d005ae014..4a67dbaf8 100644 --- a/test/runAll.spec.js +++ b/test/runAll.spec.js @@ -138,12 +138,8 @@ describe('runAll', () => { INFO ❯ Running tasks... INFO ❯ Running tasks for *.js INFO ❯ echo \\"sample\\" - ERROR ✖ echo found some errors. Please fix them and try committing again. - ✖ - ✖ Linter finished with error - ERROR ✖ echo found some errors. Please fix them and try committing again. - ✖ - ✖ Linter finished with error + ERROR ✖ echo \\"sample\\" [1] + ERROR ✖ echo \\"sample\\" [1] LOG ✔ Running tasks... INFO ❯ Applying modifications... INFO ❯ Reverting to original state because of errors... @@ -179,8 +175,8 @@ describe('runAll', () => { INFO ❯ Running tasks... INFO ❯ Running tasks for *.js INFO ❯ echo \\"sample\\" - ERROR ✖ echo was terminated with SIGINT - ERROR ✖ echo was terminated with SIGINT + ERROR ✖ echo \\"sample\\" [SIGINT] + ERROR ✖ echo \\"sample\\" [SIGINT] LOG ✔ Running tasks... INFO ❯ Applying modifications... INFO ❯ Reverting to original state because of errors... diff --git a/test/runAll.unmocked.2.spec.js b/test/runAll.unmocked.2.spec.js index dcc150157..b4287568b 100644 --- a/test/runAll.unmocked.2.spec.js +++ b/test/runAll.unmocked.2.spec.js @@ -111,10 +111,8 @@ describe('runAll', () => { INFO ❯ Running tasks... INFO ❯ Running tasks for *.js INFO ❯ prettier --list-different - ERROR ✖ prettier found some errors. Please fix them and try committing again. - ✖ ../../../..${cwd}/test.js - ERROR ✖ prettier found some errors. Please fix them and try committing again. - ✖ ../../../..${cwd}/test.js + ERROR ✖ prettier --list-different [FAILED] + ERROR ✖ prettier --list-different [FAILED] LOG ✔ Running tasks... INFO ❯ Applying modifications... INFO ❯ Reverting to original state because of errors... diff --git a/test/runAll.unmocked.spec.js b/test/runAll.unmocked.spec.js index b24b43791..2d24fea58 100644 --- a/test/runAll.unmocked.spec.js +++ b/test/runAll.unmocked.spec.js @@ -321,10 +321,8 @@ describe('runAll', () => { INFO ❯ Running tasks... INFO ❯ Running tasks for *.js INFO ❯ prettier --list-different - ERROR ✖ prettier found some errors. Please fix them and try committing again. - ✖ ../../../..${cwd}/test.js - ERROR ✖ prettier found some errors. Please fix them and try committing again. - ✖ ../../../..${cwd}/test.js + ERROR ✖ prettier --list-different [FAILED] + ERROR ✖ prettier --list-different [FAILED] LOG ✔ Running tasks... INFO ❯ Applying modifications... INFO ❯ Restoring unstaged changes to partially staged files... @@ -1034,9 +1032,21 @@ describe('runAll', () => { }) ).rejects.toThrowErrorMatchingInlineSnapshot(`"lint-staged failed"`) - expect(console.printHistory()).toMatch( - 'prettier found some errors. Please fix them and try committing again' - ) + expect(console.printHistory()).toMatchInlineSnapshot(` + " + WARN ‼ Skipping backup because \`--no-stash\` was used. + + INFO ❯ Preparing... + LOG ✔ Preparing... + INFO ❯ Running tasks... + INFO ❯ Running tasks for *.js + INFO ❯ prettier --write + ERROR ✖ prettier --write [FAILED] + ERROR ✖ prettier --write [FAILED] + LOG ✔ Running tasks... + INFO ❯ Applying modifications... + LOG ✔ Applying modifications..." + `) // Something was wrong, so the commit was aborted expect(await execGit(['rev-list', '--count', 'HEAD'])).toEqual('1')