From 958f32846dd117843051bc7d05eaa9d385666e73 Mon Sep 17 00:00:00 2001 From: Akshita Agarwal Date: Wed, 9 May 2018 17:42:52 -0700 Subject: [PATCH 1/4] Clean up test --- test/featureTests/testAssets/testAssets.ts | 10 --- .../InformationMessageObserver.test.ts | 90 ++++++++----------- test/unitTests/options.test.ts | 44 ++------- test/unitTests/testAssets/Fakes.ts | 50 +++++++---- 4 files changed, 78 insertions(+), 116 deletions(-) diff --git a/test/featureTests/testAssets/testAssets.ts b/test/featureTests/testAssets/testAssets.ts index 72c3921b80..64ec562ae6 100644 --- a/test/featureTests/testAssets/testAssets.ts +++ b/test/featureTests/testAssets/testAssets.ts @@ -7,19 +7,9 @@ import { EventStream } from "../../../src/EventStream"; import { PlatformInformation } from "../../../src/platform"; import { OmnisharpDownloader } from "../../../src/omnisharp/OmnisharpDownloader"; -import { getFakeVsCode, getNullWorkspaceConfiguration } from "../../unitTests/testAssets/Fakes"; -import { Uri } from "../../../src/vscodeAdapter"; import NetworkSettings from "../../../src/NetworkSettings"; - export function GetTestOmnisharpDownloader(sink: EventStream, platformInfo: PlatformInformation): OmnisharpDownloader { - let vscode = getFakeVsCode(); - vscode.workspace.getConfiguration = (section?: string, resource?: Uri) => { - return { - ...getNullWorkspaceConfiguration(), - }; - }; - return new OmnisharpDownloader(() => new NetworkSettings(undefined, undefined), sink, testPackageJSON, platformInfo); } diff --git a/test/unitTests/logging/InformationMessageObserver.test.ts b/test/unitTests/logging/InformationMessageObserver.test.ts index 849c082bd6..59e166affe 100644 --- a/test/unitTests/logging/InformationMessageObserver.test.ts +++ b/test/unitTests/logging/InformationMessageObserver.test.ts @@ -5,8 +5,7 @@ import { InformationMessageObserver } from '../../../src/observers/InformationMessageObserver'; import { use as chaiUse, expect, should } from 'chai'; -import { vscode, Uri } from '../../../src/vscodeAdapter'; -import { getFakeVsCode, getNullWorkspaceConfiguration, getUnresolvedDependenices } from '../testAssets/Fakes'; +import { getUnresolvedDependenices, updateConfig, getVSCodeWithConfig } from '../testAssets/Fakes'; import { Observable } from 'rxjs/Observable'; import 'rxjs/add/observable/fromPromise'; import 'rxjs/add/operator/timeout'; @@ -23,36 +22,12 @@ suite("InformationMessageObserver", () => { let commandDone = new Promise(resolve => { signalCommandDone = () => { resolve(); }; }); - let vscode: vscode = getFakeVsCode(); + let vscode = getVsCode(); let infoMessage: string; let relativePath: string; let invokedCommand: string; let observer: InformationMessageObserver = new InformationMessageObserver(vscode); - vscode.window.showInformationMessage = async (message: string, ...items: string[]) => { - infoMessage = message; - return new Promise(resolve => { - doClickCancel = () => { - resolve(undefined); - }; - - doClickOk = () => { - resolve(message); - }; - }); - }; - - vscode.commands.executeCommand = (command: string, ...rest: any[]) => { - invokedCommand = command; - signalCommandDone(); - return undefined; - }; - - vscode.workspace.asRelativePath = (pathOrUri?: string, includeWorspaceFolder?: boolean) => { - relativePath = pathOrUri; - return relativePath; - }; - setup(() => { infoMessage = undefined; relativePath = undefined; @@ -66,34 +41,16 @@ suite("InformationMessageObserver", () => { let event = getUnresolvedDependenices("someFile"); suite('Suppress Dotnet Restore Notification is true', () => { - setup(() => { - vscode.workspace.getConfiguration = (section?: string, resource?: Uri) => { - return { - ...getNullWorkspaceConfiguration(), - get: (section: string) => { - return true;// suppress the restore information - } - }; - }; - }); + setup(() => updateConfig(vscode, 'csharp', 'suppressDotnetRestoreNotification', true)); - test('The information message is not shown', () => { + test('The information message is not shown', () => { observer.post(event); expect(infoMessage).to.be.undefined; }); }); - + suite('Suppress Dotnet Restore Notification is false', () => { - setup(() => { - vscode.workspace.getConfiguration = (section?: string, resource?: Uri) => { - return { - ...getNullWorkspaceConfiguration(), - get: (section: string) => { - return false; // do not suppress the restore info - } - }; - }; - }); + setup(() => updateConfig(vscode, 'csharp', 'suppressDotnetRestoreNotification', false)); test('The information message is shown', async () => { observer.post(event); @@ -103,20 +60,49 @@ suite("InformationMessageObserver", () => { await commandDone; expect(invokedCommand).to.be.equal('dotnet.restore'); }); - + test('Given an information message if the user clicks Restore, the command is executed', async () => { observer.post(event); doClickOk(); await commandDone; expect(invokedCommand).to.be.equal('dotnet.restore'); }); - + test('Given an information message if the user clicks cancel, the command is not executed', async () => { observer.post(event); doClickCancel(); await expect(Observable.fromPromise(commandDone).timeout(1).toPromise()).to.be.rejected; expect(invokedCommand).to.be.undefined; }); - }); + }); }); + + function getVsCode() { + let vscode = getVSCodeWithConfig(); + vscode.window.showInformationMessage = async (message: string, ...items: string[]) => { + infoMessage = message; + return new Promise(resolve => { + doClickCancel = () => { + resolve(undefined); + }; + + doClickOk = () => { + resolve(message); + }; + }); + }; + + vscode.commands.executeCommand = (command: string, ...rest: any[]) => { + invokedCommand = command; + signalCommandDone(); + return undefined; + }; + + vscode.workspace.asRelativePath = (pathOrUri?: string, includeWorspaceFolder?: boolean) => { + relativePath = pathOrUri; + return relativePath; + }; + + return vscode; + } }); \ No newline at end of file diff --git a/test/unitTests/options.test.ts b/test/unitTests/options.test.ts index d9303acb5c..fe0bb84bce 100644 --- a/test/unitTests/options.test.ts +++ b/test/unitTests/options.test.ts @@ -5,41 +5,15 @@ import { should, expect } from 'chai'; import { Options } from '../../src/omnisharp/options'; -import { getFakeVsCode, getWorkspaceConfiguration } from './testAssets/Fakes'; -import { WorkspaceConfiguration } from '../../src/vscodeAdapter'; - -function getVSCode(omnisharpConfig?: WorkspaceConfiguration, csharpConfig?: WorkspaceConfiguration) { - const vscode = getFakeVsCode(); - - const _omnisharpConfig = omnisharpConfig || getWorkspaceConfiguration(); - const _csharpConfig = csharpConfig || getWorkspaceConfiguration(); - - vscode.workspace.getConfiguration = (section?, resource?) => - { - if (section === 'omnisharp') - { - return _omnisharpConfig; - } - - if (section === 'csharp') - { - return _csharpConfig; - } - - return undefined; - }; - - return vscode; -} +import { getWorkspaceConfiguration, getVSCodeWithConfig } from './testAssets/Fakes'; suite("Options tests", () => { suiteSetup(() => should()); test('Verify defaults', () => { - const vscode = getVSCode(); + const vscode = getVSCodeWithConfig(); const options = Options.Read(vscode); - expect(options.path).to.be.null; options.useGlobalMono.should.equal("auto"); options.waitForDebugger.should.equal(false); @@ -59,7 +33,7 @@ suite("Options tests", () => { { const omnisharpConfig = getWorkspaceConfiguration(); omnisharpConfig.update('loggingLevel', "verbose"); - const vscode = getVSCode(omnisharpConfig); + const vscode = getVSCodeWithConfig(omnisharpConfig); const options = Options.Read(vscode); @@ -70,7 +44,7 @@ suite("Options tests", () => { { const omnisharpConfig = getWorkspaceConfiguration(); omnisharpConfig.update('useMono', true); - const vscode = getVSCode(omnisharpConfig); + const vscode = getVSCodeWithConfig(omnisharpConfig); const options = Options.Read(vscode); @@ -81,7 +55,7 @@ suite("Options tests", () => { { const omnisharpConfig = getWorkspaceConfiguration(); omnisharpConfig.update('useMono', false); - const vscode = getVSCode(omnisharpConfig); + const vscode = getVSCodeWithConfig(omnisharpConfig); const options = Options.Read(vscode); @@ -92,7 +66,7 @@ suite("Options tests", () => { { const csharpConfig = getWorkspaceConfiguration(); csharpConfig.update('omnisharpUsesMono', true); - const vscode = getVSCode(undefined, csharpConfig); + const vscode = getVSCodeWithConfig(undefined, csharpConfig); const options = Options.Read(vscode); @@ -103,7 +77,7 @@ suite("Options tests", () => { { const csharpConfig = getWorkspaceConfiguration(); csharpConfig.update('omnisharpUsesMono', false); - const vscode = getVSCode(undefined, csharpConfig); + const vscode = getVSCodeWithConfig(undefined, csharpConfig); const options = Options.Read(vscode); @@ -114,7 +88,7 @@ suite("Options tests", () => { { const csharpConfig = getWorkspaceConfiguration(); csharpConfig.update('omnisharp', 'OldPath'); - const vscode = getVSCode(undefined, csharpConfig); + const vscode = getVSCodeWithConfig(undefined, csharpConfig); const options = Options.Read(vscode); @@ -127,7 +101,7 @@ suite("Options tests", () => { omnisharpConfig.update('path', 'NewPath'); const csharpConfig = getWorkspaceConfiguration(); csharpConfig.update('omnisharp', 'OldPath'); - const vscode = getVSCode(omnisharpConfig, csharpConfig); + const vscode = getVSCodeWithConfig(omnisharpConfig, csharpConfig); const options = Options.Read(vscode); diff --git a/test/unitTests/testAssets/Fakes.ts b/test/unitTests/testAssets/Fakes.ts index 93efb5f441..eb6671d9b5 100644 --- a/test/unitTests/testAssets/Fakes.ts +++ b/test/unitTests/testAssets/Fakes.ts @@ -5,7 +5,7 @@ import * as vscode from '../../../src/vscodeAdapter'; import * as protocol from '../../../src/omnisharp/protocol'; -import { DocumentSelector, MessageItem, TextDocument, Uri, GlobPattern } from '../../../src/vscodeAdapter'; +import { DocumentSelector, MessageItem, TextDocument, Uri, GlobPattern, WorkspaceConfiguration } from '../../../src/vscodeAdapter'; import { ITelemetryReporter } from '../../../src/observers/TelemetryObserver'; import { MSBuildDiagnosticsMessage } from '../../../src/omnisharp/protocol'; import { OmnisharpServerMsBuildProjectDiagnostics, OmnisharpServerOnError, OmnisharpServerUnresolvedDependencies, WorkspaceInformationUpdated } from '../../../src/omnisharp/loggingEvents'; @@ -31,23 +31,6 @@ export const getNullTelemetryReporter = (): ITelemetryReporter => { return reporter; }; -export const getNullWorkspaceConfiguration = (): vscode.WorkspaceConfiguration => { - let configuration: vscode.WorkspaceConfiguration = { - get: (section: string): T | undefined => { - return undefined; - }, - has: (section: string) => { return undefined; }, - inspect: () => { - return { - key: undefined - }; - }, - update: async () => { return Promise.resolve(); } - }; - - return configuration; -}; - export const getWorkspaceConfiguration = (): vscode.WorkspaceConfiguration => { let values: { [key: string]: any } = {}; @@ -164,4 +147,33 @@ export function getWorkspaceInformationUpdated(msbuild: protocol.MsBuildWorkspac }; return new WorkspaceInformationUpdated(a); -} \ No newline at end of file +} + +export function getVSCodeWithConfig(omnisharpConfig?: WorkspaceConfiguration, csharpConfig?: WorkspaceConfiguration) { + const vscode = getFakeVsCode(); + + const _omnisharpConfig = omnisharpConfig || getWorkspaceConfiguration(); + const _csharpConfig = csharpConfig || getWorkspaceConfiguration(); + + vscode.workspace.getConfiguration = (section?, resource?) => + { + if (section === 'omnisharp') + { + return _omnisharpConfig; + } + + if (section === 'csharp') + { + return _csharpConfig; + } + + return undefined; + }; + + return vscode; +} + +export function updateConfig(vscode: vscode.vscode, section: string, config: string, value: any) { + let workspaceConfig = vscode.workspace.getConfiguration(section); + workspaceConfig.update(config, value); +} From 5c321a898e5e6258fdcf1ad3fcc5075e889ccc23 Mon Sep 17 00:00:00 2001 From: Akshita Agarwal Date: Wed, 9 May 2018 17:53:31 -0700 Subject: [PATCH 2/4] clean up option test --- test/unitTests/options.test.ts | 46 ++++++++++++++-------------------- 1 file changed, 19 insertions(+), 27 deletions(-) diff --git a/test/unitTests/options.test.ts b/test/unitTests/options.test.ts index fe0bb84bce..cae363f161 100644 --- a/test/unitTests/options.test.ts +++ b/test/unitTests/options.test.ts @@ -5,7 +5,7 @@ import { should, expect } from 'chai'; import { Options } from '../../src/omnisharp/options'; -import { getWorkspaceConfiguration, getVSCodeWithConfig } from './testAssets/Fakes'; +import { getVSCodeWithConfig, updateConfig } from './testAssets/Fakes'; suite("Options tests", () => { suiteSetup(() => should()); @@ -31,9 +31,8 @@ suite("Options tests", () => { test('BACK-COMPAT: "omnisharp.loggingLevel": "verbose" == "omnisharp.loggingLevel": "debug"', () => { - const omnisharpConfig = getWorkspaceConfiguration(); - omnisharpConfig.update('loggingLevel', "verbose"); - const vscode = getVSCodeWithConfig(omnisharpConfig); + const vscode = getVSCodeWithConfig(); + updateConfig(vscode, 'omnisharp', 'loggingLevel', "verbose"); const options = Options.Read(vscode); @@ -41,11 +40,10 @@ suite("Options tests", () => { }); test('BACK-COMPAT: "omnisharp.useMono": true == "omnisharp.useGlobalMono": "always"', () => - { - const omnisharpConfig = getWorkspaceConfiguration(); - omnisharpConfig.update('useMono', true); - const vscode = getVSCodeWithConfig(omnisharpConfig); - + { + const vscode = getVSCodeWithConfig(); + updateConfig(vscode, 'omnisharp', 'useMono', true); + const options = Options.Read(vscode); options.useGlobalMono.should.equal("always"); @@ -53,10 +51,9 @@ suite("Options tests", () => { test('BACK-COMPAT: "omnisharp.useMono": false == "omnisharp.useGlobalMono": "auto"', () => { - const omnisharpConfig = getWorkspaceConfiguration(); - omnisharpConfig.update('useMono', false); - const vscode = getVSCodeWithConfig(omnisharpConfig); - + const vscode = getVSCodeWithConfig(); + updateConfig(vscode, 'omnisharp', 'useMono', false); + const options = Options.Read(vscode); options.useGlobalMono.should.equal("auto"); @@ -64,9 +61,8 @@ suite("Options tests", () => { test('BACK-COMPAT: "csharp.omnisharpUsesMono": true == "omnisharp.useGlobalMono": "always"', () => { - const csharpConfig = getWorkspaceConfiguration(); - csharpConfig.update('omnisharpUsesMono', true); - const vscode = getVSCodeWithConfig(undefined, csharpConfig); + const vscode = getVSCodeWithConfig(); + updateConfig(vscode, 'csharp', 'omnisharpUsesMono', true); const options = Options.Read(vscode); @@ -75,9 +71,8 @@ suite("Options tests", () => { test('BACK-COMPAT: "csharp.omnisharpUsesMono": false == "omnisharp.useGlobalMono": "auto"', () => { - const csharpConfig = getWorkspaceConfiguration(); - csharpConfig.update('omnisharpUsesMono', false); - const vscode = getVSCodeWithConfig(undefined, csharpConfig); + const vscode = getVSCodeWithConfig(); + updateConfig(vscode, 'csharp', 'omnisharpUsesMono', false); const options = Options.Read(vscode); @@ -86,9 +81,8 @@ suite("Options tests", () => { test('BACK-COMPAT: "csharp.omnisharp" is used if it is set and "omnisharp.path" is not', () => { - const csharpConfig = getWorkspaceConfiguration(); - csharpConfig.update('omnisharp', 'OldPath'); - const vscode = getVSCodeWithConfig(undefined, csharpConfig); + const vscode = getVSCodeWithConfig(); + updateConfig(vscode, 'csharp', 'omnisharp', 'OldPath'); const options = Options.Read(vscode); @@ -97,11 +91,9 @@ suite("Options tests", () => { test('BACK-COMPAT: "csharp.omnisharp" is not used if "omnisharp.path" is set', () => { - const omnisharpConfig = getWorkspaceConfiguration(); - omnisharpConfig.update('path', 'NewPath'); - const csharpConfig = getWorkspaceConfiguration(); - csharpConfig.update('omnisharp', 'OldPath'); - const vscode = getVSCodeWithConfig(omnisharpConfig, csharpConfig); + const vscode = getVSCodeWithConfig(); + updateConfig(vscode, 'omnisharp', 'path', 'NewPath'); + updateConfig(vscode, 'csharp', 'omnisharp', 'OldPath'); const options = Options.Read(vscode); From ab5e8a308fcdd66495c7639b0539695a1fb55809 Mon Sep 17 00:00:00 2001 From: Akshita Agarwal Date: Wed, 9 May 2018 17:55:32 -0700 Subject: [PATCH 3/4] change the function signature --- test/unitTests/testAssets/Fakes.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/unitTests/testAssets/Fakes.ts b/test/unitTests/testAssets/Fakes.ts index eb6671d9b5..2d363be1c7 100644 --- a/test/unitTests/testAssets/Fakes.ts +++ b/test/unitTests/testAssets/Fakes.ts @@ -149,11 +149,11 @@ export function getWorkspaceInformationUpdated(msbuild: protocol.MsBuildWorkspac return new WorkspaceInformationUpdated(a); } -export function getVSCodeWithConfig(omnisharpConfig?: WorkspaceConfiguration, csharpConfig?: WorkspaceConfiguration) { +export function getVSCodeWithConfig() { const vscode = getFakeVsCode(); - const _omnisharpConfig = omnisharpConfig || getWorkspaceConfiguration(); - const _csharpConfig = csharpConfig || getWorkspaceConfiguration(); + const _omnisharpConfig = getWorkspaceConfiguration(); + const _csharpConfig = getWorkspaceConfiguration(); vscode.workspace.getConfiguration = (section?, resource?) => { From 6e03edf857601cdd17d39f9ad1e88472da16d482 Mon Sep 17 00:00:00 2001 From: Akshita Agarwal Date: Wed, 9 May 2018 18:11:37 -0700 Subject: [PATCH 4/4] clean up --- .../InformationMessageObserver.test.ts | 73 ++++++++++--------- test/unitTests/testAssets/Fakes.ts | 2 +- 2 files changed, 40 insertions(+), 35 deletions(-) diff --git a/test/unitTests/logging/InformationMessageObserver.test.ts b/test/unitTests/logging/InformationMessageObserver.test.ts index 59e166affe..a3b33de86b 100644 --- a/test/unitTests/logging/InformationMessageObserver.test.ts +++ b/test/unitTests/logging/InformationMessageObserver.test.ts @@ -37,42 +37,47 @@ suite("InformationMessageObserver", () => { }); }); - suite('OmnisharpServerUnresolvedDependencies', () => { - let event = getUnresolvedDependenices("someFile"); - - suite('Suppress Dotnet Restore Notification is true', () => { - setup(() => updateConfig(vscode, 'csharp', 'suppressDotnetRestoreNotification', true)); - - test('The information message is not shown', () => { - observer.post(event); - expect(infoMessage).to.be.undefined; - }); - }); - - suite('Suppress Dotnet Restore Notification is false', () => { - setup(() => updateConfig(vscode, 'csharp', 'suppressDotnetRestoreNotification', false)); - - test('The information message is shown', async () => { - observer.post(event); - expect(relativePath).to.not.be.empty; - expect(infoMessage).to.not.be.empty; - doClickOk(); - await commandDone; - expect(invokedCommand).to.be.equal('dotnet.restore'); - }); - - test('Given an information message if the user clicks Restore, the command is executed', async () => { - observer.post(event); - doClickOk(); - await commandDone; - expect(invokedCommand).to.be.equal('dotnet.restore'); + [ + { + event: getUnresolvedDependenices("someFile"), + expectedCommand: "dotnet.restore" + } + ].forEach((elem) => { + suite(elem.event.constructor.name, () => { + suite('Suppress Dotnet Restore Notification is true', () => { + setup(() => updateConfig(vscode, 'csharp', 'suppressDotnetRestoreNotification', true)); + + test('The information message is not shown', () => { + observer.post(elem.event); + expect(infoMessage).to.be.undefined; + }); }); - test('Given an information message if the user clicks cancel, the command is not executed', async () => { - observer.post(event); - doClickCancel(); - await expect(Observable.fromPromise(commandDone).timeout(1).toPromise()).to.be.rejected; - expect(invokedCommand).to.be.undefined; + suite('Suppress Dotnet Restore Notification is false', () => { + setup(() => updateConfig(vscode, 'csharp', 'suppressDotnetRestoreNotification', false)); + + test('The information message is shown', async () => { + observer.post(elem.event); + expect(relativePath).to.not.be.empty; + expect(infoMessage).to.not.be.empty; + doClickOk(); + await commandDone; + expect(invokedCommand).to.be.equal(elem.expectedCommand); + }); + + test('Given an information message if the user clicks Restore, the command is executed', async () => { + observer.post(elem.event); + doClickOk(); + await commandDone; + expect(invokedCommand).to.be.equal(elem.expectedCommand); + }); + + test('Given an information message if the user clicks cancel, the command is not executed', async () => { + observer.post(elem.event); + doClickCancel(); + await expect(Observable.fromPromise(commandDone).timeout(1).toPromise()).to.be.rejected; + expect(invokedCommand).to.be.undefined; + }); }); }); }); diff --git a/test/unitTests/testAssets/Fakes.ts b/test/unitTests/testAssets/Fakes.ts index 2d363be1c7..82bd0e69d0 100644 --- a/test/unitTests/testAssets/Fakes.ts +++ b/test/unitTests/testAssets/Fakes.ts @@ -5,7 +5,7 @@ import * as vscode from '../../../src/vscodeAdapter'; import * as protocol from '../../../src/omnisharp/protocol'; -import { DocumentSelector, MessageItem, TextDocument, Uri, GlobPattern, WorkspaceConfiguration } from '../../../src/vscodeAdapter'; +import { DocumentSelector, MessageItem, TextDocument, Uri, GlobPattern } from '../../../src/vscodeAdapter'; import { ITelemetryReporter } from '../../../src/observers/TelemetryObserver'; import { MSBuildDiagnosticsMessage } from '../../../src/omnisharp/protocol'; import { OmnisharpServerMsBuildProjectDiagnostics, OmnisharpServerOnError, OmnisharpServerUnresolvedDependencies, WorkspaceInformationUpdated } from '../../../src/omnisharp/loggingEvents';