Repository: infinitered/solidarity
Pull request #179 (2018-02-27 01:26:23)

:rage3: windows working and fight node modules
#### Node will inject its own bin to the front of `$PATH`, as identified in #175 

This simply moves that injection to the back to prefer global over anything `node_modules/`.

Tests included.  I attempted to go a wrong way first, and that left strange artifacts I had to clean up.  Squash to kill the noise

-----------------
Closes: #175 

codecov[bot]: # [Codecov](https://codecov.io/gh/infinitered/solidarity/pull/179?src=pr&el=h1) Report
> Merging [#179](https://codecov.io/gh/infinitered/solidarity/pull/179?src=pr&el=desc) into [master](https://codecov.io/gh/infinitered/solidarity/commit/e0c765277e8e82ef7cd8cc7e2f262cfdb488d030?src=pr&el=desc) will **decrease** coverage by `0.15%`.
> The diff coverage is `66.66%`.

[![Impacted file tree graph](https://codecov.io/gh/infinitered/solidarity/pull/179/graphs/tree.svg?height=150&src=pr&token=eTjcyCooFB&width=650)](https://codecov.io/gh/infinitered/solidarity/pull/179?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #179      +/-   ##
==========================================
- Coverage   82.87%   82.71%   -0.16%     
==========================================
  Files          32       33       +1     
  Lines         613      619       +6     
  Branches      103      105       +2     
==========================================
+ Hits          508      512       +4     
- Misses         69       70       +1     
- Partials       36       37       +1
```


| [Impacted Files](https://codecov.io/gh/infinitered/solidarity/pull/179?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/extensions/functions/checkCLI.ts](https://codecov.io/gh/infinitered/solidarity/pull/179/diff?src=pr&el=tree#diff-c3JjL2V4dGVuc2lvbnMvZnVuY3Rpb25zL2NoZWNrQ0xJLnRz) | `92.85% <100%> (+8.24%)` | :arrow_up: |
| [src/extensions/functions/quirksNodeModules.ts](https://codecov.io/gh/infinitered/solidarity/pull/179/diff?src=pr&el=tree#diff-c3JjL2V4dGVuc2lvbnMvZnVuY3Rpb25zL3F1aXJrc05vZGVNb2R1bGVzLnRz) | `60% <60%> (ø)` | |
| [src/extensions/functions/solidarityReport.ts](https://codecov.io/gh/infinitered/solidarity/pull/179/diff?src=pr&el=tree#diff-c3JjL2V4dGVuc2lvbnMvZnVuY3Rpb25zL3NvbGlkYXJpdHlSZXBvcnQudHM=) | `88.88% <0%> (-11.12%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/infinitered/solidarity/pull/179?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/infinitered/solidarity/pull/179?src=pr&el=footer). Last update [e0c7652...18b3c81](https://codecov.io/gh/infinitered/solidarity/pull/179?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/.vscode/cSpell.json b/.vscode/cSpell.json
index dab156f..025b4e9 100644
--- a/.vscode/cSpell.json
+++ b/.vscode/cSpell.json
@@ -18,6 +18,7 @@
         "config",
         "configs",
         "custom",
+        "delineator",
         "docsify",
         "e.g.",
         "env",
@@ -46,6 +47,7 @@
         "specs",
         "tada",
         "tempy",
+        "typesync",
         "updtr",
         "v1",
         "walkthrough"
diff --git a/src/extensions/functions/checkCLI.ts b/src/extensions/functions/checkCLI.ts
index 83a3f30..80f7007 100644
--- a/src/extensions/functions/checkCLI.ts
+++ b/src/extensions/functions/checkCLI.ts
@@ -2,6 +2,8 @@ import { SolidarityRunContext, CLIRule } from '../../types'
 module.exports = async (rule: CLIRule, context: SolidarityRunContext): Promise<string | undefined> => {
   const { semver, solidarity } = context
   const binaryExists = require('./binaryExists')
+  // Node Modules do strange things
+  require('./quirksNodeModules')
 
   // First check for binary
   if (!binaryExists(rule.binary, context)) {
diff --git a/src/extensions/functions/checkCLIForUpdates.ts b/src/extensions/functions/checkCLIForUpdates.ts
index ef3034f..c1c4599 100644
--- a/src/extensions/functions/checkCLIForUpdates.ts
+++ b/src/extensions/functions/checkCLIForUpdates.ts
@@ -23,6 +23,7 @@ module.exports = async (rule: CLIRule, context: SolidarityRunContext): Promise<s
   // if it doesn't satisfy, upgrade
   if (rule.semver && !semver.satisfies(binarySemantic, rule.semver)) {
     rule.semver = binaryVersion
-    return print.colors.green(`Setting ${rule.binary} to '${binaryVersion}'`)
+    const lineMessage = rule.line ? ` line ${rule.line}` : ''
+    return print.colors.green(`Setting ${rule.binary}${lineMessage} to '${binaryVersion}'`)
   }
 }
diff --git a/src/extensions/functions/quirksNodeModules.ts b/src/extensions/functions/quirksNodeModules.ts
new file mode 100644
index 0000000..e409641
--- /dev/null
+++ b/src/extensions/functions/quirksNodeModules.ts
@@ -0,0 +1,9 @@
+import { reject, contains, concat, difference } from 'ramda'
+const path = require('path')
+const delineator = process.platform === 'win32' ? ';' : ':'
+
+// Node mutates path by adding to the front, move that to the back if it exists
+const originalPath = process.env.PATH || ''
+const originalArray = originalPath.split(delineator)
+const cleanArray = reject(contains('node_modules' + path.sep), originalArray)
+process.env.PATH = concat(cleanArray, difference(originalArray, cleanArray)).join(delineator)
diff --git a/src/extensions/functions/updateRequirement.ts b/src/extensions/functions/updateRequirement.ts
index 0abf550..ac11efe 100644
--- a/src/extensions/functions/updateRequirement.ts
+++ b/src/extensions/functions/updateRequirement.ts
@@ -26,7 +26,8 @@ module.exports = async (
       case 'cli':
         if (!rule.semver) return []
         const updateResult = await checkCLIForUpdates(rule, context)
-        ruleString = `Keep ${rule.binary} ${rule.semver}`
+        const lineMessage = rule.line ? ` line ${rule.line} at` : ''
+        ruleString = `Keep ${rule.binary}${lineMessage} ${rule.semver}`
         if (updateResult) {
           spinner.succeed(updateResult)
           return updateResult


Test patch:
diff --git a/__tests__/__mocks__/mockContext.ts b/__tests__/__mocks__/mockContext.ts
index 03b527d..d7478e8 100644
--- a/__tests__/__mocks__/mockContext.ts
+++ b/__tests__/__mocks__/mockContext.ts
@@ -9,6 +9,8 @@ const noConfigSolidarity = {
   setSolidaritySettings: jest.fn(),
   updateRequirement: jest.fn(),
   updateVersions: jest.fn(() => Promise.resolve()),
+  getLineWithVersion: jest.fn(),
+  removeNonVersionCharacters: jest.fn(),
 }
 
 const mockContext = {
@@ -16,6 +18,8 @@ const mockContext = {
   outputMode: undefined,
   system: {
     startTimer: jest.fn(() => jest.fn()),
+    run: jest.fn(() => '12'),
+    which: jest.fn(name => 'usr/local/bin/${name}'),
   },
   template: {
     generate: jest.fn(),
diff --git a/__tests__/command_helpers/checkCLI.ts b/__tests__/command_helpers/checkCLI.ts
index 9f4cb8c..d8d7c40 100644
--- a/__tests__/command_helpers/checkCLI.ts
+++ b/__tests__/command_helpers/checkCLI.ts
@@ -9,6 +9,11 @@ const alwaysExistCLI = {
   binary: 'node',
 }
 
+const badSemver = {
+  binary: 'node',
+  semver: 'wtfbbq!!!11',
+}
+
 const outOfDateCLI = {
   binary: 'node',
   semver: '10.99',
@@ -24,6 +29,10 @@ test('fine on existing binary', async () => {
   expect(await checkCLI(alwaysExistCLI, context)).toBe(undefined)
 })
 
+test('errors with message when an improper semver is sent', async () => {
+  expect(await checkCLI(badSemver, context)).toBe(`Invalid semver rule ${badSemver.semver}`)
+})
+
 test('returns message on improper version', async () => {
   solidarityExtension(context)
   context.solidarity.getVersion = () => '1'
diff --git a/__tests__/command_helpers/getVersion.ts b/__tests__/command_helpers/getVersion.ts
index c06a176..94e758d 100644
--- a/__tests__/command_helpers/getVersion.ts
+++ b/__tests__/command_helpers/getVersion.ts
@@ -13,7 +13,7 @@ describe('getVersion', () => {
     jasmine.DEFAULT_TIMEOUT_INTERVAL = 60000
   })
 
-  afterAll(function () {
+  afterAll(function() {
     // Fix timeout change
     jasmine.DEFAULT_TIMEOUT_INTERVAL = originalTimeout
   })
diff --git a/__tests__/command_helpers/quirksNodeModules.ts b/__tests__/command_helpers/quirksNodeModules.ts
new file mode 100644
index 0000000..582b834
--- /dev/null
+++ b/__tests__/command_helpers/quirksNodeModules.ts
@@ -0,0 +1,32 @@
+const path = require('path')
+const delineator = process.platform === 'win32' ? ';' : ':'
+test('quirks moves node_modules to back', () => {
+  // key is that it has `node_modules` + path.sep
+  const injectedStuff = `node_modules${path.sep}testOnly`
+  // prepend to PATH
+  process.env.PATH = injectedStuff + delineator + process.env.PATH
+  const pathAsArray = process.env.PATH.split(delineator)
+  // Yup it is prepended to front
+  expect(pathAsArray[0]).toBe(injectedStuff)
+  // require mutates PATH
+  require('../../src/extensions/functions/quirksNodeModules')
+  const newPathAsArray = process.env.PATH.split(delineator)
+  // Not in the front
+  expect(newPathAsArray[0]).not.toBe(injectedStuff)
+  // still there though (moved to back)
+  expect(process.env.PATH.includes(injectedStuff)).toBeTruthy()
+})
+
+test('quirks does not move just any injected path to back', () => {
+  const injectedStuff = `taco${path.sep}testOnly`
+  // prepend to PATH
+  process.env.PATH = injectedStuff + delineator + process.env.PATH
+  const pathAsArray = process.env.PATH.split(delineator)
+  // Yup it is prepended to front
+  expect(pathAsArray[0]).toBe(injectedStuff)
+  // require does not mutate PATH this time
+  require('../../src/extensions/functions/quirksNodeModules')
+  const newPathAsArray = process.env.PATH.split(delineator)
+  // Not in the front
+  expect(newPathAsArray[0]).toBe(injectedStuff)
+})