test(performance): make tests more deterministic by relying more on system counts (#5786)

## Problem
Performance tests are currently flaky, making them a poor signal for
performance regressions within the code base. Additionally, false alarms
slow down development of unrelated features as test failures keep CI
from being "green".

Therefore, rather than relying only on system usage thresholds for
performance regressions, we can count the number of high-risk /
potentially slow operations made by the code as a deterministic measure
of its performance. However, directly counting each individual
potentially slow operation used within the performance tests highly
couples the test to the implementation details, making the tests less
effective if the implementation changes.

Therefore, the goal of this PR is the following:

1. decrease performance test flakiness by increasing thresholds. 
2. increase performance test effectiveness by relying on deterministic
measures.
3. avoid coupling the tests to the implementation details. 

## Solution
To meet goal (1), we increase the thresholds of the tests to decrease
the changes of a false alarm.

To meet goal (2), we count expensive operations. But, to avoid tying it
to the implementation details, we count the expensive operations using
somewhat-loose upper bounds. Thus, implementation changes modifying the
exact number of expensive operations by a small constant do not set a
false alarm. However, if they increase the number of expensive
operations by a multiplicative factor, the upper bound will alert us.

As an example, we don't want the test to fail if it makes 5-10 more file
system calls when working with a few hundred files, but we do want the
test to fail if it makes 2x the number of files system calls. Therefore,
in the code the bounds are often described as "operations per file",
since it is the multiplicative increase we are concerned about. This
allows us to achieve goal (3).

### Implementation Details
- The most common "expensive operation" we count is file system calls
(through our `fs` module). Some other examples include the use of `zip`
libraries or loading large files into memory.
- We separate the upper bounds for the file system into `read` and
`write` bounds. This granularity allows us to assert that specific code
paths do not modify any files.

## Notes
- `AdmZip` removed in
#4769 , so we don't
address spying on it here. Once `zip.js` is implemented, we can spy on
it in a follow up.
---

<!--- REMINDER: Ensure that your PR meets the guidelines in
CONTRIBUTING.md -->

License: I confirm that my contribution is made under the terms of the
Apache 2.0 license.

Author: Hweinstock

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_) {

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()
 

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

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,
+    }
+}

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

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 () {

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)
+})

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 {

packages/core/src/testInteg/perf/getFileSha384.test.ts

@@ -4,31 +4,26 @@
  */
 import assert from 'assert'
 import path from 'path'
+import sinon from 'sinon'
 import { getTestWorkspaceFolder } from '../integrationTestsUtilities'
 import { fs, getRandomString } from '../../shared'
 import { LspController } from '../../amazonq'
-import { performanceTest } from '../../shared/performance/performance'
+import { getEqualOSTestOptions, performanceTest } from '../../shared/performance/performance'
+import { FileSystem } from '../../shared/fs/fs'
+import { getFsCallsUpperBound } from './utilities'
+
+interface SetupResult {
+    testFile: string
+    fsSpy: sinon.SinonSpiedInstance<FileSystem>
+}
 
 function performanceTestWrapper(label: string, fileSize: number) {
     return performanceTest(
-        {
-            testRuns: 1,
-            linux: {
-                userCpuUsage: 400,
-                systemCpuUsage: 35,
-                heapTotal: 4,
-            },
-            darwin: {
-                userCpuUsage: 400,
-                systemCpuUsage: 35,
-                heapTotal: 4,
-            },
-            win32: {
-                userCpuUsage: 400,
-                systemCpuUsage: 35,
-                heapTotal: 4,
-            },
-        },
+        getEqualOSTestOptions({
+            userCpuUsage: 400,
+            systemCpuUsage: 35,
+            heapTotal: 4,
+        }),
         label,
         function () {
             return {
@@ -37,14 +32,15 @@ function performanceTestWrapper(label: string, fileSize: number) {
                     const fileContent = getRandomString(fileSize)
                     const testFile = path.join(workspace, 'test-file')
                     await fs.writeFile(testFile, fileContent)
-
-                    return testFile
+                    const fsSpy = sinon.spy(fs)
+                    return { testFile, fsSpy }
                 },
-                execute: async (testFile: string) => {
-                    return await LspController.instance.getFileSha384(testFile)
+                execute: async (setup: SetupResult) => {
+                    return await LspController.instance.getFileSha384(setup.testFile)
                 },
-                verify: async (_testFile: string, result: string) => {
+                verify: async (setup: SetupResult, result: string) => {
                     assert.strictEqual(result.length, 96)
+                    assert.ok(getFsCallsUpperBound(setup.fsSpy) <= 1, 'makes a single call to fs')
                 },
             }
         }
@@ -53,6 +49,9 @@ function performanceTestWrapper(label: string, fileSize: number) {
 
 describe('getFileSha384', function () {
     describe('performance tests', function () {
+        afterEach(function () {
+            sinon.restore()
+        })
         performanceTestWrapper('1MB', 1000)
         performanceTestWrapper('2MB', 2000)
         performanceTestWrapper('4MB', 4000)