add more testing

Author: Hweinstock

packages/core/src/shared/utilities/pathFind.ts

@@ -6,44 +6,16 @@
 import * as vscode from 'vscode'
 import * as path from 'path'
 import fs from '../../shared/fs/fs'
-import { ChildProcess, ChildProcessOptions } from './processUtils'
 import { GitExtension } from '../extensions/git'
 import { Settings } from '../settings'
-import { getLogger } from '../logger/logger'
+import * as processUtils from './processUtils'
 
 /** Full path to VSCode CLI. */
 let vscPath: string
 let sshPath: string
 let gitPath: string
 let bashPath: string
 
-/**
- * Tries to execute a program at path `p` with the given args and
- * optionally checks the output for `expected`.
- *
- * @param p path to a program to execute
- * @param args program args
- * @param doLog log failures
- * @param expected output must contain this string
- */
-export async function tryRun(
-    p: string,
-    args: string[],
-    logging: 'yes' | 'no' | 'noresult' = 'yes',
-    expected?: string,
-    opt?: ChildProcessOptions
-): Promise<boolean> {
-    const proc = new ChildProcess(p, args, { logging: 'no' })
-    const r = await proc.run(opt)
-    const ok = r.exitCode === 0 && (expected === undefined || r.stdout.includes(expected))
-    if (logging === 'noresult') {
-        getLogger().info('tryRun: %s: %s', ok ? 'ok' : 'failed', proc)
-    } else if (logging !== 'no') {
-        getLogger().info('tryRun: %s: %s %O', ok ? 'ok' : 'failed', proc, proc.result())
-    }
-    return ok
-}
-
 /**
  * Gets the fullpath to `code` (VSCode CLI), or falls back to "code" (not
  * absolute) if it works.
@@ -78,7 +50,7 @@ export async function getVscodeCliPath(): Promise<string | undefined> {
         if (!vsc || (vsc !== 'code' && !(await fs.exists(vsc)))) {
             continue
         }
-        if (await tryRun(vsc, ['--version'])) {
+        if (await processUtils.tryRun(vsc, ['--version'])) {
             vscPath = vsc
             return vsc
         }
@@ -103,7 +75,7 @@ export async function findTypescriptCompiler(): Promise<string | undefined> {
 
     for (const tsc of tscPaths) {
         // Try to run "tsc -v".
-        if (await tryRun(tsc, ['-v'], 'yes', 'Version')) {
+        if (await processUtils.tryRun(tsc, ['-v'], 'yes', 'Version')) {
             return tsc
         }
     }
@@ -115,8 +87,8 @@ export async function findTypescriptCompiler(): Promise<string | undefined> {
  * Gets the configured `ssh` path, or falls back to "ssh" (not absolute),
  * or tries common locations, or returns undefined.
  */
