Skip to content

Commit

Permalink
refactor: Use internal execGit to list staged files (#624)
Browse files Browse the repository at this point in the history
  • Loading branch information
Iiro Jäppinen authored and okonet committed Jun 23, 2019
1 parent e332c80 commit 279863d
Show file tree
Hide file tree
Showing 9 changed files with 103 additions and 68 deletions.
2 changes: 1 addition & 1 deletion .lintstagedrc.json
Expand Up @@ -2,6 +2,6 @@
"linters": {
"*.{js,json,md}": ["prettier --write", "git add"],
"*.js": ["npm run lint:base --fix", "git add"],
".*{rc, json}": "jsonlint"
".*{rc, json}": ["jsonlint", "git add"]
}
}
3 changes: 1 addition & 2 deletions package.json
Expand Up @@ -43,9 +43,7 @@
"npm-which": "^3.0.1",
"p-map": "^1.1.1",
"path-is-inside": "^1.0.2",
"pify": "^3.0.0",
"please-upgrade-node": "^3.0.2",
"staged-git-files": "1.1.2",
"string-argv": "^0.0.2",
"stringify-object": "^3.2.2",
"yup": "^0.27.0"
Expand All @@ -61,6 +59,7 @@
"eslint": "^4.18.1",
"eslint-config-okonet": "^5.0.1",
"eslint-plugin-node": "^6.0.0",
"fs-extra": "^8.0.1",
"husky": "^1.1.4",
"jest": "^23.6.0",
"jest-snapshot-serializer-ansi": "^1.0.0",
Expand Down
12 changes: 12 additions & 0 deletions src/getStagedFiles.js
@@ -0,0 +1,12 @@
'use strict'

const execGit = require('./execGit')

module.exports = async function getStagedFiles(options) {
try {
const lines = await execGit(['diff', '--staged', '--diff-filter=ACM', '--name-only'], options)
return lines ? lines.split('\n') : []
} catch (error) {
return null
}
}
20 changes: 10 additions & 10 deletions src/runAll.js
@@ -1,13 +1,12 @@
'use strict'

const sgf = require('staged-git-files')
const generateTasks = require('./generateTasks')
const git = require('./gitWorkflow')
const getStagedFiles = require('./getStagedFiles')
const Listr = require('listr')
const has = require('lodash/has')
const pify = require('pify')
const makeCmdTasks = require('./makeCmdTasks')
const generateTasks = require('./generateTasks')
const resolveGitDir = require('./resolveGitDir')
const git = require('./gitWorkflow')

const debug = require('debug')('lint-staged:run')

