Add test for the download validation failed case

akshita31 · akshita31 · commit a2aa28beecb2 · 2018-12-18T19:30:34.000-08:00
diff --git a/src/main.ts b/src/main.ts
@@ -42,6 +42,7 @@ import { AbsolutePathPackage } from './packageManager/AbsolutePathPackage';
 import { downloadAndInstallPackages } from './packageManager/downloadAndInstallPackages';
 import IInstallDependencies from './packageManager/IInstallDependencies';
 import { installRuntimeDependencies } from './InstallRuntimeDependencies';
+import { isValidDownload } from './packageManager/isValidDownload';
 
 export async function activate(context: vscode.ExtensionContext): Promise<CSharpExtensionExports> {
 
@@ -119,7 +120,7 @@ export async function activate(context: vscode.ExtensionContext): Promise<CSharp
     eventStream.subscribe(telemetryObserver.post);
 
     let networkSettingsProvider = vscodeNetworkSettingsProvider(vscode);
-    let installDependencies: IInstallDependencies = (dependencies: AbsolutePathPackage[]) => downloadAndInstallPackages(dependencies, networkSettingsProvider, eventStream);
+    let installDependencies: IInstallDependencies = (dependencies: AbsolutePathPackage[]) => downloadAndInstallPackages(dependencies, networkSettingsProvider, eventStream, isValidDownload);
     let runtimeDependenciesExist = await ensureRuntimeDependencies(extension, eventStream, platformInfo, installDependencies);
 
     // activate language services
diff --git a/src/omnisharp/OmnisharpDownloader.ts b/src/omnisharp/OmnisharpDownloader.ts
@@ -12,6 +12,7 @@ import { downloadAndInstallPackages } from '../packageManager/downloadAndInstall
 import { DownloadFile } from '../packageManager/FileDownloader';
 import { getRuntimeDependenciesPackages } from '../tools/RuntimeDependencyPackageUtils';
 import { getAbsolutePathPackagesToInstall } from '../packageManager/getAbsolutePathPackagesToInstall';
+import { isValidDownload } from '../packageManager/isValidDownload';
 
 export class OmnisharpDownloader {
 
@@ -30,7 +31,7 @@ export class OmnisharpDownloader {
         if (packagesToInstall && packagesToInstall.length > 0) {
             this.eventStream.post(new PackageInstallation(`OmniSharp Version = ${version}`));
             this.eventStream.post(new LogPlatformInfo(this.platformInfo));
-            await downloadAndInstallPackages(packagesToInstall, this.networkSettingsProvider, this.eventStream);
+            await downloadAndInstallPackages(packagesToInstall, this.networkSettingsProvider, this.eventStream, isValidDownload);
             this.eventStream.post(new InstallationSuccess());
         }
     }
diff --git a/src/packageManager/downloadAndInstallPackages.ts b/src/packageManager/downloadAndInstallPackages.ts
@@ -14,30 +14,29 @@ import { touchInstallFile, InstallFileType, deleteInstallFile, installFileExists
 import { InstallationFailure, IntegrityCheckFailure } from "../omnisharp/loggingEvents";
 import { mkdirpSync } from "fs-extra";
 import { PackageInstallStart } from "../omnisharp/loggingEvents";
-import { isValidDownload } from './isValidDownload';
+import { DownloadValidator } from './isValidDownload';
 
-export async function downloadAndInstallPackages(packages: AbsolutePathPackage[], provider: NetworkSettingsProvider, eventStream: EventStream) {
+export async function downloadAndInstallPackages(packages: AbsolutePathPackage[], provider: NetworkSettingsProvider, eventStream: EventStream, downloadValidator: DownloadValidator) {
     if (packages) {
         eventStream.post(new PackageInstallStart());
         for (let pkg of packages) {
             let installationStage = "touchBeginFile";
             try {
                 mkdirpSync(pkg.installPath.value);
                 await touchInstallFile(pkg.installPath, InstallFileType.Begin);
-                let count = 0;
-                while (count < 2) {
-                    count++;
+                let count = 1;
+                let willTryInstallingPackage = () => count <= 2; // try 2 times
+                while (willTryInstallingPackage()) {
+                    count = count + 1;
                     let buffer = await DownloadFile(pkg.description, eventStream, provider, pkg.url, pkg.fallbackUrl);
-                    if (isValidDownload(buffer, pkg.integrity, eventStream)) {
+                    if (downloadValidator(buffer, pkg.integrity, eventStream)) {
                         await InstallZip(buffer, pkg.description, pkg.installPath, pkg.binaries, eventStream);
                         installationStage = 'touchLockFile';
                         await touchInstallFile(pkg.installPath, InstallFileType.Lock);
                         break;
                     }
                     else {
-                        let shouldRetry = count == 1;
-                        eventStream.post(new IntegrityCheckFailure(pkg.description, pkg.url, shouldRetry));
-                        break;
+                        eventStream.post(new IntegrityCheckFailure(pkg.description, pkg.url, willTryInstallingPackage()));
                     }
                 }
             }
diff --git a/src/packageManager/isValidDownload.ts b/src/packageManager/isValidDownload.ts
@@ -7,13 +7,17 @@ import * as crypto from "crypto";
 import { EventStream } from "../EventStream";
 import { IntegrityCheckSuccess, DownloadValidation } from "../omnisharp/loggingEvents";
 
+export interface DownloadValidator {
+    (buffer: Buffer, integrity: string, eventStream: EventStream): boolean;
+}
+
 export function isValidDownload(buffer: Buffer, integrity: string, eventStream: EventStream): boolean {
     let hash = crypto.createHash('sha256');
     if (integrity && integrity.length > 0) {
         eventStream.post(new DownloadValidation());
         hash.update(buffer);
-        let value = hash.digest('hex');
-        if (value.toUpperCase() == integrity.toUpperCase()) {
+        let value = hash.digest('hex');       
+        if (value.toUpperCase() == integrity.toUpperCase()) {         
             eventStream.post(new IntegrityCheckSuccess());
             return true;
         }
diff --git a/tasks/offlinePackagingTasks.ts b/tasks/offlinePackagingTasks.ts
@@ -23,6 +23,7 @@ import NetworkSettings from '../src/NetworkSettings';
 import { commandLineOptions } from '../tasks/commandLineArguments';
 import { getRuntimeDependenciesPackages } from '../src/tools/RuntimeDependencyPackageUtils';
 import { getAbsolutePathPackagesToInstall } from '../src/packageManager/getAbsolutePathPackagesToInstall';
+import { isValidDownload } from '../src/packageManager/isValidDownload';
 
 gulp.task('vsix:offline:package', async () => {
     del.sync(vscodeignorePath);
@@ -93,7 +94,7 @@ async function install(platformInfo: PlatformInformation, packageJSON: any) {
     let runTimeDependencies = getRuntimeDependenciesPackages(packageJSON);
     let packagesToInstall = await getAbsolutePathPackagesToInstall(runTimeDependencies, platformInfo, codeExtensionPath);
     let provider = () => new NetworkSettings(undefined, undefined);
-    await downloadAndInstallPackages(packagesToInstall, provider, eventStream);
+    await downloadAndInstallPackages(packagesToInstall, provider, eventStream, isValidDownload);
     await debugUtil.CoreClrDebugUtil.writeEmptyFile(debuggerUtil.installCompleteFilePath());
 }
 
diff --git a/test/unitTests/Packages/downloadAndInstallPackages.test.ts b/test/unitTests/Packages/downloadAndInstallPackages.test.ts
@@ -12,12 +12,13 @@ import TestZip from '../testAssets/TestZip';
 import { downloadAndInstallPackages } from '../../../src/packageManager/downloadAndInstallPackages';
 import NetworkSettings from '../../../src/NetworkSettings';
 import { EventStream } from '../../../src/EventStream';
-import { DownloadStart, DownloadSizeObtained, DownloadProgress, DownloadSuccess, InstallationStart, PackageInstallStart } from '../../../src/omnisharp/loggingEvents';
+import { DownloadStart, DownloadSizeObtained, DownloadProgress, DownloadSuccess, InstallationStart, PackageInstallStart, IntegrityCheckFailure } from '../../../src/omnisharp/loggingEvents';
 import MockHttpsServer from '../testAssets/MockHttpsServer';
 import { createTestFile } from '../testAssets/TestFile';
 import TestEventBus from '../testAssets/TestEventBus';
 import { AbsolutePathPackage } from '../../../src/packageManager/AbsolutePathPackage';
 import { AbsolutePath } from '../../../src/packageManager/AbsolutePath';
+import { DownloadValidator } from '../../../src/packageManager/isValidDownload';
 
 chai.use(chaiAsPromised);
 let expect = chai.expect;
@@ -31,6 +32,7 @@ suite(`${downloadAndInstallPackages.name}`, () => {
     let eventBus: TestEventBus;
     let downloadablePackage: AbsolutePathPackage[];
     let notDownloadablePackage: AbsolutePathPackage[];
+    let downloadValidator: DownloadValidator = () => true;
 
     const packageDescription = "Test Package";
     const networkSettingsProvider = () => new NetworkSettings(undefined, false);
@@ -45,7 +47,7 @@ suite(`${downloadAndInstallPackages.name}`, () => {
             {
                 url: `${server.baseUrl}/downloadablePackage`,
                 description: packageDescription,
-                installPath: new AbsolutePath(tmpDirPath)
+                installPath: new AbsolutePath(tmpDirPath),
             }];
 
         notDownloadablePackage = <AbsolutePathPackage[]>[
@@ -66,21 +68,16 @@ suite(`${downloadAndInstallPackages.name}`, () => {
     });
 
     suite("If the download and install succeeds", () => {
-        test("The expected files are installs at the specified path", async () => {
-            await downloadAndInstallPackages(downloadablePackage, networkSettingsProvider, eventStream);
+        test("The expected files are installed at the specified path", async () => {
+            await downloadAndInstallPackages(downloadablePackage, networkSettingsProvider, eventStream, downloadValidator);
             for (let elem of testZip.files) {
                 let filePath = path.join(tmpDirPath, elem.path);
                 expect(await util.fileExists(filePath)).to.be.true;
             }
         });
 
         test("install.Lock is present", async () => {
-            await downloadAndInstallPackages(downloadablePackage, networkSettingsProvider, eventStream);
-            for (let elem of testZip.files) {
-                let filePath = path.join(tmpDirPath, elem.path);
-                expect(await util.fileExists(filePath)).to.be.true;
-            }
-
+            await downloadAndInstallPackages(downloadablePackage, networkSettingsProvider, eventStream, downloadValidator);
             expect(await util.fileExists(path.join(tmpDirPath, "install.Lock"))).to.be.true;
         });
 
@@ -94,20 +91,68 @@ suite(`${downloadAndInstallPackages.name}`, () => {
                 new InstallationStart(packageDescription)
             ];
 
-            await downloadAndInstallPackages(downloadablePackage, networkSettingsProvider, eventStream);
-            console.log(eventBus.getEvents());
+            await downloadAndInstallPackages(downloadablePackage, networkSettingsProvider, eventStream, downloadValidator);
+            expect(eventBus.getEvents()).to.be.deep.equal(eventsSequence);
+        });
+
+        test("If the download validation fails for the first time and passed second time, the correct events are logged", async() => {
+            let count = 1;
+            let downloadValidator = () => {
+                if (count > 1) {
+                    return true; // fail the first time and then pass the subsequent times
+                }
+
+                count++;
+                return false;
+            };
+
+            let eventsSequence = [
+                new PackageInstallStart(),
+                new DownloadStart(packageDescription),
+                new DownloadSizeObtained(testZip.size),
+                new DownloadProgress(100, packageDescription),
+                new DownloadSuccess(' Done!'),
+                new IntegrityCheckFailure(packageDescription, downloadablePackage[0].url, true),
+                new DownloadStart(packageDescription),
+                new DownloadSizeObtained(testZip.size),
+                new DownloadProgress(100, packageDescription),
+                new DownloadSuccess(' Done!'),
+                new InstallationStart(packageDescription)
+            ];
+
+            await downloadAndInstallPackages(downloadablePackage, networkSettingsProvider, eventStream, downloadValidator);
             expect(eventBus.getEvents()).to.be.deep.equal(eventsSequence);
         });
     });
 
     suite("If the download and install fails", () => {
+        test("If the download succeeds but the validation fails, events are logged", async() => {
+            let downloadValidator = () => false;
+            let eventsSequence = [
+                new PackageInstallStart(),
+                new DownloadStart(packageDescription),
+                new DownloadSizeObtained(testZip.size),
+                new DownloadProgress(100, packageDescription),
+                new DownloadSuccess(' Done!'),
+                new IntegrityCheckFailure(packageDescription, downloadablePackage[0].url, true),
+                new DownloadStart(packageDescription),
+                new DownloadSizeObtained(testZip.size),
+                new DownloadProgress(100, packageDescription),
+                new DownloadSuccess(' Done!'),
+                new IntegrityCheckFailure(packageDescription, downloadablePackage[0].url, false),
+            ];
+
+            await downloadAndInstallPackages(downloadablePackage, networkSettingsProvider, eventStream, downloadValidator);
+            expect(eventBus.getEvents()).to.be.deep.equal(eventsSequence);
+        });
+
         test("Throws an exception when the download fails", async () => {
-            await downloadAndInstallPackages(notDownloadablePackage, networkSettingsProvider, eventStream).should.be.rejected;
+            await downloadAndInstallPackages(notDownloadablePackage, networkSettingsProvider, eventStream, downloadValidator).should.be.rejected;
         });
 
         test("install.Lock is not present when the download fails", async () => {
             try {
-                await downloadAndInstallPackages(notDownloadablePackage, networkSettingsProvider, eventStream);
+                await downloadAndInstallPackages(notDownloadablePackage, networkSettingsProvider, eventStream, downloadValidator);
             } 
             catch (error) {
                 expect(await util.fileExists(path.join(tmpDirPath, "install.Lock"))).to.be.false;

Original file line number	Diff line number	Diff line change
`@@ -12,6 +12,7 @@ import { downloadAndInstallPackages } from '../packageManager/downloadAndInstall`
`12`	`12`	`import { DownloadFile } from '../packageManager/FileDownloader';`
`13`	`13`	`import { getRuntimeDependenciesPackages } from '../tools/RuntimeDependencyPackageUtils';`
`14`	`14`	`import { getAbsolutePathPackagesToInstall } from '../packageManager/getAbsolutePathPackagesToInstall';`
	`15`	`+import { isValidDownload } from '../packageManager/isValidDownload';`
`15`	`16`
`16`	`17`	`export class OmnisharpDownloader {`
`17`	`18`
`@@ -30,7 +31,7 @@ export class OmnisharpDownloader {`
`30`	`31`	`if (packagesToInstall && packagesToInstall.length > 0) {`
`31`	`32`	this.eventStream.post(new PackageInstallation(`OmniSharp Version = ${version}`));
`32`	`33`	`this.eventStream.post(new LogPlatformInfo(this.platformInfo));`
`33`		`- await downloadAndInstallPackages(packagesToInstall, this.networkSettingsProvider, this.eventStream);`
	`34`	`+ await downloadAndInstallPackages(packagesToInstall, this.networkSettingsProvider, this.eventStream, isValidDownload);`
`34`	`35`	`this.eventStream.post(new InstallationSuccess());`
`35`	`36`	`}`
`36`	`37`	`}`