run search first, avoid re-verifying

Author: Hweinstock

packages/core/src/shared/sam/cli/samCliLocator.ts

@@ -13,9 +13,14 @@ import { PerfLog } from '../../logger/perfLogger'
 import { tryRun } from '../../utilities/pathFind'
 import { mergeResolvedShellPath } from '../../env/resolveEnv'
 
+interface SamLocation {
+    path: string
+    version: string
+    verified?: boolean
+}
 export class SamCliLocationProvider {
     private static samCliLocator: BaseSamCliLocator | undefined
-    protected static cachedSamLocation: { path: string; version: string } | undefined
+    protected static cachedSamLocation: SamLocation | undefined
 
     /** Checks that the given `sam` actually works by invoking `sam --version`. */
     private static async isValidSamLocation(samPath: string) {
@@ -31,11 +36,11 @@ export class SamCliLocationProvider {
         const cachedLoc = forceSearch ? undefined : SamCliLocationProvider.cachedSamLocation
 
         // Avoid searching the system for `sam` (especially slow on Windows).
-        if (cachedLoc && (await SamCliLocationProvider.isValidSamLocation(cachedLoc.path))) {
+        if (cachedLoc && (cachedLoc.verified || (await SamCliLocationProvider.isValidSamLocation(cachedLoc.path)))) {
+            cachedLoc.verified = true
             perflog.done()
             return cachedLoc
         }
-
         SamCliLocationProvider.cachedSamLocation = await SamCliLocationProvider.getSamCliLocator().getLocation()
         perflog.done()
         return SamCliLocationProvider.cachedSamLocation

packages/core/src/shared/sam/cli/samCliSettings.ts

@@ -84,7 +84,6 @@ export class SamCliSettings extends fromExtensionManifest('aws.samcli', descript
             return { path: fromConfig, autoDetected: false }
         }
         const fromSearch = await this.locationProvider.getLocation(forceSearch)
-
         SamCliSettings.logIfChanged(`SAM CLI location (version: ${fromSearch?.version}): ${fromSearch?.path}`)
         return { path: fromSearch?.path, autoDetected: true }
     }

packages/core/src/test/shared/sam/cli/samCliLocalInvoke.test.ts

@@ -14,9 +14,10 @@ import {
 import { ChildProcess } from '../../../../shared/utilities/processUtils'
 import { assertArgIsPresent, assertArgNotPresent, assertArgsContainArgument } from './samCliTestUtils'
 import { fs } from '../../../../shared'
+import { SamCliSettings } from '../../../../shared/sam/cli/samCliSettings'
 import { isWin } from '../../../../shared/vscode/env'
-// eslint-disable-next-line aws-toolkits/no-only-in-tests
-describe.only('SamCliLocalInvokeInvocation', async function () {
+
+describe('SamCliLocalInvokeInvocation', async function () {
     class TestSamLocalInvokeCommand implements SamLocalInvokeCommand {
         public constructor(private readonly onInvoke: ({ ...params }: SamLocalInvokeCommandArgs) => void) {}
 
@@ -31,6 +32,15 @@ describe.only('SamCliLocalInvokeInvocation', async function () {
     let placeholderEventFile: string
     const nonRelevantArg = 'arg is not of interest to this test'
 
+    before(async function () {
+        // File system search on windows can take a while.
+        if (isWin()) {
+            this.timeout(45000)
+        }
+        // This will place the result in the cache allowing all tests to run under same conditions.
+        await SamCliSettings.instance.getOrDetectSamCli()
+    })
+
     beforeEach(async function () {
         tempFolder = await makeTemporaryToolkitFolder()
         placeholderTemplateFile = path.join(tempFolder, 'template.yaml')
@@ -42,40 +52,34 @@ describe.only('SamCliLocalInvokeInvocation', async function () {
     afterEach(async function () {
         await fs.delete(tempFolder, { recursive: true })
     })
-    for (const _ of Array.from({ length: 1000 }, (i) => i)) {
-        // eslint-disable-next-line aws-toolkits/no-only-in-tests
-        it.only('invokes `sam local` with args', async function () {
-            if (isWin()) {
-                // TODO: insert 'C:\\Program Files\\Amazon\\AWSSAMCLI\\bin\\sam.cmd' into the cache.
-                this.timeout(45000)
+
+    it('invokes `sam local` with args', async function () {
+        const taskInvoker: SamLocalInvokeCommand = new TestSamLocalInvokeCommand(
+            (invokeArgs: SamLocalInvokeCommandArgs) => {
+                assert.ok(invokeArgs.args.length >= 2, 'Expected args to be present')
+                assert.strictEqual(invokeArgs.args[0], 'local')
+                assert.strictEqual(invokeArgs.args[1], 'invoke')
+                // --debug is present because tests run with "debug" log-level. #1403
+                assert.strictEqual(invokeArgs.args[2], '--debug')
+                assert.strictEqual(invokeArgs.args[4], '--template')
+                assert.strictEqual(invokeArgs.args[6], '--event')
+                assert.strictEqual(invokeArgs.args[8], '--env-vars')
+
+                // `extraArgs` are appended to the end.
+                assert.strictEqual(invokeArgs.args[10], '--build-dir')
+                assert.strictEqual(invokeArgs.args[11], 'my/build/dir/')
             }
-            const taskInvoker: SamLocalInvokeCommand = new TestSamLocalInvokeCommand(
-                (invokeArgs: SamLocalInvokeCommandArgs) => {
-                    assert.ok(invokeArgs.args.length >= 2, 'Expected args to be present')
-                    assert.strictEqual(invokeArgs.args[0], 'local')
-                    assert.strictEqual(invokeArgs.args[1], 'invoke')
-                    // --debug is present because tests run with "debug" log-level. #1403
-                    assert.strictEqual(invokeArgs.args[2], '--debug')
-                    assert.strictEqual(invokeArgs.args[4], '--template')
-                    assert.strictEqual(invokeArgs.args[6], '--event')
-                    assert.strictEqual(invokeArgs.args[8], '--env-vars')
-
-                    // `extraArgs` are appended to the end.
-                    assert.strictEqual(invokeArgs.args[10], '--build-dir')
-                    assert.strictEqual(invokeArgs.args[11], 'my/build/dir/')
-                }
-            )
-
-            await new SamCliLocalInvokeInvocation({
-                templateResourceName: nonRelevantArg,
-                templatePath: placeholderTemplateFile,
-                eventPath: placeholderEventFile,
-                environmentVariablePath: nonRelevantArg,
-                invoker: taskInvoker,
-                extraArgs: ['--build-dir', 'my/build/dir/'],
-            }).execute()
-        })
-    }
+        )
+
+        await new SamCliLocalInvokeInvocation({
+            templateResourceName: nonRelevantArg,
+            templatePath: placeholderTemplateFile,
+            eventPath: placeholderEventFile,
+            environmentVariablePath: nonRelevantArg,
+            invoker: taskInvoker,
+            extraArgs: ['--build-dir', 'my/build/dir/'],
+        }).execute()
+    })
 
     it('Passes template resource name to sam cli', async function () {
         const expectedResourceName = 'HelloWorldResource'