-export async function findSshPath(): Promise<string | undefined> {
-    if (sshPath !== undefined) {
+export async function findSshPath(useCache: boolean = true): Promise<string | undefined> {
+    if (useCache && sshPath !== undefined) {
         return sshPath
     }
 
@@ -132,7 +104,7 @@ export async function findSshPath(): Promise<string | undefined> {
         if (!p || ('ssh' !== p && !(await fs.exists(p)))) {
             continue
         }
-        if (await tryRun(p, ['-G', 'x'], 'noresult' /* "ssh -G" prints quasi-sensitive info. */)) {
+        if (await processUtils.tryRun(p, ['-G', 'x'], 'noresult' /* "ssh -G" prints quasi-sensitive info. */)) {
             sshPath = p
             return p
         }
@@ -154,7 +126,7 @@ export async function findGitPath(): Promise<string | undefined> {
         if (!p || ('git' !== p && !(await fs.exists(p)))) {
             continue
         }
-        if (await tryRun(p, ['--version'])) {
+        if (await processUtils.tryRun(p, ['--version'])) {
             gitPath = p
             return p
         }
@@ -174,7 +146,7 @@ export async function findBashPath(): Promise<string | undefined> {
         if (!p || ('bash' !== p && !(await fs.exists(p)))) {
             continue
         }
-        if (await tryRun(p, ['--version'])) {
+        if (await processUtils.tryRun(p, ['--version'])) {
             bashPath = p
             return p
         }

packages/core/src/shared/utilities/processUtils.ts

@@ -7,6 +7,7 @@ import * as proc from 'child_process'
 import * as crossSpawn from 'cross-spawn'
 import * as logger from '../logger'
 import { Timeout, CancellationError, waitUntil } from './timeoutUtils'
+import { getLogger } from '..'
 
 export interface RunParameterContext {
     /** Reports an error parsed from the stdin/stdout streams. */
@@ -368,3 +369,30 @@ export class ChildProcess {
         return `${pid} [${this.#command} ${noparams ? '...' : this.#args.join(' ')}]`
     }
 }
+
+/**
+ * Tries to execute a program at path `p` with the given args and
+ * optionally checks the output for `expected`.
+ *
+ * @param p path to a program to execute
+ * @param args program args
+ * @param doLog log failures
+ * @param expected output must contain this string
+ */
+export async function tryRun(
+    p: string,
+    args: string[],
+    logging: 'yes' | 'no' | 'noresult' = 'yes',
+    expected?: string,
+    opt?: ChildProcessOptions
+): Promise<boolean> {
+    const proc = new ChildProcess(p, args, { logging: 'no' })
+    const r = await proc.run(opt)
+    const ok = r.exitCode === 0 && (expected === undefined || r.stdout.includes(expected))
+    if (logging === 'noresult') {
+        getLogger().info('tryRun: %s: %s', ok ? 'ok' : 'failed', proc)
+    } else if (logging !== 'no') {
+        getLogger().info('tryRun: %s: %s %O', ok ? 'ok' : 'failed', proc, proc.result())
+    }
+    return ok
+}

packages/core/src/test/shared/utilities/pathFind.test.ts

@@ -4,12 +4,14 @@
  */
 
 import assert from 'assert'
+import sinon from 'sinon'
 import * as vscode from 'vscode'
 import * as os from 'os'
 import * as path from 'path'
 import * as testutil from '../../testUtil'
 import { fs } from '../../../shared'
-import { findTypescriptCompiler, getVscodeCliPath } from '../../../shared/utilities/pathFind'
+import { findSshPath, findTypescriptCompiler, getVscodeCliPath } from '../../../shared/utilities/pathFind'
+import * as processUtils from '../../../shared/utilities/processUtils'
 
 describe('pathFind', function () {
     it('findTypescriptCompiler()', async function () {
@@ -42,12 +44,40 @@ describe('pathFind', function () {
         assert.ok(regex.test(vscPath), `expected regex ${regex} to match: "${vscPath}"`)
     })
 
-    // it('does a dry run of ssh before returning it', async function () {
-    //     const tryRunStub = sinon.stub(PathFinder, 'tryRun')
-    //     tryRunStub.resolves(true)
-    //     const path = await pathFinder.findSshPath()
-    //     assert.ok(path)
-    //     assert.ok(tryRunStub.calledOnce)
-    //     assert.ok(tryRunStub.calledWith(path))
-    // })
+    describe('findSshPath', function () {
+        let tryRunStub: sinon.SinonStub
+        before(function () {
+            tryRunStub = sinon.stub(processUtils, 'tryRun')
+        })
+
+        after(function () {
+            tryRunStub.restore()
+        })
+
+        it('first tries $PATH', async function () {
+            tryRunStub.onFirstCall().resolves(true)
+
+            const result = await findSshPath(false)
+            assert.ok(result)
+            testutil.assertEqualPaths(result, 'ssh')
+            tryRunStub.resetHistory()
+        })
+
+        it('if $PATH fails, tries /usr/bin/ssh', async function () {
+            tryRunStub.onFirstCall().resolves(false)
+            tryRunStub.onSecondCall().resolves(true)
+
+            const result = await findSshPath(false)
+            assert.ok(result)
+            testutil.assertEqualPaths(result, '/usr/bin/ssh')
+            tryRunStub.resetHistory()
+        })
+
+        it('dry runs the resulting ssh', async function () {
+            tryRunStub.onFirstCall().resolves(true)
+            await findSshPath(false)
+            assert.ok(tryRunStub.calledOnce)
+            tryRunStub.resetHistory()
+        })
+    })
 })