Merge master into feature/q-dev-execution

aws-toolkit-automation · web-flow · commit e9b5bb890454 · 2024-10-23T20:12:46.000+02:00
diff --git a/.github/workflows/node.js.yml b/.github/workflows/node.js.yml
@@ -41,7 +41,7 @@ jobs:
         runs-on: ubuntu-latest
         strategy:
             matrix:
-                node-version: [16.x]
+                node-version: [18.x]
                 vscode-version: [stable]
         env:
             NODE_OPTIONS: '--max-old-space-size=8192'
@@ -62,7 +62,7 @@ jobs:
         strategy:
             fail-fast: false
             matrix:
-                node-version: [16.x]
+                node-version: [18.x]
                 vscode-version: [minimum, stable, insiders]
         env:
             VSCODE_TEST_VERSION: ${{ matrix.vscode-version }}
@@ -110,7 +110,7 @@ jobs:
         strategy:
             fail-fast: true
             matrix:
-                node-version: [16.x]
+                node-version: [18.x]
                 vscode-version: [stable, insiders]
         env:
             VSCODE_TEST_VERSION: ${{ matrix.vscode-version }}
@@ -136,7 +136,7 @@ jobs:
         strategy:
             fail-fast: false
             matrix:
-                node-version: [16.x]
+                node-version: [18.x]
                 vscode-version: [stable, insiders]
         env:
             VSCODE_TEST_VERSION: ${{ matrix.vscode-version }}
diff --git a/buildspec/linuxIntegrationTests.yml b/buildspec/linuxIntegrationTests.yml
@@ -21,7 +21,7 @@ phases:
     install:
         run-as: root
         runtime-versions:
-            nodejs: 16
+            nodejs: 18
             dotnet: 6.0
             java: latest
 
diff --git a/buildspec/linuxTests.yml b/buildspec/linuxTests.yml
@@ -18,7 +18,7 @@ phases:
     install:
         run-as: root
         runtime-versions:
-            nodejs: 16
+            nodejs: 18
         commands:
             - bash buildspec/shared/linux-install.sh
 
diff --git a/buildspec/packageTestVsix.yml b/buildspec/packageTestVsix.yml
@@ -13,7 +13,7 @@ phases:
     install:
         run-as: root
         runtime-versions:
-            nodejs: 16
+            nodejs: 18
         commands:
             - bash buildspec/shared/linux-install.sh
 
diff --git a/buildspec/windowsTests.yml b/buildspec/windowsTests.yml
@@ -5,7 +5,7 @@ env:
 phases:
     install:
         runtime-versions:
-            nodejs: 16
+            nodejs: 18
         commands:
             - |
                 if(-Not($Env:CODECOV_TOKEN -eq $null)) {
diff --git a/packages/core/scripts/test/launchTestUtilities.ts b/packages/core/scripts/test/launchTestUtilities.ts
@@ -182,13 +182,16 @@ async function invokeVSCodeCli(vsCodeExecutablePath: string, args: string[]): Pr
     }
 
     console.log(`Invoking vscode CLI command:\n    "${cli}" ${JSON.stringify(cmdArgs)}`)
+    // Shell option must be true on windows to avoid security error: https://nodejs.org/en/blog/vulnerability/april-2024-security-releases-2
     const spawnResult = proc.spawnSync(cli, cmdArgs, {
         encoding: 'utf-8',
         stdio: 'pipe',
+        shell: process.platform === 'win32',
     })
 
     if (spawnResult.status !== 0) {
         console.log('output: %s', spawnResult.output)
+        console.log('error: %O', spawnResult.error)
         throw new Error(`VS Code CLI command failed (exit-code: ${spawnResult.status}): ${cli} ${cmdArgs}`)
     }
 
diff --git a/packages/core/src/amazonqFeatureDev/util/files.ts b/packages/core/src/amazonqFeatureDev/util/files.ts
@@ -18,6 +18,7 @@ import { AmazonqCreateUpload, Span, telemetry as amznTelemetry } from '../../sha
 import { TelemetryHelper } from './telemetryHelper'
 import { maxRepoSizeBytes } from '../constants'
 import { isCodeFile } from '../../shared/filetypes'
