test(sam): slow SamCliLocalInvokeInvocation, ChildProcess tests #5964

## Problem
- The test failure is due to timeout on Windows when first searching for
the SAM cli path in `src/shared/sam/cli/samCliLocator.ts`. This is a
known slow process.
- The verification of SAM is also slow (100-200ms), even if result is
cached and we do this each time we read from cache.

## Solution
- Verify the cache once, rather than on each read. 
- Find sam cli in the before hook to allow all sam tests to run under
same conditions. Otherwise, the first test is then responsible for
actually finding the path causing it to take way longer than the rest of
the tests.
- Allow 3x retries on before hook on windows to avoid flakiness. 

## Notes
- An alternative solution could involve manually inserting into the
cache, or mocking a piece of the component, but wanted to keep the test
coverage as is.
- This approach also allows us to isolate the finding code into its own
test (rather than whatever the first test is) to see if there are
performance regressions.
- Additionally, retry mechanism was more effective than increasing the
timeout in reducing flakiness.

Author: Hweinstock

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

@@ -6,7 +6,6 @@
 import * as proc from 'child_process'
 import { pushIf } from '../../utilities/collectionUtils'
 import * as nls from 'vscode-nls'
-import { fileExists } from '../../filesystemUtilities'
 import { getLogger, getDebugConsoleLogger, Logger } from '../../logger'
 import { ChildProcess } from '../../utilities/processUtils'
 import { Timeout } from '../../utilities/timeoutUtils'
@@ -15,6 +14,7 @@ import * as vscode from 'vscode'
 import globals from '../../extensionGlobals'
 import { SamCliSettings } from './samCliSettings'
 import { addTelemetryEnvVar, collectSamErrors, SamCliError } from './samCliInvokerUtils'
+import { fs } from '../../fs/fs'
 
 const localize = nls.loadMessageBundle()
 
@@ -232,7 +232,6 @@ export class SamCliLocalInvokeInvocation {
 
     public async execute(timeout?: Timeout): Promise<ChildProcess> {
         await this.validate()
-
         const sam = await this.config.getOrDetectSamCli()
         if (!sam.path) {
             getLogger().warn('SAM CLI not found and not configured')
@@ -288,11 +287,11 @@ export class SamCliLocalInvokeInvocation {
             throw new Error('template resource name is missing or empty')
         }
 
-        if (!(await fileExists(this.args.templatePath))) {
+        if (!(await fs.exists(this.args.templatePath))) {
             throw new Error(`template path does not exist: ${this.args.templatePath}`)
         }
 
-        if (this.args.eventPath !== undefined && !(await fileExists(this.args.eventPath))) {
+        if (this.args.eventPath !== undefined && !(await fs.exists(this.args.eventPath))) {
             throw new Error(`event path does not exist: ${this.args.eventPath}`)
         }
     }

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

@@ -13,13 +13,20 @@ import { PerfLog } from '../../logger/perfLogger'
 import { tryRun } from '../../utilities/pathFind'
 import { mergeResolvedShellPath } from '../../env/resolveEnv'
 
+interface SamLocation {
+    path: string
+    version: string
+}
 export class SamCliLocationProvider {
     private static samCliLocator: BaseSamCliLocator | undefined
-    protected static cachedSamLocation: { path: string; version: string } | undefined
+    protected static cachedSamLocation: SamLocation | undefined
+    private static samLocationValid: boolean = false
 
     /** Checks that the given `sam` actually works by invoking `sam --version`. */
     private static async isValidSamLocation(samPath: string) {
-        return await tryRun(samPath, ['--version'], 'no', 'SAM CLI')
+        const isValid = await tryRun(samPath, ['--version'], 'no', 'SAM CLI')
+        this.samLocationValid = isValid
+        return isValid
     }
 
     /**
@@ -31,11 +38,14 @@ 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 &&
+            (SamCliLocationProvider.samLocationValid ||
+                (await SamCliLocationProvider.isValidSamLocation(cachedLoc.path)))
+        ) {
             perflog.done()
             return cachedLoc
         }
-
         SamCliLocationProvider.cachedSamLocation = await SamCliLocationProvider.getSamCliLocator().getLocation()
         perflog.done()
         return SamCliLocationProvider.cachedSamLocation

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

@@ -83,7 +83,6 @@ export class SamCliSettings extends fromExtensionManifest('aws.samcli', descript
             SamCliSettings.logIfChanged(`SAM CLI location (from settings): ${fromConfig}`)
             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,6 +14,8 @@ 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'
 
 describe('SamCliLocalInvokeInvocation', async function () {
     class TestSamLocalInvokeCommand implements SamLocalInvokeCommand {
@@ -30,6 +32,15 @@ describe('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.retries(3)
+        }
+        // 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')