Skip to content

Commit

Permalink
fix: do not drop backup stash when reverting to original state fails
Browse files Browse the repository at this point in the history
  • Loading branch information
iiroj committed Feb 24, 2020
1 parent abe4b92 commit f589336
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 22 deletions.
1 change: 1 addition & 0 deletions lib/gitWorkflow.js
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ class GitWorkflow {
// Restore meta information about ongoing git merge
await this.restoreMergeStatus()
} catch (error) {
ctx.gitRestoreOriginalStateError = true
handleError(error, ctx)
}
}
Expand Down
72 changes: 51 additions & 21 deletions lib/runAll.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,40 @@ const getRenderer = ({ debug, quiet }) => {
return 'update'
}

const MESSAGES = {
TASK_ERROR: 'Skipped because of errors from tasks.',
GIT_ERROR: 'Skipped because of previous git error.'
}

const shouldSkipApplyModifications = ctx => {
// Should be skipped in case of git errors
if (ctx.gitError) {
return MESSAGES.GIT_ERROR
}
// Should be skipped when tasks fail
if (ctx.taskError) {
return MESSAGES.TASK_ERROR
}
}

const shouldSkipRevert = ctx => {
// Should be skipped in case of unknown git errors
if (ctx.gitError && !ctx.gitApplyEmptyCommit && !ctx.gitApplyModificationsError) {
return MESSAGES.GIT_ERROR
}
}

const shouldSkipCleanup = ctx => {
// Should be skipped in case of unknown git errors
if (ctx.gitError && !ctx.gitApplyEmptyCommit && !ctx.gitApplyModificationsError) {
return MESSAGES.GIT_ERROR
}
// Should be skipped when reverting to original state fails
if (ctx.gitRestoreOriginalStateError) {
return MESSAGES.GIT_ERROR
}
}

/**
* Executes all tasks and either resolves or rejects the promise
*
Expand All @@ -39,7 +73,7 @@ const getRenderer = ({ debug, quiet }) => {
* @param {Logger} logger
* @returns {Promise}
*/
module.exports = async function runAll(
const runAll = async (
{
allowEmpty = false,
config,
Expand All @@ -52,7 +86,7 @@ module.exports = async function runAll(
concurrent = true
},
logger = console
) {
) => {
debugLog('Running all linter scripts')

const { gitDir, gitConfigDir } = await resolveGitRepo(cwd)
Expand Down Expand Up @@ -127,7 +161,7 @@ module.exports = async function runAll(
task: () => new Listr(chunkListrTasks, { ...listrOptions, concurrent }),
skip: (ctx = {}) => {
// Skip if the first step (backup) failed
if (ctx.gitError) return 'Skipped because of previous git error.'
if (ctx.gitError) return MESSAGES.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 All @@ -151,15 +185,6 @@ module.exports = async function runAll(

const git = new GitWorkflow({ allowEmpty, gitConfigDir, gitDir, stagedFileChunks })

// Running git reset or dropping the backup stash should be skipped
// when there are git errors NOT related to applying unstaged modifications.
// In the latter case, the original state is restored.
const cleanupNotSafe = ctx =>
ctx.gitError &&
!ctx.gitApplyEmptyCommit &&
!ctx.gitApplyModificationsError &&
'Skipped because of previous git error.'

const runner = new Listr(
[
{
Expand All @@ -169,22 +194,19 @@ module.exports = async function runAll(
...listrTasks,
{
title: 'Applying modifications...',
skip: ctx => {
if (ctx.gitError) return 'Skipped because of previous git error.'
if (ctx.taskError) return 'Skipped because of errors from tasks.'
},
task: ctx => git.applyModifications(ctx)
task: ctx => git.applyModifications(ctx),
skip: shouldSkipApplyModifications
},
{
title: 'Reverting to original state...',
task: ctx => git.restoreOriginalState(ctx),
enabled: ctx => ctx.taskError || ctx.gitApplyEmptyCommit || ctx.gitApplyModificationsError,
skip: cleanupNotSafe,
task: ctx => git.restoreOriginalState(ctx)
skip: shouldSkipRevert
},
{
title: 'Cleaning up...',
skip: cleanupNotSafe,
task: ctx => git.dropBackup(ctx)
task: ctx => git.dropBackup(ctx),
skip: shouldSkipCleanup
}
],
listrOptions
Expand Down Expand Up @@ -219,3 +241,11 @@ module.exports = async function runAll(
throw error
}
}

module.exports = runAll

module.exports.shouldSkip = {
shouldSkipApplyModifications,
shouldSkipRevert,
shouldSkipCleanup
}
18 changes: 17 additions & 1 deletion test/runAll.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import path from 'path'

import resolveGitRepo from '../lib/resolveGitRepo'
import getStagedFiles from '../lib/getStagedFiles'
import runAll from '../lib/runAll'
import runAll, { shouldSkip } from '../lib/runAll'

jest.mock('../lib/resolveGitRepo')
jest.mock('../lib/getStagedFiles')
Expand Down Expand Up @@ -100,3 +100,19 @@ describe('runAll', () => {
expect(console.printHistory()).toMatchSnapshot()
})
})

describe('shouldSkip', () => {
describe('shouldSkipRevert', () => {
it('should return error message when there is an unkown git error', () => {
const result = shouldSkip.shouldSkipRevert({ gitError: true })
expect(typeof result === 'string').toEqual(true)
})
})

describe('shouldSkipCleanup', () => {
it('should return error message when reverting to original state fails', () => {
const result = shouldSkip.shouldSkipCleanup({ gitRestoreOriginalStateError: true })
expect(typeof result === 'string').toEqual(true)
})
})
})

0 comments on commit f589336

Please sign in to comment.