patch: [TIP-8901] Fix hnvm fallback to send warning logs to stderr (#56)

.gitignore

@@ -1,3 +1,4 @@
 .hnvm
 .tmp
 .DS_Store
+test/node_modules/

lib/hnvm/config.sh

@@ -24,8 +24,12 @@ elif [[ -e "/dev/fd/1" && -w "/dev/fd/1" && ! -S "/dev/stdout" ]]; then
   COMMAND_OUTPUT="/dev/fd/1"
 else
   # If COMMAND_OUTPUT is still not assigned by here, fall back to posix-standard /dev/null
-  echo "WARNING: Could not find a writable, non-socket stdout redirect target!"
-  echo "WARNING: Further HNVM output will be redirected to '/dev/null'"
+  #
+  # Very important: any debug warnings should ONLY go to stderr.
+  # If these echoes go to stdout, you get issues like this:
+  # https://compass-tech.atlassian.net/jira/servicedesk/projects/TIP/queues/custom/268/TIP-8901
+  echo "WARNING: Could not find a writable, non-socket stdout redirect target!" >&2
+  echo "WARNING: Further HNVM output will be redirected to '/dev/null'" >&2
   COMMAND_OUTPUT="/dev/null"
 fi
 

test/node-subprocess.test.js

@@ -4,27 +4,41 @@ jest.setTimeout(60_000)
 
 describe('node with a fixed verison', () => {
   const VERSION = '16.13.0'
+  const PNPM_VERSION = '6.0.0'
   let context
 
   beforeAll(() => {
     context = createTestContext()
-    context.createPackageJson({engines: {node: VERSION}})
+    context.createPackageJson({engines: {node: VERSION, pnpm: PNPM_VERSION}})
   })
 
   afterAll(() => {
     context.cleanup()
   })
 
   it('should fallback to using "/dev/null" if HNVM_OUTPUT_DESTINATION is a socket', () => {
-    const result = context.execFileSyncWithSocketOutput(
+    const hnvmProcess = context.execFileSyncWithSocketOutput(
       context.binaries.node,
       ['-p', '"Hello, World!"']
     )
 
-    expect(result).toContain('Hello, World!')
-    expect(result).toContain(
+    expect(hnvmProcess.stdout).toContain('Hello, World!')
+    expect(hnvmProcess.stderr).toContain(
       "WARNING: Could not find a writable, non-socket stdout redirect target!"
     )
-    expect(result).toContain("WARNING: Further HNVM output will be redirected to '/dev/null'")
+    expect(hnvmProcess.stderr).toContain("WARNING: Further HNVM output will be redirected to '/dev/null'")
   })
+
+  it('[TIP-8901] should give only the pnpm version on stdout, when HNVM_OUTPUT_DESTINATION is a socket', () => {
+    const hnvmProcess = context.execFileSyncWithSocketOutput(
+      context.binaries.pnpm,
+      ['--version']
+    )
+
+    expect(hnvmProcess.stdout).toContain('6.0.0')
+    expect(hnvmProcess.stderr).toContain(
+      "WARNING: Could not find a writable, non-socket stdout redirect target!"
+    )
+    expect(hnvmProcess.stderr).toContain("WARNING: Further HNVM output will be redirected to '/dev/null'")
+  });
 })

test/package.json

@@ -8,7 +8,7 @@
     "test": "jest"
   },
   "engines": {
-    "hnvm": "14.18.0"
+    "hnvm": "17.1.0"
   },
   "jest": {
     "testEnvironment": "node"

test/utils.js

@@ -73,9 +73,11 @@ function createTestContext() {
       const outputFile = path.join(testDir, 'stdout')
       fs.writeFileSync(outputFile, '')
 
+      // need to explicitly set PATH, otherwise it defaults to using "/usr/gnu/bin:/usr/local/bin:/bin:/usr/bin:."
+      // this default PATH may have issues finding `jq`, which will break the test when it runs source code
       const stdout = childProcess.execFileSync(file, args, {
         encoding: 'utf-8',
-        env: {HNVM_PATH: hnvmDir, HNVM_OUTPUT_DESTINATION: outputFile},
+        env: {HNVM_PATH: hnvmDir, HNVM_OUTPUT_DESTINATION: outputFile, PATH: process.env.PATH},
         cwd: cwdDir,
       })
 
@@ -84,13 +86,15 @@ function createTestContext() {
     execFileSyncWithSocketOutput(file, args) {
       // Provide a socket as HNVM_OUTPUT_DESTINATION
       // Used for tests that want to test behavior when the redirect target is a socket
-      const stdout = childProcess.execFileSync(file, args, {
+      // need to explicitly set PATH, otherwise it defaults to using "/usr/gnu/bin:/usr/local/bin:/bin:/usr/bin:."
+      // this default PATH may have issues finding `jq`, which will break the test when it runs source code
+      const hnvmProcess = childProcess.spawnSync(file, args, {
         encoding: 'utf-8',
-        env: {HNVM_PATH: hnvmDir, HNVM_OUTPUT_DESTINATION: testStdoutSocket},
+        env: {HNVM_PATH: hnvmDir, HNVM_OUTPUT_DESTINATION: testStdoutSocket, PATH: process.env.PATH},
         cwd: cwdDir,
-      })
+      });
 
-      return stdout
+      return hnvmProcess;
     },
   }
 }