Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 35 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
3f765c5
move performance test for prepareRepo to integ
Hweinstock Oct 7, 2024
5a0357a
move security scan test
Hweinstock Oct 7, 2024
e07593f
split up perf and non perf tests
Hweinstock Oct 8, 2024
3c1f909
remove shared code to utils
Hweinstock Oct 8, 2024
f5d6c60
move out more shared code
Hweinstock Oct 8, 2024
2897aee
Merge branch 'master' into pTests/moveToInteg
Hweinstock Oct 8, 2024
caf2be8
move some files around
Hweinstock Oct 8, 2024
658ed5a
update imports
Hweinstock Oct 8, 2024
366b038
fix test changes
Hweinstock Oct 8, 2024
1786ff4
delete duplicate test file
Hweinstock Oct 8, 2024
3b987b7
fix tests again
Hweinstock Oct 8, 2024
56819a5
fix tests again
Hweinstock Oct 8, 2024
a13a6c7
resolve conflicts
Hweinstock Oct 10, 2024
da6c8ce
initial work
Hweinstock Oct 10, 2024
b3619a9
delete unneeded code
Hweinstock Oct 10, 2024
f9ed46d
Merge branch 'master' into pTest/systemSpy
Hweinstock Oct 14, 2024
dbefbf0
Merge branch 'master' into pTests/moveToInteg
Hweinstock Oct 14, 2024
685924c
move tests into testPerf
Hweinstock Oct 14, 2024
0aa5546
rename folder
Hweinstock Oct 14, 2024
b0b63ad
implement spy
Hweinstock Oct 14, 2024
4fe7c7b
increase thresholds
Hweinstock Oct 14, 2024
f5947c1
Merge branch 'pTests/moveToInteg' into pTest/systemSpy
Hweinstock Oct 14, 2024
fda96df
build shared utility
Hweinstock Oct 14, 2024
10130a6
refactor collectFiles
Hweinstock Oct 14, 2024
55ee15e
add spies for startSecScan
Hweinstock Oct 14, 2024
10236d8
add system spies for zipCode
Hweinstock Oct 14, 2024
a6eda5b
merge in master
Hweinstock Oct 14, 2024
b8ed1f4
Merge branch 'master' into pTests/systemSpy
Hweinstock Oct 14, 2024
efe25af
add filesystem spy
Hweinstock Oct 14, 2024
74dc701
Merge branch 'master' into pTests/systemSpy
Hweinstock Oct 14, 2024
3a11a85
add spy to file hash test
Hweinstock Oct 15, 2024
1a2017b
add spy for vfs
Hweinstock Oct 15, 2024
5e2d976
adjust thresholds
Hweinstock Oct 15, 2024
157c628
remove temp debump to 1 testrun
Hweinstock Oct 15, 2024
6ecc708
adjust thresholds
Hweinstock Oct 15, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions packages/core/src/amazonqFeatureDev/util/files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')

Expand All @@ -28,17 +29,17 @@ export async function prepareRepoData(
repoRootPaths: string[],
workspaceFolders: CurrentWsFolders,
telemetry: TelemetryHelper,
span: Span<AmazonqCreateUpload>
span: Span<AmazonqCreateUpload>,
zip: AdmZip = new AdmZip()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: AdmZip will be replaced by zip.js #4769

Copy link
Contributor Author

@Hweinstock Hweinstock Oct 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, I see. Do you recommend I remove the AdmZip call counting in the meantime or leave it until that other PR is done?

) {
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_) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
/**
Expand Down Expand Up @@ -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()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/shared/utilities/workspaceUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 12 additions & 2 deletions packages/core/src/testInteg/perf/buildIndex.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down
124 changes: 79 additions & 45 deletions packages/core/src/testInteg/perf/collectFiles.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,55 +8,89 @@ import * as sinon from 'sinon'
import { 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,
heapTotal: 2,
duration: 0.8,
function performanceTestWrapper(totalFiles: number) {
return performanceTest(
{
darwin: {
userCpuUsage: 100,
systemCpuUsage: 35,
heapTotal: 2,
},
linux: {
userCpuUsage: 100,
systemCpuUsage: 35,
heapTotal: 2,
},
win32: {
userCpuUsage: 100,
systemCpuUsage: 35,
heapTotal: 2,
},
},
},
'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)
})
24 changes: 18 additions & 6 deletions packages/core/src/testInteg/perf/getFileSha384.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,23 @@
*/
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 { 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,
testRuns: 10,
linux: {
userCpuUsage: 400,
systemCpuUsage: 35,
Expand All @@ -37,14 +45,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')
},
}
}
Expand All @@ -53,6 +62,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)
Expand Down
Loading
Loading