Expand All @@ -32,14 +31,15 @@ module.exports = async function runAll(config) {

debug('Resolved git directory to be `%s`', gitDir)

sgf.cwd = gitDir
const files = await getStagedFiles({ cwd: gitDir })

if (!files) {
throw new Error('Unable to get staged files!')
}

/* files is an Object{ filename: String, status: String } */
const files = await pify(sgf)('ACM')
const filenames = files.map(file => file.filename)
debug('Loaded list of staged files in git:\n%O', filenames)
debug('Loaded list of staged files in git:\n%O', files)

const tasks = (await generateTasks(config, gitDir, filenames)).map(task => ({
const tasks = (await generateTasks(config, gitDir, files)).map(task => ({
title: `Running tasks for ${task.pattern}`,
task: async () =>
new Listr(
Expand Down
2 changes: 2 additions & 0 deletions test/__snapshots__/runAll.spec.js.snap
Expand Up @@ -29,6 +29,8 @@ LOG Running tasks for *.js [completed]
LOG Running linters... [completed]"
`;

exports[`runAll should reject promise when error during getStagedFiles 1`] = `"Unable to get staged files!"`;

exports[`runAll should resolve the promise with no files 1`] = `
"
LOG No staged files match any of provided globs."
Expand Down
26 changes: 26 additions & 0 deletions test/getStagedFiles.spec.js
@@ -0,0 +1,26 @@
import getStagedFiles from '../src/getStagedFiles'
import execGit from '../src/execGit'

jest.mock('../src/execGit')

describe('getStagedFiles', () => {
it('should return array of file names', async () => {
execGit.mockImplementationOnce(async () => 'foo.js\nbar.js')
const staged = await getStagedFiles()
expect(staged).toEqual(['foo.js', 'bar.js'])
})

it('should return empty array when no staged files', async () => {
execGit.mockImplementationOnce(async () => '')
const staged = await getStagedFiles()
expect(staged).toEqual([])
})

it('should return null in case of error', async () => {
execGit.mockImplementationOnce(async () => {
throw new Error('fatal: not a git repository (or any of the parent directories): .git')
})
const staged = await getStagedFiles()
expect(staged).toEqual(null)
})
})
33 changes: 16 additions & 17 deletions test/gitStash.spec.js
Expand Up @@ -2,8 +2,7 @@ const execa = require('execa')
const path = require('path')
const tmp = require('tmp')
const gitflow = require('../src/gitWorkflow')
const pify = require('pify')
const fsp = pify(require('fs'))
const fs = require('fs-extra')

tmp.setGracefulCleanup()
jest.unmock('execa')
Expand All @@ -21,7 +20,7 @@ async function gitStatus(opts = gitOpts) {
}

async function readFile(filepath, dir = wcDirPath) {
const content = await fsp.readFile(path.join(dir, filepath), { encoding: 'utf-8' })
const content = await fs.readFile(path.join(dir, filepath), { encoding: 'utf-8' })
return content.replace(/\r\n/g, '\n')
}

Expand All @@ -36,7 +35,7 @@ describe('git', () => {
// Init repository
await gitflow.execGit('init', gitOpts)
// Create JS file
await fsp.writeFile(
await fs.writeFile(
path.join(wcDirPath, 'test.js'),
`module.exports = {
test: 'test',
Expand All @@ -45,7 +44,7 @@ describe('git', () => {
}
`
)
await fsp.writeFile(
await fs.writeFile(
path.join(wcDirPath, 'test.css'),
`.test {
border: 1px solid green;
Expand All @@ -59,9 +58,9 @@ describe('git', () => {
// Create inital commit
await gitflow.execGit(['commit', '-m', '"Initial commit"'], gitOpts)
// Update one of the files
await fsp.writeFile(path.join(wcDirPath, 'test.css'), '.test { border: red; }')
await fs.writeFile(path.join(wcDirPath, 'test.css'), '.test { border: red; }')
// Update one of the files
await fsp.writeFile(path.join(wcDirPath, 'test.js'), initialContent)
await fs.writeFile(path.join(wcDirPath, 'test.js'), initialContent)
})

afterEach(() => {
Expand Down Expand Up @@ -97,7 +96,7 @@ describe('git', () => {
it('should return true if files are modified and in the index', async () => {
await gitflow.execGit(['checkout', 'test.css'], gitOpts)
await gitflow.execGit(['add', 'test.js'], gitOpts)
await fsp.writeFile(path.join(wcDirPath, 'test.js'), '')
await fs.writeFile(path.join(wcDirPath, 'test.js'), '')
const res = await gitflow.hasPartiallyStagedFiles(gitOpts)
expect(res).toEqual(true)
})
Expand Down Expand Up @@ -226,7 +225,7 @@ M test.js"
test: 'test2',
test: 'test2',
};`
await fsp.writeFile(path.join(wcDirPath, 'test.js'), eslintContent)
await fs.writeFile(path.join(wcDirPath, 'test.js'), eslintContent)

// Expect both indexed and modified state on one file
expect(await gitStatus()).toMatchInlineSnapshot(`"MM test.js"`)
Expand Down Expand Up @@ -262,7 +261,7 @@ M test.js"
test: 'test2',
test: 'test3',
}`
await fsp.writeFile(path.join(wcDirPath, 'test.js'), userContent)
await fs.writeFile(path.join(wcDirPath, 'test.js'), userContent)

// Expect test.js is in both index and modified
expect(await gitStatus()).toMatchInlineSnapshot(`
Expand All @@ -278,7 +277,7 @@ MM test.js"
expect(await gitStatus()).toMatchInlineSnapshot(`"M test.js"`)

// Do additional edits (imitate eslint --fix)
await fsp.writeFile(
await fs.writeFile(
path.join(wcDirPath, 'test.js'),
`module.exports = {
test: 'test2',
Expand Down Expand Up @@ -330,7 +329,7 @@ M test.js"
const newContent = `module.exports = {
test: "test2",
};`
await fsp.writeFile(path.join(wcDirPath, 'test.js'), newContent)
await fs.writeFile(path.join(wcDirPath, 'test.js'), newContent)
// and add to index
await gitflow.execGit(['add', 'test.js'], gitOpts)
await gitflow.updateStash(gitOpts)
Expand Down Expand Up @@ -372,7 +371,7 @@ M test.js"
test: 'test2',
test: 'test3'
}`
await fsp.writeFile(path.join(wcDirPath, 'test.js'), userContent)
await fs.writeFile(path.join(wcDirPath, 'test.js'), userContent)

// Expect test.js is in both index and modified
expect(await gitStatus()).toMatchInlineSnapshot(`
Expand All @@ -389,7 +388,7 @@ MM test.js"
expect(await gitflow.execGit(['diff', '--cached'], gitOpts)).toEqual(initialIndex)

// Do additional edits (imitate eslint --fix)
await fsp.writeFile(
await fs.writeFile(
path.join(wcDirPath, 'test.js'),
`module.exports = {
test: "test2"
Expand Down Expand Up @@ -423,7 +422,7 @@ MM test.js"
await gitflow.execGit(['checkout', '--', '.'], gitOpts)

// Do additional edits and stage them
await fsp.writeFile(
await fs.writeFile(
path.join(wcDirPath, 'test.js'),
`module.exports = {
test: 'test',
Expand All @@ -441,7 +440,7 @@ MM test.js"
await gitflow.execGit(['add', 'test.js'], gitOpts)

// Do additional edits without adding to index
await fsp.writeFile(
await fs.writeFile(
path.join(wcDirPath, 'test.js'),
`module.exports = {
test: 'edited',
Expand Down Expand Up @@ -469,7 +468,7 @@ MM test.js"
expect(await gitflow.execGit(['diff', '--cached'], gitOpts)).toEqual(initialIndex)

// Imitate running prettier on the version from the index
await fsp.writeFile(
await fs.writeFile(
path.join(wcDirPath, 'test.js'),
`module.exports = {
test: "test",
Expand Down
47 changes: 14 additions & 33 deletions test/runAll.spec.js
@@ -1,16 +1,16 @@
import makeConsoleMock from 'consolemock'
import sgfMock from 'staged-git-files'
import execa from 'execa'

import { getConfig } from '../src/getConfig'
import getStagedFiles from '../src/getStagedFiles'
import runAll from '../src/runAll'
import { hasPartiallyStagedFiles, gitStashSave, gitStashPop, updateStash } from '../src/gitWorkflow'

jest.mock('staged-git-files')
jest.mock('../src/getStagedFiles')
jest.mock('../src/gitWorkflow')

sgfMock.mockImplementation((params, callback) => {
callback(null, [])
})
getStagedFiles.mockImplementation(async () => [])

const globalConsoleTemp = global.console

describe('runAll', () => {
Expand Down Expand Up @@ -60,19 +60,15 @@ describe('runAll', () => {

it('should not skip tasks if there are files', async () => {
expect.assertions(1)
sgfMock.mockImplementationOnce((params, callback) => {
callback(null, [{ filename: 'sample.js', status: 'sample' }])
})
getStagedFiles.mockImplementationOnce(async () => ['sample.js'])
await runAll(getConfig({ linters: { '*.js': ['echo "sample"'] } }))
expect(console.printHistory()).toMatchSnapshot()
})

it('should not skip stashing and restoring if there are partially staged files', async () => {
expect.assertions(4)
hasPartiallyStagedFiles.mockImplementationOnce(() => Promise.resolve(true))
sgfMock.mockImplementationOnce((params, callback) => {
callback(null, [{ filename: 'sample.js', status: 'Modified' }])
})
getStagedFiles.mockImplementationOnce(async () => ['sample.js'])
await runAll(getConfig({ linters: { '*.js': ['echo "sample"'] } }))
expect(gitStashSave).toHaveBeenCalledTimes(1)
expect(updateStash).toHaveBeenCalledTimes(1)
Expand All @@ -83,9 +79,7 @@ describe('runAll', () => {
it('should skip stashing and restoring if there are no partially staged files', async () => {
expect.assertions(4)
hasPartiallyStagedFiles.mockImplementationOnce(() => Promise.resolve(false))
sgfMock.mockImplementationOnce((params, callback) => {
callback(null, [{ filename: 'sample.js', status: 'Modified' }])
})
getStagedFiles.mockImplementationOnce(async () => ['sample.js'])
await runAll(getConfig({ linters: { '*.js': ['echo "sample"'] } }))
expect(gitStashSave).toHaveBeenCalledTimes(0)
expect(updateStash).toHaveBeenCalledTimes(0)
Expand All @@ -96,9 +90,7 @@ describe('runAll', () => {
it('should skip updating stash if there are errors during linting', async () => {
expect.assertions(4)
hasPartiallyStagedFiles.mockImplementationOnce(() => Promise.resolve(true))
sgfMock.mockImplementationOnce((params, callback) => {
callback(null, [{ filename: 'sample.js', status: 'Modified' }])
})
getStagedFiles.mockImplementationOnce(async () => ['sample.js'])
execa.mockImplementation(() =>
Promise.resolve({
stdout: '',
Expand All @@ -123,9 +115,7 @@ describe('runAll', () => {
it('should skip linters and stash update but perform working copy restore if terminated', async () => {
expect.assertions(4)
hasPartiallyStagedFiles.mockImplementationOnce(() => Promise.resolve(true))
sgfMock.mockImplementationOnce((params, callback) => {
callback(null, [{ filename: 'sample.js', status: 'Modified' }])
})
getStagedFiles.mockImplementationOnce(async () => ['sample.js'])
execa.mockImplementation(() =>
Promise.resolve({
stdout: '',
Expand All @@ -149,25 +139,16 @@ describe('runAll', () => {
expect(gitStashPop).toHaveBeenCalledTimes(1)
})

it('should reject promise when staged-git-files errors', async () => {
it('should reject promise when error during getStagedFiles', async () => {
expect.assertions(1)
sgfMock.mockImplementationOnce((params, callback) => {
callback('test', undefined)
})

try {
await runAll(getConfig({}))
} catch (err) {
expect(err).toEqual('test')
}
getStagedFiles.mockImplementationOnce(async () => null)
await expect(runAll(getConfig({}))).rejects.toThrowErrorMatchingSnapshot()
})

it('should skip stashing changes if no lint-staged files are changed', async () => {
expect.assertions(4)
hasPartiallyStagedFiles.mockImplementationOnce(() => Promise.resolve(true))
sgfMock.mockImplementationOnce((params, callback) => {
callback(null, [{ filename: 'sample.java', status: 'Modified' }])
})
getStagedFiles.mockImplementationOnce(async () => ['sample.java'])
execa.mockImplementationOnce(() =>
Promise.resolve({
stdout: '',
Expand Down

0 comments on commit 279863d

Please sign in to comment.