From 2dc45ae5f6f400d6b6492a134006c926f01338d0 Mon Sep 17 00:00:00 2001 From: Robert Date: Mon, 5 Dec 2022 14:20:00 +0000 Subject: [PATCH 1/6] Add downloadPercentage to VariantAnalysisScannedRepositoryState --- .../ql-vscode/src/remote-queries/shared/variant-analysis.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/extensions/ql-vscode/src/remote-queries/shared/variant-analysis.ts b/extensions/ql-vscode/src/remote-queries/shared/variant-analysis.ts index e4ef697d641..2c8174516b8 100644 --- a/extensions/ql-vscode/src/remote-queries/shared/variant-analysis.ts +++ b/extensions/ql-vscode/src/remote-queries/shared/variant-analysis.ts @@ -127,6 +127,7 @@ export enum VariantAnalysisScannedRepositoryDownloadStatus { export interface VariantAnalysisScannedRepositoryState { repositoryId: number; downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus; + downloadPercentage?: number; } export interface VariantAnalysisScannedRepositoryResult { From 5c5c2e485d8495a793bbed04599d28ddfd41ad01 Mon Sep 17 00:00:00 2001 From: Robert Date: Mon, 5 Dec 2022 14:44:42 +0000 Subject: [PATCH 2/6] Show download percentage in UI --- .../variant-analysis/RepoRow.stories.tsx | 31 ++++++++++--- .../src/view/common/icon/Codicon.tsx | 1 + .../src/view/variant-analysis/RepoRow.tsx | 32 ++++++++----- .../VariantAnalysisAnalyzedRepos.tsx | 2 +- .../__tests__/RepoRow.spec.tsx | 45 +++++++++++++++---- 5 files changed, 86 insertions(+), 25 deletions(-) diff --git a/extensions/ql-vscode/src/stories/variant-analysis/RepoRow.stories.tsx b/extensions/ql-vscode/src/stories/variant-analysis/RepoRow.stories.tsx index 26d4970466d..02b0677eb9f 100644 --- a/extensions/ql-vscode/src/stories/variant-analysis/RepoRow.stories.tsx +++ b/extensions/ql-vscode/src/stories/variant-analysis/RepoRow.stories.tsx @@ -15,7 +15,7 @@ import { createMockRepositoryWithMetadata } from "../../../test/factories/remote import * as analysesResults from "../remote-queries/data/analysesResultsMessage.json"; import * as rawResults from "../remote-queries/data/rawResults.json"; -import { RepoRow } from "../../view/variant-analysis/RepoRow"; +import { RepoRow, RepoRowProps } from "../../view/variant-analysis/RepoRow"; export default { title: "Variant Analysis/Repo Row", @@ -29,7 +29,7 @@ export default { ], } as ComponentMeta; -const Template: ComponentStory = (args) => ( +const Template: ComponentStory = (args: RepoRowProps) => ( ); @@ -77,7 +77,22 @@ SucceededDownloading.args = { ...Pending.args, status: VariantAnalysisRepoStatus.Succeeded, resultCount: 198, - downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus.InProgress, + downloadState: { + repositoryId: 63537249, + downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus.InProgress, + }, +}; + +export const SucceededDownloadingWithPercentage = Template.bind({}); +SucceededDownloadingWithPercentage.args = { + ...Pending.args, + status: VariantAnalysisRepoStatus.Succeeded, + resultCount: 198, + downloadState: { + repositoryId: 63537249, + downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus.InProgress, + downloadPercentage: 42, + }, }; export const SucceededSuccessfulDownload = Template.bind({}); @@ -85,7 +100,10 @@ SucceededSuccessfulDownload.args = { ...Pending.args, status: VariantAnalysisRepoStatus.Succeeded, resultCount: 198, - downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus.Succeeded, + downloadState: { + repositoryId: 63537249, + downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus.Succeeded, + }, }; export const SucceededFailedDownload = Template.bind({}); @@ -93,7 +111,10 @@ SucceededFailedDownload.args = { ...Pending.args, status: VariantAnalysisRepoStatus.Succeeded, resultCount: 198, - downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus.Failed, + downloadState: { + repositoryId: 63537249, + downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus.Failed, + }, }; export const InterpretedResults = Template.bind({}); diff --git a/extensions/ql-vscode/src/view/common/icon/Codicon.tsx b/extensions/ql-vscode/src/view/common/icon/Codicon.tsx index a5ea123fb42..b7d887eded2 100644 --- a/extensions/ql-vscode/src/view/common/icon/Codicon.tsx +++ b/extensions/ql-vscode/src/view/common/icon/Codicon.tsx @@ -17,6 +17,7 @@ export const Codicon = ({ name, label, className, slot }: Props) => ( diff --git a/extensions/ql-vscode/src/view/variant-analysis/RepoRow.tsx b/extensions/ql-vscode/src/view/variant-analysis/RepoRow.tsx index a1aa2790fda..57565415738 100644 --- a/extensions/ql-vscode/src/view/variant-analysis/RepoRow.tsx +++ b/extensions/ql-vscode/src/view/variant-analysis/RepoRow.tsx @@ -6,6 +6,7 @@ import { isCompletedAnalysisRepoStatus, VariantAnalysisRepoStatus, VariantAnalysisScannedRepositoryDownloadStatus, + VariantAnalysisScannedRepositoryState, } from "../../remote-queries/shared/variant-analysis"; import { formatDecimal } from "../../pure/number"; import { @@ -91,7 +92,7 @@ export type RepoRowProps = { repository: Partial & Pick; status?: VariantAnalysisRepoStatus; - downloadStatus?: VariantAnalysisScannedRepositoryDownloadStatus; + downloadState?: VariantAnalysisScannedRepositoryState; resultCount?: number; interpretedResults?: AnalysisAlert[]; @@ -163,7 +164,7 @@ const filterRepoRowExpandedTelemetry = (v: boolean) => v; export const RepoRow = ({ repository, status, - downloadStatus, + downloadState, resultCount, interpretedResults, rawResults, @@ -185,7 +186,7 @@ export const RepoRow = ({ if ( resultsLoaded || status !== VariantAnalysisRepoStatus.Succeeded || - downloadStatus !== + downloadState?.downloadStatus !== VariantAnalysisScannedRepositoryDownloadStatus.Succeeded ) { setExpanded((oldIsExpanded) => !oldIsExpanded); @@ -203,7 +204,7 @@ export const RepoRow = ({ resultsLoaded, repository.fullName, status, - downloadStatus, + downloadState, setExpanded, ]); @@ -234,10 +235,11 @@ export const RepoRow = ({ [onSelectedChange, repository], ); - const disabled = !canExpand(status, downloadStatus) || resultsLoading; + const disabled = + !canExpand(status, downloadState?.downloadStatus) || resultsLoading; const expandableContentLoaded = isExpandableContentLoaded( status, - downloadStatus, + downloadState?.downloadStatus, resultsLoaded, ); @@ -252,7 +254,9 @@ export const RepoRow = ({ onChange={onChangeCheckbox} onClick={onClickCheckbox} checked={selected} - disabled={!repository.id || !canSelect(status, downloadStatus)} + disabled={ + !repository.id || !canSelect(status, downloadState?.downloadStatus) + } /> {isExpanded && ( @@ -278,11 +282,17 @@ export const RepoRow = ({ )} {!status && } - {downloadStatus === + {downloadState?.downloadStatus === VariantAnalysisScannedRepositoryDownloadStatus.InProgress && ( - + )} - {downloadStatus === + {downloadState?.downloadStatus === VariantAnalysisScannedRepositoryDownloadStatus.Failed && ( )} @@ -296,7 +306,7 @@ export const RepoRow = ({ {isExpanded && expandableContentLoaded && ( diff --git a/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisAnalyzedRepos.tsx b/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisAnalyzedRepos.tsx index 112c0195041..0fa7ec78340 100644 --- a/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisAnalyzedRepos.tsx +++ b/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisAnalyzedRepos.tsx @@ -89,7 +89,7 @@ export const VariantAnalysisAnalyzedRepos = ({ key={repository.repository.id} repository={repository.repository} status={repository.analysisStatus} - downloadStatus={state?.downloadStatus} + downloadState={state} resultCount={repository.resultCount} interpretedResults={results?.interpretedResults} rawResults={results?.rawResults} diff --git a/extensions/ql-vscode/src/view/variant-analysis/__tests__/RepoRow.spec.tsx b/extensions/ql-vscode/src/view/variant-analysis/__tests__/RepoRow.spec.tsx index 86820343c79..570be30b7de 100644 --- a/extensions/ql-vscode/src/view/variant-analysis/__tests__/RepoRow.spec.tsx +++ b/extensions/ql-vscode/src/view/variant-analysis/__tests__/RepoRow.spec.tsx @@ -87,7 +87,10 @@ describe(RepoRow.name, () => { render({ status: VariantAnalysisRepoStatus.Succeeded, resultCount: 178, - downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus.Pending, + downloadState: { + repositoryId: 1, + downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus.Pending, + }, }); expect( @@ -101,7 +104,11 @@ describe(RepoRow.name, () => { render({ status: VariantAnalysisRepoStatus.Succeeded, resultCount: 178, - downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus.InProgress, + downloadState: { + repositoryId: 1, + downloadStatus: + VariantAnalysisScannedRepositoryDownloadStatus.InProgress, + }, }); expect( @@ -115,7 +122,11 @@ describe(RepoRow.name, () => { render({ status: VariantAnalysisRepoStatus.Succeeded, resultCount: 178, - downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus.Succeeded, + downloadState: { + repositoryId: 1, + downloadStatus: + VariantAnalysisScannedRepositoryDownloadStatus.Succeeded, + }, }); expect( @@ -129,7 +140,10 @@ describe(RepoRow.name, () => { render({ status: VariantAnalysisRepoStatus.Succeeded, resultCount: 178, - downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus.Failed, + downloadState: { + repositoryId: 1, + downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus.Failed, + }, }); expect( @@ -320,7 +334,11 @@ describe(RepoRow.name, () => { it("can expand the repo item when succeeded and loaded", async () => { render({ status: VariantAnalysisRepoStatus.Succeeded, - downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus.Succeeded, + downloadState: { + repositoryId: 1, + downloadStatus: + VariantAnalysisScannedRepositoryDownloadStatus.Succeeded, + }, interpretedResults: [], }); @@ -340,7 +358,11 @@ describe(RepoRow.name, () => { it("can expand the repo item when succeeded and not loaded", async () => { const { rerender } = render({ status: VariantAnalysisRepoStatus.Succeeded, - downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus.Succeeded, + downloadState: { + repositoryId: 1, + downloadStatus: + VariantAnalysisScannedRepositoryDownloadStatus.Succeeded, + }, }); await userEvent.click( @@ -411,7 +433,10 @@ describe(RepoRow.name, () => { it("does not allow selecting the item if the item has not been downloaded successfully", async () => { render({ status: VariantAnalysisRepoStatus.Succeeded, - downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus.Failed, + downloadState: { + repositoryId: 1, + downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus.Failed, + }, }); // It seems like sometimes the first render doesn't have the checkbox disabled @@ -424,7 +449,11 @@ describe(RepoRow.name, () => { it("allows selecting the item if the item has been downloaded", async () => { render({ status: VariantAnalysisRepoStatus.Succeeded, - downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus.Succeeded, + downloadState: { + repositoryId: 1, + downloadStatus: + VariantAnalysisScannedRepositoryDownloadStatus.Succeeded, + }, }); expect(screen.getByRole("checkbox")).toBeEnabled(); From 6cf351f332fbda17fadd4d8b9605870dd256f8c9 Mon Sep 17 00:00:00 2001 From: Robert Date: Wed, 14 Dec 2022 15:49:56 +0000 Subject: [PATCH 3/6] Output percentage during results download --- extensions/ql-vscode/src/extension.ts | 1 - .../remote-queries/gh-api/gh-api-client.ts | 10 ------- .../variant-analysis-manager.ts | 18 +++++++++++ .../variant-analysis-results-manager.ts | 24 ++++++++------- .../gh-api/gh-api-client.test.ts | 20 ------------- .../variant-analysis-manager.test.ts | 29 ++++++++---------- .../variant-analysis-results-manager.test.ts | 30 +++++++++---------- 7 files changed, 60 insertions(+), 72 deletions(-) diff --git a/extensions/ql-vscode/src/extension.ts b/extensions/ql-vscode/src/extension.ts index 37050dc50a5..1df08e74950 100644 --- a/extensions/ql-vscode/src/extension.ts +++ b/extensions/ql-vscode/src/extension.ts @@ -633,7 +633,6 @@ async function activateWithInstalledDistribution( ); await ensureDir(variantAnalysisStorageDir); const variantAnalysisResultsManager = new VariantAnalysisResultsManager( - app.credentials, cliServer, extLogger, ); diff --git a/extensions/ql-vscode/src/remote-queries/gh-api/gh-api-client.ts b/extensions/ql-vscode/src/remote-queries/gh-api/gh-api-client.ts index a80323e0d6b..9ecdfbae790 100644 --- a/extensions/ql-vscode/src/remote-queries/gh-api/gh-api-client.ts +++ b/extensions/ql-vscode/src/remote-queries/gh-api/gh-api-client.ts @@ -81,16 +81,6 @@ export async function getVariantAnalysisRepo( return response.data; } -export async function getVariantAnalysisRepoResult( - credentials: Credentials, - downloadUrl: string, -): Promise { - const octokit = await credentials.getOctokit(); - const response = await octokit.request(`GET ${downloadUrl}`); - - return response.data; -} - export async function getRepositoryFromNwo( credentials: Credentials, owner: string, diff --git a/extensions/ql-vscode/src/remote-queries/variant-analysis-manager.ts b/extensions/ql-vscode/src/remote-queries/variant-analysis-manager.ts index b1243c38003..bfc9146e72a 100644 --- a/extensions/ql-vscode/src/remote-queries/variant-analysis-manager.ts +++ b/extensions/ql-vscode/src/remote-queries/variant-analysis-manager.ts @@ -68,6 +68,7 @@ export class VariantAnalysisManager implements VariantAnalysisViewManager { private static readonly REPO_STATES_FILENAME = "repo_states.json"; + private static readonly DOWNLOAD_PERCENTAGE_UPDATE_DELAY_MS = 3000; private readonly _onVariantAnalysisAdded = this.push( new EventEmitter(), @@ -514,10 +515,27 @@ export class VariantAnalysisManager await this.onRepoStateUpdated(variantAnalysis.id, repoState); try { + let lastRepoStateUpdate = 0; + const updateRepoStateCallback = async (downloadPercentage: number) => { + const now = new Date().getTime(); + if ( + lastRepoStateUpdate < + now - VariantAnalysisManager.DOWNLOAD_PERCENTAGE_UPDATE_DELAY_MS + ) { + lastRepoStateUpdate = now; + await this.onRepoStateUpdated(variantAnalysis.id, { + repositoryId: scannedRepo.repository.id, + downloadStatus: + VariantAnalysisScannedRepositoryDownloadStatus.InProgress, + downloadPercentage, + }); + } + }; await this.variantAnalysisResultsManager.download( variantAnalysis.id, repoTask, this.getVariantAnalysisStorageLocation(variantAnalysis.id), + updateRepoStateCallback, ); } catch (e) { repoState.downloadStatus = diff --git a/extensions/ql-vscode/src/remote-queries/variant-analysis-results-manager.ts b/extensions/ql-vscode/src/remote-queries/variant-analysis-results-manager.ts index a8e7ce50a3e..36c139be18c 100644 --- a/extensions/ql-vscode/src/remote-queries/variant-analysis-results-manager.ts +++ b/extensions/ql-vscode/src/remote-queries/variant-analysis-results-manager.ts @@ -1,14 +1,14 @@ import { + appendFileSync, pathExists, mkdir, outputJson, - writeFileSync, readJson, } from "fs-extra"; +import fetch from "node-fetch"; import { EOL } from "os"; import { join } from "path"; -import { Credentials } from "../common/authentication"; import { Logger } from "../common"; import { AnalysisAlert, AnalysisRawResults } from "./shared/analysis-result"; import { sarifParser } from "../sarif-parser"; @@ -21,7 +21,6 @@ import { VariantAnalysisScannedRepositoryResult, } from "./shared/variant-analysis"; import { DisposableObject, DisposeHandler } from "../pure/disposable-object"; -import { getVariantAnalysisRepoResult } from "./gh-api/gh-api-client"; import { EventEmitter } from "vscode"; import { unzipFile } from "../pure/zip"; @@ -63,7 +62,6 @@ export class VariantAnalysisResultsManager extends DisposableObject { readonly onResultLoaded = this._onResultLoaded.event; constructor( - private readonly credentials: Credentials, private readonly cliServer: CodeQLCliServer, private readonly logger: Logger, ) { @@ -75,6 +73,7 @@ export class VariantAnalysisResultsManager extends DisposableObject { variantAnalysisId: number, repoTask: VariantAnalysisRepositoryTask, variantAnalysisStoragePath: string, + onDownloadPercentageChanged: (downloadPercentage: number) => Promise, ): Promise { if (!repoTask.artifactUrl) { throw new Error("Missing artifact URL"); @@ -85,11 +84,6 @@ export class VariantAnalysisResultsManager extends DisposableObject { repoTask.repository.fullName, ); - const result = await getVariantAnalysisRepoResult( - this.credentials, - repoTask.artifactUrl, - ); - if (!(await pathExists(resultDirectory))) { await mkdir(resultDirectory, { recursive: true }); } @@ -100,12 +94,22 @@ export class VariantAnalysisResultsManager extends DisposableObject { ); const zipFilePath = join(resultDirectory, "results.zip"); + + const response = await fetch(repoTask.artifactUrl); + let amountDownloaded = 0; + for await (const chunk of response.body) { + appendFileSync(zipFilePath, Buffer.from(chunk)); + amountDownloaded += chunk.length; + await onDownloadPercentageChanged( + Math.floor((amountDownloaded / response.size) * 100), + ); + } + const unzippedFilesDirectory = join( resultDirectory, VariantAnalysisResultsManager.RESULTS_DIRECTORY, ); - writeFileSync(zipFilePath, Buffer.from(result)); await unzipFile(zipFilePath, unzippedFilesDirectory); this._onResultDownloaded.fire({ diff --git a/extensions/ql-vscode/test/unit-tests/remote-queries/gh-api/gh-api-client.test.ts b/extensions/ql-vscode/test/unit-tests/remote-queries/gh-api/gh-api-client.test.ts index 608b6e974f5..fb012188a60 100644 --- a/extensions/ql-vscode/test/unit-tests/remote-queries/gh-api/gh-api-client.test.ts +++ b/extensions/ql-vscode/test/unit-tests/remote-queries/gh-api/gh-api-client.test.ts @@ -1,10 +1,7 @@ -import { faker } from "@faker-js/faker"; - import { getRepositoryFromNwo, getVariantAnalysis, getVariantAnalysisRepo, - getVariantAnalysisRepoResult, submitVariantAnalysis, } from "../../../../src/remote-queries/gh-api/gh-api-client"; import { createMockSubmission } from "../../../factories/remote-queries/shared/variant-analysis-submission"; @@ -69,23 +66,6 @@ describe("getVariantAnalysisRepo", () => { }); }); -describe("getVariantAnalysisRepoResult", () => { - it("returns the variant analysis repo result", async () => { - await mockServer.loadScenario("problem-query-success"); - - const result = await getVariantAnalysisRepoResult( - testCredentialsWithRealOctokit(), - `https://objects-origin.githubusercontent.com/codeql-query-console/codeql-variant-analysis-repo-tasks/${variantAnalysisId}/${repoTaskId}/${faker.datatype.uuid()}`, - ); - - expect(result).toBeDefined(); - expect(result).toBeInstanceOf(ArrayBuffer); - expect(result.byteLength).toBe( - variantAnalysisRepoJson_response.body.artifact_size_in_bytes, - ); - }); -}); - describe("getRepositoryFromNwo", () => { it("returns the repository", async () => { await mockServer.loadScenario("problem-query-success"); diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/remote-queries/variant-analysis-manager.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/remote-queries/variant-analysis-manager.test.ts index a98338fad38..e45787323d5 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/remote-queries/variant-analysis-manager.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/remote-queries/variant-analysis-manager.test.ts @@ -21,6 +21,8 @@ import * as ghApiClient from "../../../../src/remote-queries/gh-api/gh-api-clien import * as ghActionsApiClient from "../../../../src/remote-queries/gh-api/gh-actions-api-client"; import * as fs from "fs-extra"; import { join } from "path"; +import { Response } from "node-fetch"; +import * as fetchModule from "node-fetch"; import { VariantAnalysisManager } from "../../../../src/remote-queries/variant-analysis-manager"; import { CodeQLCliServer } from "../../../../src/cli"; @@ -95,7 +97,6 @@ describe("Variant Analysis Manager", () => { cli = extension.cliServer; app = createMockApp({}); variantAnalysisResultsManager = new VariantAnalysisResultsManager( - app.credentials, cli, extLogger, ); @@ -334,32 +335,21 @@ describe("Variant Analysis Manager", () => { }); describe("autoDownloadVariantAnalysisResult", () => { - let arrayBuffer: ArrayBuffer; - let getVariantAnalysisRepoStub: jest.SpiedFunction< typeof ghApiClient.getVariantAnalysisRepo >; let getVariantAnalysisRepoResultStub: jest.SpiedFunction< - typeof ghApiClient.getVariantAnalysisRepoResult + typeof fetchModule.default >; let repoStatesPath: string; beforeEach(async () => { - const sourceFilePath = join( - __dirname, - "../data/variant-analysis-results.zip", - ); - arrayBuffer = fs.readFileSync(sourceFilePath).buffer; - getVariantAnalysisRepoStub = jest.spyOn( ghApiClient, "getVariantAnalysisRepo", ); - getVariantAnalysisRepoResultStub = jest.spyOn( - ghApiClient, - "getVariantAnalysisRepoResult", - ); + getVariantAnalysisRepoResultStub = jest.spyOn(fetchModule, "default"); repoStatesPath = join( storagePath, @@ -374,7 +364,6 @@ describe("Variant Analysis Manager", () => { delete dummyRepoTask.artifact_url; getVariantAnalysisRepoStub.mockResolvedValue(dummyRepoTask); - getVariantAnalysisRepoResultStub.mockResolvedValue(arrayBuffer); }); it("should not try to download the result", async () => { @@ -395,7 +384,15 @@ describe("Variant Analysis Manager", () => { dummyRepoTask = createMockVariantAnalysisRepoTask(); getVariantAnalysisRepoStub.mockResolvedValue(dummyRepoTask); - getVariantAnalysisRepoResultStub.mockResolvedValue(arrayBuffer); + + const sourceFilePath = join( + __dirname, + "../data/variant-analysis-results.zip", + ); + const arrayBuffer = fs.readFileSync(sourceFilePath).buffer; + getVariantAnalysisRepoResultStub.mockResolvedValue( + new Response(arrayBuffer), + ); }); it("should return early if variant analysis is cancelled", async () => { diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/remote-queries/variant-analysis-results-manager.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/remote-queries/variant-analysis-results-manager.test.ts index 5da89124fe0..9bb12aba545 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/remote-queries/variant-analysis-results-manager.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/remote-queries/variant-analysis-results-manager.test.ts @@ -3,19 +3,18 @@ import { CodeQLExtensionInterface } from "../../../../src/extension"; import { extLogger } from "../../../../src/common"; import * as fs from "fs-extra"; import { join, resolve } from "path"; +import { Response, RequestInfo, RequestInit } from "node-fetch"; +import * as fetchModule from "node-fetch"; import { VariantAnalysisResultsManager } from "../../../../src/remote-queries/variant-analysis-results-manager"; import { CodeQLCliServer } from "../../../../src/cli"; import { storagePath } from "../global.helper"; import { faker } from "@faker-js/faker"; -import * as ghApiClient from "../../../../src/remote-queries/gh-api/gh-api-client"; import { createMockVariantAnalysisRepositoryTask } from "../../../factories/remote-queries/shared/variant-analysis-repo-tasks"; import { VariantAnalysisRepositoryTask, VariantAnalysisScannedRepositoryResult, } from "../../../../src/remote-queries/shared/variant-analysis"; -import { testCredentialsWithStub } from "../../../factories/authentication"; -import { Credentials } from "../../../../src/common/authentication"; jest.setTimeout(10_000); @@ -44,7 +43,6 @@ describe(VariantAnalysisResultsManager.name, () => { jest.spyOn(extLogger, "log").mockResolvedValue(undefined); variantAnalysisResultsManager = new VariantAnalysisResultsManager( - testCredentialsWithStub(), cli, extLogger, ); @@ -89,6 +87,7 @@ describe(VariantAnalysisResultsManager.name, () => { variantAnalysisId, dummyRepoTask, variantAnalysisStoragePath, + () => Promise.resolve(), ), ).rejects.toThrow("Missing artifact URL"); }); @@ -98,7 +97,7 @@ describe(VariantAnalysisResultsManager.name, () => { let arrayBuffer: ArrayBuffer; let getVariantAnalysisRepoResultStub: jest.SpiedFunction< - typeof ghApiClient.getVariantAnalysisRepoResult + typeof fetchModule.default >; beforeEach(async () => { @@ -109,15 +108,13 @@ describe(VariantAnalysisResultsManager.name, () => { arrayBuffer = fs.readFileSync(sourceFilePath).buffer; getVariantAnalysisRepoResultStub = jest - .spyOn(ghApiClient, "getVariantAnalysisRepoResult") - .mockImplementation( - (_credentials: Credentials, downloadUrl: string) => { - if (downloadUrl === dummyRepoTask.artifactUrl) { - return Promise.resolve(arrayBuffer); - } - return Promise.reject(new Error("Unexpected artifact URL")); - }, - ); + .spyOn(fetchModule, "default") + .mockImplementation((url: RequestInfo, _init?: RequestInit) => { + if (url === dummyRepoTask.artifactUrl) { + return Promise.resolve(new Response(arrayBuffer)); + } + return Promise.reject(new Error("Unexpected artifact URL")); + }); }); it("should call the API to download the results", async () => { @@ -125,6 +122,7 @@ describe(VariantAnalysisResultsManager.name, () => { variantAnalysisId, dummyRepoTask, variantAnalysisStoragePath, + () => Promise.resolve(), ); expect(getVariantAnalysisRepoResultStub).toHaveBeenCalledTimes(1); @@ -135,6 +133,7 @@ describe(VariantAnalysisResultsManager.name, () => { variantAnalysisId, dummyRepoTask, variantAnalysisStoragePath, + () => Promise.resolve(), ); expect(fs.existsSync(`${repoTaskStorageDirectory}/results.zip`)).toBe( @@ -147,6 +146,7 @@ describe(VariantAnalysisResultsManager.name, () => { variantAnalysisId, dummyRepoTask, variantAnalysisStoragePath, + () => Promise.resolve(), ); expect( @@ -160,6 +160,7 @@ describe(VariantAnalysisResultsManager.name, () => { variantAnalysisId, dummyRepoTask, variantAnalysisStoragePath, + () => Promise.resolve(), ); expect( @@ -185,7 +186,6 @@ describe(VariantAnalysisResultsManager.name, () => { beforeEach(() => { variantAnalysisResultsManager = new VariantAnalysisResultsManager( - testCredentialsWithStub(), cli, extLogger, ); From 37c9414dadd7b96161b6e9c1db2493c356e52b7b Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Wed, 25 Jan 2023 16:07:29 +0100 Subject: [PATCH 4/6] Use async `appendFile` --- .../remote-queries/variant-analysis-results-manager.ts | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/extensions/ql-vscode/src/remote-queries/variant-analysis-results-manager.ts b/extensions/ql-vscode/src/remote-queries/variant-analysis-results-manager.ts index 36c139be18c..4965b16f6cc 100644 --- a/extensions/ql-vscode/src/remote-queries/variant-analysis-results-manager.ts +++ b/extensions/ql-vscode/src/remote-queries/variant-analysis-results-manager.ts @@ -1,10 +1,4 @@ -import { - appendFileSync, - pathExists, - mkdir, - outputJson, - readJson, -} from "fs-extra"; +import { appendFile, pathExists, mkdir, outputJson, readJson } from "fs-extra"; import fetch from "node-fetch"; import { EOL } from "os"; import { join } from "path"; @@ -98,7 +92,7 @@ export class VariantAnalysisResultsManager extends DisposableObject { const response = await fetch(repoTask.artifactUrl); let amountDownloaded = 0; for await (const chunk of response.body) { - appendFileSync(zipFilePath, Buffer.from(chunk)); + await appendFile(zipFilePath, Buffer.from(chunk)); amountDownloaded += chunk.length; await onDownloadPercentageChanged( Math.floor((amountDownloaded / response.size) * 100), From 610a623bc57617059a5197b47724918f29376490 Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Thu, 26 Jan 2023 12:24:47 +0100 Subject: [PATCH 5/6] Fix download tests --- .../remote-queries/variant-analysis-manager.test.ts | 9 +++++---- .../variant-analysis-results-manager.test.ts | 8 ++++---- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/remote-queries/variant-analysis-manager.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/remote-queries/variant-analysis-manager.test.ts index e45787323d5..55aaf00b2d0 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/remote-queries/variant-analysis-manager.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/remote-queries/variant-analysis-manager.test.ts @@ -21,6 +21,7 @@ import * as ghApiClient from "../../../../src/remote-queries/gh-api/gh-api-clien import * as ghActionsApiClient from "../../../../src/remote-queries/gh-api/gh-actions-api-client"; import * as fs from "fs-extra"; import { join } from "path"; +import { Readable } from "stream"; import { Response } from "node-fetch"; import * as fetchModule from "node-fetch"; @@ -389,10 +390,10 @@ describe("Variant Analysis Manager", () => { __dirname, "../data/variant-analysis-results.zip", ); - const arrayBuffer = fs.readFileSync(sourceFilePath).buffer; - getVariantAnalysisRepoResultStub.mockResolvedValue( - new Response(arrayBuffer), - ); + const fileContents = fs.readFileSync(sourceFilePath); + const response = new Response(Readable.from(fileContents)); + response.size = fileContents.length; + getVariantAnalysisRepoResultStub.mockResolvedValue(response); }); it("should return early if variant analysis is cancelled", async () => { diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/remote-queries/variant-analysis-results-manager.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/remote-queries/variant-analysis-results-manager.test.ts index 9bb12aba545..0166728ca13 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/remote-queries/variant-analysis-results-manager.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/remote-queries/variant-analysis-results-manager.test.ts @@ -3,6 +3,7 @@ import { CodeQLExtensionInterface } from "../../../../src/extension"; import { extLogger } from "../../../../src/common"; import * as fs from "fs-extra"; import { join, resolve } from "path"; +import { Readable } from "stream"; import { Response, RequestInfo, RequestInit } from "node-fetch"; import * as fetchModule from "node-fetch"; @@ -94,24 +95,23 @@ describe(VariantAnalysisResultsManager.name, () => { }); describe("when the artifact_url is present", () => { - let arrayBuffer: ArrayBuffer; - let getVariantAnalysisRepoResultStub: jest.SpiedFunction< typeof fetchModule.default >; + let fileContents: Buffer; beforeEach(async () => { const sourceFilePath = join( __dirname, "../data/variant-analysis-results.zip", ); - arrayBuffer = fs.readFileSync(sourceFilePath).buffer; + fileContents = fs.readFileSync(sourceFilePath); getVariantAnalysisRepoResultStub = jest .spyOn(fetchModule, "default") .mockImplementation((url: RequestInfo, _init?: RequestInit) => { if (url === dummyRepoTask.artifactUrl) { - return Promise.resolve(new Response(arrayBuffer)); + return Promise.resolve(new Response(Readable.from(fileContents))); } return Promise.reject(new Error("Unexpected artifact URL")); }); From 0efbbbfef14e8b4dee79f48c789bfbcbbfbebf5c Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Thu, 26 Jan 2023 12:25:06 +0100 Subject: [PATCH 6/6] Add test for download progress reporting --- .../variant-analysis-results-manager.test.ts | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/remote-queries/variant-analysis-results-manager.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/remote-queries/variant-analysis-results-manager.test.ts index 0166728ca13..3bb4fe0e2a0 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/remote-queries/variant-analysis-results-manager.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/remote-queries/variant-analysis-results-manager.test.ts @@ -154,6 +154,47 @@ describe(VariantAnalysisResultsManager.name, () => { ).toBe(true); }); + it("should report download progress", async () => { + // This generates a "fake" stream which "downloads" the file in 5 chunks, + // rather than in 1 chunk. This is used for testing that we actually get + // multiple progress reports. + async function* generateInParts() { + const partLength = fileContents.length / 5; + for (let i = 0; i < 5; i++) { + yield fileContents.slice(i * partLength, (i + 1) * partLength); + } + } + + getVariantAnalysisRepoResultStub.mockImplementation( + (url: RequestInfo, _init?: RequestInit) => { + if (url === dummyRepoTask.artifactUrl) { + const response = new Response(Readable.from(generateInParts())); + response.size = fileContents.length; + return Promise.resolve(response); + } + return Promise.reject(new Error("Unexpected artifact URL")); + }, + ); + + const downloadPercentageChanged = jest + .fn() + .mockResolvedValue(undefined); + + await variantAnalysisResultsManager.download( + variantAnalysisId, + dummyRepoTask, + variantAnalysisStoragePath, + downloadPercentageChanged, + ); + + expect(downloadPercentageChanged).toHaveBeenCalledTimes(5); + expect(downloadPercentageChanged).toHaveBeenCalledWith(20); + expect(downloadPercentageChanged).toHaveBeenCalledWith(40); + expect(downloadPercentageChanged).toHaveBeenCalledWith(60); + expect(downloadPercentageChanged).toHaveBeenCalledWith(80); + expect(downloadPercentageChanged).toHaveBeenCalledWith(100); + }); + describe("isVariantAnalysisRepoDownloaded", () => { it("should return true once results are downloaded", async () => { await variantAnalysisResultsManager.download(