fix: Only pass gitDir for git specific executables (#162)

* change npm to git since gitDir is only used for git executables

* fix(gitDir): `gitDir` should only be applied when using `git` binary

Inverses logic of checking for "not equal to `npm`", instead check
that binary to run is equal to `git` (and accounting for absolute
paths). This means that commands besides `npm` will also run in CWD
instead of `gitDir`.

Closes #158

src/runScript.js

@@ -10,8 +10,10 @@ module.exports = function runScript(commands, pathsToLint, packageJson, options)
         task: () => {
             try {
                 const res = findBin(linter, pathsToLint, packageJson, options)
+                // Only use gitDir as CWD if we are using the git binary
+                // e.g `npm` should run tasks in the actual CWD
                 const execaOptions =
-                    res.bin !== 'npm' && options && options.gitDir ? { cwd: options.gitDir } : {}
+                    res.bin.endsWith('git') && options && options.gitDir ? { cwd: options.gitDir } : {}
                 return new Promise((resolve, reject) => {
                     execa(res.bin, res.args, execaOptions)
                         .then(() => {
@@ -29,4 +31,3 @@ ${ err.stdout }`))
         }
     }))
 }
-

test/__mocks__/execa.js

@@ -1,3 +1 @@
-jest.genMockFromModule('execa')
-
 module.exports = jest.fn()

test/runScript-mock-findBin.spec.js

@@ -0,0 +1,65 @@
+// This is in a separate test file because I was unable to get `jest.mock` working in a test block
+// `jest.mock` gets hoisted, but even with `jest.doMock` it wasn't working
+/* eslint no-underscore-dangle: 0 */
+
+import expect from 'expect'
+import isPromise from 'is-promise'
+import mockFn from 'execa'
+import runScript from '../src/runScript'
+
+jest.mock('execa')
+
+// Mock findBin to return an absolute path
+jest.mock('../src/findBin', () => (commands, paths) => {
+    const [
+      bin,
+      ...otherArgs
+    ] = commands.split(' ')
+
+    return ({
+        bin: `/usr/local/bin/${ bin }`,
+        args: otherArgs.concat(['--']).concat(paths)
+    })
+}, { virtual: true })
+
+expect.extend({
+    toBeAPromise() {
+        expect.assert(
+            isPromise(this.actual),
+            'expected %s to be a Promise',
+            this.actual
+        )
+        return this
+    }
+})
+
+const packageJSON = {
+    scripts: {
+        test: 'noop',
+        test2: 'noop'
+    },
+    'lint-staged': {}
+}
+
+describe.only('runScript with absolute paths', () => {
+    beforeEach(() => {
+        mockFn.mockReset()
+        mockFn.mockImplementation(() => new Promise(() => {}))
+    })
+
+    it('can pass `gitDir` as `cwd` to `execa()` when git is called via absolute path', () => {
+        const res = runScript(
+          ['git add'],
+          'test.js',
+          packageJSON,
+          { gitDir: '../' }
+        )
+
+        expect(res[0].task()).toBeAPromise()
+        expect(mockFn.mock.calls.length).toEqual(1)
+        expect(mockFn.mock.calls[0][0]).toMatch('/usr/local/bin/git')
+        expect(mockFn.mock.calls[0][1]).toEqual(['add', '--', 'test.js'])
+        expect(mockFn.mock.calls[0][2]).toEqual({ cwd: '../' })
+    })
+})
+

test/runScript.spec.js

@@ -30,6 +30,7 @@ describe('runScript', () => {
 
     beforeEach(() => {
         mockFn.mockReset()
+        mockFn.mockImplementation(() => new Promise(() => {}))
     })
 
     it('should return an array', () => {
@@ -105,6 +106,20 @@ describe('runScript', () => {
         expect(mockFn.mock.calls[1][2]).toEqual({ cwd: '../' })
     })
 
+    it('should not pass `gitDir` as `cwd` to `execa()` if a non-git binary is called', () => {
+        const res = runScript(
+          ['jest'],
+          'test.js',
+          packageJSON,
+          { gitDir: '../' }
+        )
+        expect(res[0].task()).toBeAPromise()
+        expect(mockFn.mock.calls.length).toEqual(1)
+        expect(mockFn.mock.calls[0]).toEqual(
+          ['jest', ['--', 'test.js'], {}]
+        )
+    })
+
     it('should use --silent in non-verbose mode', () => {
         const res = runScript(
             'test',
@@ -133,4 +148,3 @@ describe('runScript', () => {
         )
     })
 })
-