quoting default pathToJest (#523)

CHANGELOG.md

@@ -15,6 +15,7 @@ Bug-fixes within the same version aren't needed
 * Fix remove ANSI characters from test messages - [@jmarceli](https://github.com/jmarceli)
 * Adjust test result indicators for colorblind people - [@jmarceli](https://github.com/jmarceli)
 * Use short message instead of terse message in test diagnostic tooltip and tab - [@jmarceli](https://github.com/jmarceli)
+* quoting default "pathToJest" to preserve special characters, if any. - @connectdotz
 * Fix for when `branch.end.column` is `null`. [@garyking](https://github.com/garyking)
 
 -->

src/JestProcessManagement/JestProcess.ts

@@ -1,4 +1,3 @@
-import { platform } from 'os'
 import { Runner, ProjectWorkspace } from 'jest-editor-support'
 import { WatchMode } from '../Jest'
 import { ExitCallback } from './JestProcessManager'
@@ -70,7 +69,6 @@ export class JestProcess {
 
     const options = {
       noColor: true,
-      shell: platform() === 'win32',
     }
     this.runner = new Runner(this.projectWorkspace, options)
 

src/helpers.ts

@@ -79,7 +79,8 @@ export function pathToJest({ pathToJest, rootPath }: IPluginResourceSettings) {
     return 'npm test --'
   }
 
-  return getLocalPathForExecutable(rootPath, 'jest') || 'jest' + nodeBinExtension
+  const p = getLocalPathForExecutable(rootPath, 'jest') || 'jest' + nodeBinExtension
+  return `"${p}"`
 }
 
 /**
@@ -95,34 +96,6 @@ export function pathToConfig(pluginSettings: IPluginResourceSettings) {
   return ''
 }
 
-export function pathToJestPackageJSON(pluginSettings: IPluginResourceSettings): string | null {
-  let pathToNodeModules = join(pluginSettings.rootPath, 'node_modules')
-
-  if (pluginSettings.pathToJest) {
-    const relativeJestCmd = removeSurroundingQuotes(pluginSettings.pathToJest.split(' ')[0])
-    const relativePathToNodeModules = relativeJestCmd.replace(/node_modules.+$/i, 'node_modules')
-
-    pathToNodeModules = join(pluginSettings.rootPath, relativePathToNodeModules)
-  }
-
-  const defaultPath = normalize(join(pathToNodeModules, 'jest/package.json'))
-  const cliPath = normalize(join(pathToNodeModules, 'jest-cli/package.json'))
-  const craPath = normalize(join(pathToNodeModules, 'react-scripts/node_modules/jest/package.json'))
-  const paths = [defaultPath, cliPath, craPath]
-
-  for (const i in paths) {
-    if (existsSync(paths[i])) {
-      return paths[i]
-    }
-  }
-
-  return null
-}
-
-function removeSurroundingQuotes(str: string) {
-  return str.replace(/^['"`]/, '').replace(/['"`]$/, '')
-}
-
 /**
  *  Taken From https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions
  */

tests/Coverage/CoverageCodeLensProvider.test.ts

@@ -0,0 +1,87 @@
+jest.unmock('../../src/Coverage/CoverageCodeLensProvider')
+
+// tslint:disable max-classes-per-file
+const rangeConstructor = jest.fn()
+jest.mock('vscode', () => {
+  class CodeLens {
+    range: any
+    command: any
+
+    constructor(range, command) {
+      this.range = range
+      this.command = command
+    }
+  }
+
+  // class EventEmitter {
+  //   fire() {}
+  // }
+
+  class Position {
+    lineNumber: string
+    character: string
+
+    constructor(lineNumber, character) {
+      this.lineNumber = lineNumber
+      this.character = character
+    }
+  }
+
+  class Range {
+    start: Position
+    end: Position
+
+    constructor(start, end) {
+      rangeConstructor(...arguments)
+      this.start = start
+      this.end = end
+    }
+  }
+
+  return {
+    CodeLens,
+    Position,
+    Range,
+  }
+})
+
+import { CoverageCodeLensProvider } from '../../src/Coverage/CoverageCodeLensProvider'
+
+// import * as vscode from 'vscode'
+
+describe('CoverageCodeLensProvider', () => {
+  let mockJestExt
+  let provider
+
+  beforeEach(() => {
+    mockJestExt = {
+      coverageMapProvider: { getFileCoverage: jest.fn() },
+    }
+    const mockGetExt = jest.fn().mockReturnValue(mockJestExt)
+    provider = new CoverageCodeLensProvider(mockGetExt)
+  })
+  describe('provideCodeLenses', () => {
+    const doc = { fileName: 'file.js' }
+
+    test('do nothing when no coverage', () => {
+      mockJestExt.coverageMapProvider.getFileCoverage = () => null
+      const result = provider.provideCodeLenses(doc)
+      expect(result).toBeUndefined()
+    })
+
+    test('can summarize', () => {
+      const coverage = {
+        toSummary: () => ({
+          toJSON: () => ({
+            branches: { pct: 10 },
+            lines: { pct: 46.15 },
+          }),
+        }),
+      }
+      mockJestExt.coverageMapProvider.getFileCoverage = () => coverage
+      const result = provider.provideCodeLenses(doc)
+      expect(result).toHaveLength(1)
+      expect(result[0].command.title).toEqual('branches: 10%, lines: 46.15%')
+    })
+  })
+})

tests/helpers.test.ts

@@ -18,8 +18,7 @@ jest.mock('path', () => ({
   normalize: mockNormalize,
 }))
 
-import { isCreateReactAppTestCommand, pathToJest, pathToJestPackageJSON, nodeBinExtension, cleanAnsi } from '../src/helpers'
-import * as path from 'path'
+import { isCreateReactAppTestCommand, pathToJest, nodeBinExtension, cleanAnsi } from '../src/helpers'
 
 // Manually (forcefully) set the executable's file extension to test its addition independendly of the operating system.
 ;(nodeBinExtension as string) = '.TEST'
@@ -40,113 +39,6 @@ describe('ModuleHelpers', () => {
     })
   })
 
-  describe('pathToJestPackageJSON', () => {
-    const defaultPathToJest = null
-    const defaultSettings: any = {
-      pathToJest: defaultPathToJest,
-      rootPath: '',
-    }
-
-    beforeEach(() => {
-      mockJoin.mockImplementation(require.requireActual('path').join)
-      mockNormalize.mockImplementation(require.requireActual('path').normalize)
-    })
-
-    it('should return null when not found', () => {
-      mockExistsSync.mockReturnValue(false)
-
-      expect(pathToJestPackageJSON(defaultSettings)).toBe(null)
-    })
-
-    describe('pathToJest: <default>', () => {
-      // tslint:disable no-shadowed-variable
-      describe('rootPath: <default>', () => {
-        it('should return package.json when Jest is installed as a dependency', () => {
-          const expected = path.join('node_modules', 'jest', 'package.json')
-          mockExistsSync.mockImplementation(path => path === expected)
-
-          expect(pathToJestPackageJSON(defaultSettings)).toBe(expected)
-        })
-
-        it('should return package.json when React Scripts are installed as a dependency', () => {
-          const expected = path.join('node_modules', 'react-scripts', 'node_modules', 'jest', 'package.json')
-          mockExistsSync.mockImplementation(path => path === expected)
-
-          expect(pathToJestPackageJSON(defaultSettings)).toBe(expected)
-        })
-      })
-
-      describe('rootPath: <string>', () => {
-        it('should return package.json when Jest is installed as a dependency', () => {
-          const expected = path.join('..', '..', 'node_modules', 'jest', 'package.json')
-          mockExistsSync.mockImplementation(path => path === expected)
-
-          const workspace: any = {
-            pathToJest: defaultPathToJest,
-            rootPath: path.join('..', '..'),
-          }
-          expect(pathToJestPackageJSON(workspace)).toBe(expected)
-        })
-
-        it('should return package.json when Jest-cli is installed as a dependency', () => {
-          const expected = path.join('..', '..', 'node_modules', 'jest-cli', 'package.json')
-          mockExistsSync.mockImplementation(path => path === expected)
-
-          const workspace: any = {
-            rootPath: path.join('..', '..'),
-            pathToJest: defaultPathToJest,
-          }
-          expect(pathToJestPackageJSON(workspace)).toBe(expected)
-        })
-
-        it('should return package.json when React Scripts are installed as a dependency', () => {
-          const expected = path.join(
-            '..',
-            '..',
-            'node_modules',
-            'react-scripts',
-            'node_modules',
-            'jest',
-            'package.json'
-          )
-          mockExistsSync.mockImplementation(path => path === expected)
-
-          const workspace: any = {
-            rootPath: path.join('..', '..'),
-            pathToJest: '',
-          }
-          expect(pathToJestPackageJSON(workspace)).toBe(expected)
-        })
-      })
-    })
-
-    describe('pathToJest: npm test --', () => {
-      it('will not find package.json', () => {
-        const expected = null
-        mockExistsSync.mockImplementation(path => path === expected)
-
-        const workspace: any = {
-          rootPath: '',
-          pathToJest: 'npm test --',
-        }
-        expect(pathToJestPackageJSON(workspace)).toBe(null)
-      })
-    })
-
-    describe(`pathToJest: ${path.join('"test scripts"', 'test')}`, () => {
-      it('will not find package.json', () => {
-        const expected = path.join('"test scripts"', 'test')
-        mockExistsSync.mockImplementation(path => path === expected)
-
-        const workspace: any = {
-          rootPath: '',
-          pathToJest: expected,
-        }
-        expect(pathToJestPackageJSON(workspace)).toBe(null)
-      })
-    })
-  })
-
   describe('isCreateReactAppTestCommand', () => {
     it('should return true for CRA', () => {
       expect(isCreateReactAppTestCommand('react-scripts test --env=jsdom')).toBe(true)
@@ -193,19 +85,35 @@ describe('ModuleHelpers', () => {
       expect(pathToJest(settings)).toBe(expected)
       expect(mockNormalize).toBeCalledWith(settings.pathToJest)
     })
-
     it('defaults to "node_modules/.bin/jest" when Jest is locally installed', () => {
       const expected = 'node_modules/.bin/jest.TEST'
 
       mockJoin.mockImplementation(require.requireActual('path').posix.join)
       mockNormalize.mockImplementation(arg => arg)
       mockExistsSync.mockImplementation(path => path === expected)
 
-      expect(pathToJest(defaultSettings)).toBe(expected)
+      expect(pathToJest(defaultSettings)).toBe(`"${expected}"`)
     })
+    it('default jestToPath path can preserve special characters', () => {
+      mockJoin.mockImplementation(require.requireActual('path').posix.join)
+      mockNormalize.mockImplementation(arg => arg)
 
+      const testPaths = [
+        '/root/my dir/space',
+        '/root/my dir/escape-space',
+        '/root/👍/emoji',
+        '/root/外國人/unicode',
+        '/root/\\space/double-escape',
+      ]
+      testPaths.forEach(p => {
+        const settings = { ...defaultSettings, rootPath: p }
+        const expected = `${p}/node_modules/.bin/jest.TEST`
+        mockExistsSync.mockImplementation(path => path === expected)
+        expect(pathToJest(settings)).toBe(`"${expected}"`)
+      })
+    })
     it('defaults to "jest" when Jest is not locally installed', () => {
-      const expected = 'jest.TEST'
+      const expected = '"jest.TEST"'
 
       mockJoin.mockImplementation(require.requireActual('path').posix.join)
       mockNormalize.mockImplementation(arg => arg)