Skip to content

Commit

Permalink
fix: log task output after running listr to keep everything
Browse files Browse the repository at this point in the history
  • Loading branch information
iiroj committed Apr 24, 2020
1 parent e95d1b0 commit d69c65b
Show file tree
Hide file tree
Showing 12 changed files with 115 additions and 77 deletions.
2 changes: 2 additions & 0 deletions lib/gitWorkflow.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

/**
Expand Down
7 changes: 5 additions & 2 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
45 changes: 22 additions & 23 deletions lib/resolveTaskFn.js
Original file line number Diff line number Diff line change
@@ -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.
*
Expand All @@ -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}]`)}`)
}

/**
Expand Down Expand Up @@ -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)
}
}
23 changes: 10 additions & 13 deletions lib/runAll.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const { ApplyEmptyCommitError, GetBackupStashError, GitError } = require('./symb
const {
applyModificationsSkipped,
cleanupSkipped,
getInitialState,
hasPartiallyStagedFiles,
restoreOriginalStateEnabled,
restoreOriginalStateSkipped,
Expand Down Expand Up @@ -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 })
Expand Down Expand Up @@ -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
}),
Expand All @@ -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
Expand Down Expand Up @@ -231,20 +228,20 @@ 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.
logger.error(RESTORE_STASH_EXAMPLE)
}
}
const error = new Error('lint-staged failed')
throw Object.assign(error, context)
throw Object.assign(error, { ctx })
}
}

Expand Down
8 changes: 8 additions & 0 deletions lib/state.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -68,6 +75,7 @@ const cleanupSkipped = (ctx) => {
}

module.exports = {
getInitialState,
hasPartiallyStagedFiles,
applyModificationsSkipped,
restoreUnstagedChangesSkipped,
Expand Down
6 changes: 5 additions & 1 deletion test/__snapshots__/index.spec.js.snap
Original file line number Diff line number Diff line change
@@ -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`] = `
"
Expand Down
6 changes: 4 additions & 2 deletions test/index2.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ describe('lintStaged', () => {
Object {
"ctx": Object {
"errors": Set {},
"hasPartiallyStagedFiles": false,
"hasPartiallyStagedFiles": null,
"output": Array [],
"shouldBackup": true,
},
"dateFormat": false,
Expand All @@ -49,7 +50,8 @@ describe('lintStaged', () => {
Object {
"ctx": Object {
"errors": Set {},
"hasPartiallyStagedFiles": false,
"hasPartiallyStagedFiles": null,
"output": Array [],
"shouldBackup": true,
},
"dateFormat": false,
Expand Down
51 changes: 35 additions & 16 deletions test/resolveTaskFn.spec.js
Original file line number Diff line number Diff line change
@@ -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()
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -88,7 +87,7 @@ describe('resolveTaskFn', () => {
gitDir: '../'
})

await taskFn(defaultCtx)
await taskFn()
expect(execa).toHaveBeenCalledTimes(1)
expect(execa).lastCalledWith('git', ['diff', 'test.js'], {
cwd: '../',
Expand All @@ -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,
Expand All @@ -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(),
Expand All @@ -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',
Expand All @@ -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)
Expand All @@ -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)
})
})
2 changes: 1 addition & 1 deletion test/resolveTaskFn.unmocked.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ describe('resolveTaskFn', () => {
shell: true
})

await expect(taskFn()).resolves.toMatchInlineSnapshot(`"√ node passed!"`)
await expect(taskFn()).resolves.toMatchInlineSnapshot(`undefined`)
})
})
Loading

0 comments on commit d69c65b

Please sign in to comment.