+import { fs } from '../../shared'
 
 const getSha256 = (file: Buffer) => createHash('sha256').update(file).digest('base64')
 
@@ -28,17 +29,17 @@ export async function prepareRepoData(
     repoRootPaths: string[],
     workspaceFolders: CurrentWsFolders,
     telemetry: TelemetryHelper,
-    span: Span<AmazonqCreateUpload>
+    span: Span<AmazonqCreateUpload>,
+    zip: AdmZip = new AdmZip()
 ) {
     try {
         const files = await collectFiles(repoRootPaths, workspaceFolders, true, maxRepoSizeBytes)
-        const zip = new AdmZip()
 
         let totalBytes = 0
         const ignoredExtensionMap = new Map<string, number>()
 
         for (const file of files) {
-            const fileSize = (await vscode.workspace.fs.stat(file.fileUri)).size
+            const fileSize = (await fs.stat(file.fileUri)).size
             const isCodeFile_ = isCodeFile(file.relativeFilePath)
 
             if (fileSize >= maxFileSizeBytes || !isCodeFile_) {
diff --git a/packages/core/src/codewhisperer/commands/startSecurityScan.ts b/packages/core/src/codewhisperer/commands/startSecurityScan.ts
@@ -93,7 +93,8 @@ export async function startSecurityScan(
     editor: vscode.TextEditor | undefined,
     client: DefaultCodeWhispererClient,
     context: vscode.ExtensionContext,
-    scope: CodeWhispererConstants.CodeAnalysisScope
+    scope: CodeWhispererConstants.CodeAnalysisScope,
+    zipUtil: ZipUtil = new ZipUtil()
 ) {
     const logger = getLoggerForScope(scope)
     /**
@@ -130,7 +131,6 @@ export async function startSecurityScan(
          * Step 1: Generate zip
          */
         throwIfCancelled(scope, codeScanStartTime)
-        const zipUtil = new ZipUtil()
         const zipMetadata = await zipUtil.generateZip(editor?.document.uri, scope)
         const projectPaths = zipUtil.getProjectPaths()
 
diff --git a/packages/core/src/codewhisperer/service/transformByQ/transformApiHandler.ts b/packages/core/src/codewhisperer/service/transformByQ/transformApiHandler.ts
@@ -287,13 +287,15 @@ interface ZipCodeResult {
     fileSize: number
 }
 
-export async function zipCode({ dependenciesFolder, humanInTheLoopFlag, modulePath, zipManifest }: IZipCodeParams) {
+export async function zipCode(
+    { dependenciesFolder, humanInTheLoopFlag, modulePath, zipManifest }: IZipCodeParams,
+    zip: AdmZip = new AdmZip()
+) {
     let tempFilePath = undefined
     let logFilePath = undefined
     let dependenciesCopied = false
     try {
         throwIfCancelled()
-        const zip = new AdmZip()
 
         // If no modulePath is passed in, we are not uploaded the source folder
         // NOTE: We only upload dependencies for human in the loop work
diff --git a/packages/core/src/shared/performance/performance.ts b/packages/core/src/shared/performance/performance.ts
@@ -206,3 +206,11 @@ function assertPerformanceMetrics(
         `Expected total duration for ${name} to be less than ${expectedDuration}. Actual duration was ${foundDuration}`
     )
 }
+
+export function getEqualOSTestOptions(testOptions: Partial<PerformanceMetrics>): Partial<TestOptions> {
+    return {
+        linux: testOptions,
+        darwin: testOptions,
+        win32: testOptions,
+    }
+}
diff --git a/packages/core/src/shared/utilities/workspaceUtils.ts b/packages/core/src/shared/utilities/workspaceUtils.ts
@@ -582,7 +582,7 @@ export async function collectFilesForIndex(
                 continue
             }
 
-            const fileStat = await vscode.workspace.fs.stat(file)
+            const fileStat = await fs.stat(file)
             // ignore single file over 10 MB
             if (fileStat.size > 10 * 1024 * 1024) {
                 continue
diff --git a/packages/core/src/testInteg/perf/buildIndex.test.ts b/packages/core/src/testInteg/perf/buildIndex.test.ts
@@ -11,26 +11,36 @@ import { LspClient, LspController } from '../../amazonq'
 import { LanguageClient, ServerOptions } from 'vscode-languageclient'
 import { createTestWorkspace } from '../../test/testUtil'
 import { GetUsageRequestType, IndexRequestType } from '../../amazonq/lsp/types'
-import { getRandomString } from '../../shared'
+import { fs, getRandomString } from '../../shared'
+import { FileSystem } from '../../shared/fs/fs'
+import { getFsCallsUpperBound } from './utilities'
 
 interface SetupResult {
     clientReqStub: sinon.SinonStub
+    fsSpy: sinon.SinonSpiedInstance<FileSystem>
+    findFilesSpy: sinon.SinonSpy
 }
 
 async function verifyResult(setup: SetupResult) {
     assert.ok(setup.clientReqStub.calledTwice)
     assert.ok(setup.clientReqStub.firstCall.calledWith(IndexRequestType))
     assert.ok(setup.clientReqStub.secondCall.calledWith(GetUsageRequestType))
+
+    assert.strictEqual(getFsCallsUpperBound(setup.fsSpy), 0, 'should not make any fs calls')
+    assert.ok(setup.findFilesSpy.callCount <= 2, 'findFiles should not be called more than twice')
 }
 
 async function setupWithWorkspace(numFiles: number, options: { fileContent: string }): Promise<SetupResult> {
     // Force VSCode to find my test workspace only to keep test contained and controlled.
     const testWorksapce = await createTestWorkspace(numFiles, options)
     sinon.stub(vscode.workspace, 'workspaceFolders').value([testWorksapce])
+
     // Avoid sending real request to lsp.
     const clientReqStub = sinon.stub(LanguageClient.prototype, 'sendRequest').resolves(true)
+    const fsSpy = sinon.spy(fs)
+    const findFilesSpy = sinon.spy(vscode.workspace, 'findFiles')
     LspClient.instance.client = new LanguageClient('amazonq', 'test-client', {} as ServerOptions, {})
-    return { clientReqStub }
+    return { clientReqStub, fsSpy, findFilesSpy }
 }
 
 describe('buildIndex', function () {
diff --git a/packages/core/src/testInteg/perf/collectFiles.test.ts b/packages/core/src/testInteg/perf/collectFiles.test.ts
@@ -5,58 +5,80 @@
 import assert from 'assert'
 import * as vscode from 'vscode'
 import * as sinon from 'sinon'
-import { performanceTest } from '../../shared/performance/performance'
+import { getEqualOSTestOptions, performanceTest } from '../../shared/performance/performance'
 import { createTestWorkspaceFolder, toFile } from '../../test/testUtil'
 import path from 'path'
-import { randomUUID } from '../../shared'
+import { fs, randomUUID } from '../../shared'
 import { collectFiles } from '../../shared/utilities/workspaceUtils'
+import { getFsCallsUpperBound } from './utilities'
+import { FileSystem } from '../../shared/fs/fs'
 
-performanceTest(
-    // collecting all files in the workspace and zipping them is pretty resource intensive
-    {
-        linux: {
-            userCpuUsage: 85,
+function performanceTestWrapper(totalFiles: number) {
+    return performanceTest(
+        getEqualOSTestOptions({
+            userCpuUsage: 100,
+            systemCpuUsage: 35,
             heapTotal: 2,
-            duration: 0.8,
-        },
-    },
-    'calculate cpu and memory usage',
-    function () {
-        const totalFiles = 100
-        return {
-            setup: async () => {
-                const workspace = await createTestWorkspaceFolder()
+        }),
+        'calculate cpu and memory usage',
+        function () {
+            return {
+                setup: async () => {
+                    const workspace = await createTestWorkspaceFolder()
 
-                sinon.stub(vscode.workspace, 'workspaceFolders').value([workspace])
+                    sinon.stub(vscode.workspace, 'workspaceFolders').value([workspace])
+                    const fsSpy = sinon.spy(fs)
+                    const findFilesSpy = sinon.spy(vscode.workspace, 'findFiles')
+                    const fileContent = randomUUID()
+                    for (let x = 0; x < totalFiles; x++) {
+                        await toFile(fileContent, path.join(workspace.uri.fsPath, `file.${x}`))
+                    }
 
-                const fileContent = randomUUID()
-                for (let x = 0; x < totalFiles; x++) {
-                    await toFile(fileContent, path.join(workspace.uri.fsPath, `file.${x}`))
-                }
+                    return {
+                        workspace,
+                        fsSpy,
+                        findFilesSpy,
+                    }
+                },
+                execute: async ({ workspace }: { workspace: vscode.WorkspaceFolder }) => {
+                    return {
+                        result: await collectFiles([workspace.uri.fsPath], [workspace], true),
+                    }
+                },
+                verify: (
+                    setup: {
+                        workspace: vscode.WorkspaceFolder
+                        fsSpy: sinon.SinonSpiedInstance<FileSystem>
+                        findFilesSpy: sinon.SinonSpy
+                    },
+                    { result }: { result: Awaited<ReturnType<typeof collectFiles>> }
+                ) => {
+                    assert.deepStrictEqual(result.length, totalFiles)
+                    const sortedFiles = [...result].sort((a, b) => {
+                        const numA = parseInt(a.relativeFilePath.split('.')[1])
+                        const numB = parseInt(b.relativeFilePath.split('.')[1])
+                        return numA - numB
+                    })
+                    for (let x = 0; x < totalFiles; x++) {
+                        assert.deepStrictEqual(sortedFiles[x].relativeFilePath, `file.${x}`)
+                    }
 
-                return {
-                    workspace,
-                }
-            },
-            execute: async ({ workspace }: { workspace: vscode.WorkspaceFolder }) => {
-                return {
-                    result: await collectFiles([workspace.uri.fsPath], [workspace], true),
-                }
-            },
-            verify: (
-                _: { workspace: vscode.WorkspaceFolder },
-                { result }: { result: Awaited<ReturnType<typeof collectFiles>> }
-            ) => {
-                assert.deepStrictEqual(result.length, totalFiles)
-                const sortedFiles = [...result].sort((a, b) => {
-                    const numA = parseInt(a.relativeFilePath.split('.')[1])
-                    const numB = parseInt(b.relativeFilePath.split('.')[1])
-                    return numA - numB
-                })
-                for (let x = 0; x < totalFiles; x++) {
-                    assert.deepStrictEqual(sortedFiles[x].relativeFilePath, `file.${x}`)
-                }
-            },
+                    assert.ok(
+                        getFsCallsUpperBound(setup.fsSpy) <= totalFiles * 5,
+                        'total system calls below 5 per file'
+                    )
+                    assert.ok(setup.findFilesSpy.callCount <= 2, 'findFiles not called more than twice')
+                },
+            }
         }
-    }
-)
+    )
+}
+
+describe('collectFiles', function () {
+    afterEach(function () {
+        sinon.restore()
+    })
+    performanceTestWrapper(10)
+    performanceTestWrapper(100)
+    performanceTestWrapper(250)
+})
diff --git a/packages/core/src/testInteg/perf/downloadExportResultArchive.test.ts b/packages/core/src/testInteg/perf/downloadExportResultArchive.test.ts
@@ -9,7 +9,7 @@ import * as sinon from 'sinon'
 import path from 'path'
 import { fs, getRandomString } from '../../shared'
 import { createTestWorkspace } from '../../test/testUtil'
-import { performanceTest } from '../../shared/performance/performance'
+import { getEqualOSTestOptions, performanceTest } from '../../shared/performance/performance'
 import { downloadExportResultArchive } from '../../shared/utilities/download'
 
 interface SetupResult {
@@ -47,24 +47,11 @@ async function setup(pieces: number, pieceSize: number): Promise<SetupResult> {
 
 function perfTest(pieces: number, pieceSize: number, label: string) {
     return performanceTest(
-        {
-            testRuns: 10,
-            linux: {
-                userCpuUsage: 200,
-                systemCpuUsage: 35,
-                heapTotal: 4,
-            },
-            darwin: {
-                userCpuUsage: 200,
-                systemCpuUsage: 35,
-                heapTotal: 4,
-            },
-            win32: {
-                userCpuUsage: 200,
-                systemCpuUsage: 35,
-                heapTotal: 4,
-            },
-        },
+        getEqualOSTestOptions({
+            userCpuUsage: 200,
+            systemCpuUsage: 35,
+            heapTotal: 4,
+        }),
         label,
         function () {
             return {
diff --git a/packages/core/src/testInteg/perf/getFileSha384.test.ts b/packages/core/src/testInteg/perf/getFileSha384.test.ts
diff --git a/packages/core/src/testInteg/perf/prepareRepoData.test.ts b/packages/core/src/testInteg/perf/prepareRepoData.test.ts
diff --git a/packages/core/src/testInteg/perf/registerNewFiles.test.ts b/packages/core/src/testInteg/perf/registerNewFiles.test.ts
diff --git a/packages/core/src/testInteg/perf/startSecurityScan.test.ts b/packages/core/src/testInteg/perf/startSecurityScan.test.ts
diff --git a/packages/core/src/testInteg/perf/tryInstallLsp.test.ts b/packages/core/src/testInteg/perf/tryInstallLsp.test.ts
diff --git a/packages/core/src/testInteg/perf/utilities.ts b/packages/core/src/testInteg/perf/utilities.ts
diff --git a/packages/core/src/testInteg/perf/zipcode.test.ts b/packages/core/src/testInteg/perf/zipcode.test.ts

Original file line number	Diff line number	Diff line change
`@@ -182,13 +182,16 @@ async function invokeVSCodeCli(vsCodeExecutablePath: string, args: string[]): Pr`
`182`	`182`	`}`
`183`	`183`
`184`	`184`	console.log(`Invoking vscode CLI command:\n "${cli}" ${JSON.stringify(cmdArgs)}`)
	`185`	`+ // Shell option must be true on windows to avoid security error: https://nodejs.org/en/blog/vulnerability/april-2024-security-releases-2`
`185`	`186`	`const spawnResult = proc.spawnSync(cli, cmdArgs, {`
`186`	`187`	`encoding: 'utf-8',`
`187`	`188`	`stdio: 'pipe',`
	`189`	`+ shell: process.platform === 'win32',`
`188`	`190`	`})`
`189`	`191`
`190`	`192`	`if (spawnResult.status !== 0) {`
`191`	`193`	`console.log('output: %s', spawnResult.output)`
	`194`	`+ console.log('error: %O', spawnResult.error)`
`192`	`195`	throw new Error(`VS Code CLI command failed (exit-code: ${spawnResult.status}): ${cli} ${cmdArgs}`)
`193`	`196`	`}`
`194`	`197`
Original file line number	Diff line number	Diff line change
`@@ -206,3 +206,11 @@ function assertPerformanceMetrics(`
`206`	`206`	`Expected total duration for ${name} to be less than ${expectedDuration}. Actual duration was ${foundDuration}`
`207`	`207`	`)`
`208`	`208`	`}`
	`209`	`+`
	`210`	`+export function getEqualOSTestOptions(testOptions: Partial<PerformanceMetrics>): Partial<TestOptions> {`
	`211`	`+ return {`
	`212`	`+ linux: testOptions,`
	`213`	`+ darwin: testOptions,`
	`214`	`+ win32: testOptions,`
	`215`	`+ }`
	`216`	`+}`