From e3e1ae92b2b08dab123ebd830933cd08f55f83e8 Mon Sep 17 00:00:00 2001 From: Kim Santiago <31145923+kisantia@users.noreply.github.com> Date: Thu, 17 Dec 2020 10:28:42 -0800 Subject: [PATCH] Fix unstable data-workspace tests (#13824) * stub file existing validation * add error message * change back to calling dialog.validate() * move tests to separate dialogbase file and add more error message validation --- .../data-workspace/src/dialogs/dialogBase.ts | 12 ++-- .../src/dialogs/openExistingDialog.ts | 18 +++-- .../src/test/dialogs/dialogBase.test.ts | 69 +++++++++++++++++++ .../src/test/dialogs/newProjectDialog.test.ts | 34 ++------- .../test/dialogs/openExistingDialog.test.ts | 27 -------- 5 files changed, 92 insertions(+), 68 deletions(-) create mode 100644 extensions/data-workspace/src/test/dialogs/dialogBase.test.ts diff --git a/extensions/data-workspace/src/dialogs/dialogBase.ts b/extensions/data-workspace/src/dialogs/dialogBase.ts index 3d8bb2f7c9..a807d3d4ed 100644 --- a/extensions/data-workspace/src/dialogs/dialogBase.ts +++ b/extensions/data-workspace/src/dialogs/dialogBase.ts @@ -36,9 +36,7 @@ export abstract class DialogBase { protected abstract initialize(view: azdata.ModelView): Promise; - protected async validate(): Promise { - return Promise.resolve(true); - } + abstract validate(): Promise; public async open(): Promise { const tab = azdata.window.createTab(''); @@ -77,6 +75,10 @@ export abstract class DialogBase { }; } + public getErrorMessage(): azdata.window.DialogMessage { + return this._dialogObject.message; + } + protected createHorizontalContainer(view: azdata.ModelView, items: azdata.Component[]): azdata.FlexContainer { return view.modelBuilder.flexContainer().withItems(items, { CSSStyles: { 'margin-right': '5px', 'margin-bottom': '10px' } }).withLayout({ flexFlow: 'row', alignItems: 'center' }).component(); } @@ -159,7 +161,7 @@ export abstract class DialogBase { } } - protected async validateNewWorkspace(sameFolderAsNewProject: boolean): Promise { + public async validateNewWorkspace(sameFolderAsNewProject: boolean): Promise { // workspace file should end in .code-workspace const workspaceValid = this.workspaceInputBox!.value!.endsWith(constants.WorkspaceFileExtension); if (!workspaceValid) { @@ -171,7 +173,7 @@ export abstract class DialogBase { if (!sameFolderAsNewProject) { const workspaceParentDirectoryExists = await directoryExist(path.dirname(this.workspaceInputBox!.value!)); if (!workspaceParentDirectoryExists) { - this.showErrorMessage(constants.WorkspaceParentDirectoryNotExistError(this.workspaceInputBox!.value!)); + this.showErrorMessage(constants.WorkspaceParentDirectoryNotExistError(path.dirname(this.workspaceInputBox!.value!))); return false; } } diff --git a/extensions/data-workspace/src/dialogs/openExistingDialog.ts b/extensions/data-workspace/src/dialogs/openExistingDialog.ts index adac9632ba..b48d73a328 100644 --- a/extensions/data-workspace/src/dialogs/openExistingDialog.ts +++ b/extensions/data-workspace/src/dialogs/openExistingDialog.ts @@ -37,9 +37,7 @@ export class OpenExistingDialog extends DialogBase { try { // the selected location should be an existing directory if (this._targetTypeRadioCardGroup?.selectedCardId === constants.Project) { - const fileExists = await fileExist(this._projectFile); - if (!fileExists) { - this.showErrorMessage(constants.FileNotExistError(constants.Project.toLowerCase(), this._projectFile)); + if (!await this.validateFile(this._projectFile, constants.Project.toLowerCase())) { return false; } @@ -47,9 +45,7 @@ export class OpenExistingDialog extends DialogBase { return false; } } else if (this._targetTypeRadioCardGroup?.selectedCardId === constants.Workspace) { - const fileExists = await fileExist(this._workspaceFile); - if (!fileExists) { - this.showErrorMessage(constants.FileNotExistError(constants.Workspace.toLowerCase(), this._workspaceFile)); + if (!await this.validateFile(this._workspaceFile, constants.Workspace.toLowerCase())) { return false; } } @@ -62,6 +58,16 @@ export class OpenExistingDialog extends DialogBase { } } + public async validateFile(file: string, fileType: string) { + const fileExists = await fileExist(file); + if (!fileExists) { + this.showErrorMessage(constants.FileNotExistError(fileType, file)); + return false; + } + + return true; + } + async onComplete(): Promise { try { if (this._targetTypeRadioCardGroup?.selectedCardId === constants.Workspace) { diff --git a/extensions/data-workspace/src/test/dialogs/dialogBase.test.ts b/extensions/data-workspace/src/test/dialogs/dialogBase.test.ts new file mode 100644 index 0000000000..a380cf60c5 --- /dev/null +++ b/extensions/data-workspace/src/test/dialogs/dialogBase.test.ts @@ -0,0 +1,69 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the Source EULA. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as should from 'should'; +import * as TypeMoq from 'typemoq'; +import * as os from 'os'; +import * as path from 'path'; +import * as sinon from 'sinon'; +import * as utils from '../../common/utils'; +import * as constants from '../../common/constants'; +import { NewProjectDialog } from '../../dialogs/newProjectDialog'; +import { WorkspaceService } from '../../services/workspaceService'; +import { testProjectType } from '../testUtils'; + +suite('DialogBase - workspace validation', function (): void { + // DialogBase is an abstract class, so we'll just use a NewProjectDialog to test the common base class functions + let dialog: NewProjectDialog; + + this.beforeEach(async () => { + const workspaceServiceMock = TypeMoq.Mock.ofType(); + workspaceServiceMock.setup(x => x.getAllProjectTypes()).returns(() => Promise.resolve([testProjectType])); + + dialog = new NewProjectDialog(workspaceServiceMock.object); + await dialog.open(); + + dialog.model.name = `TestProject_${new Date().getTime()}`; + dialog.model.location = os.tmpdir(); + }); + + this.afterEach(() => { + sinon.restore(); + }); + + test('Should validate new workspace location missing file extension', async function (): Promise { + dialog.workspaceInputBox!.value = 'test'; + should.equal(await dialog.validateNewWorkspace(false), false, 'Validation should fail because workspace does not end in .code-workspace'); + should.equal(dialog.getErrorMessage().text, constants.WorkspaceFileInvalidError(dialog.workspaceInputBox!.value)); + }); + + test('Should validate new workspace location with invalid location', async function (): Promise { + // use invalid folder + dialog.workspaceInputBox!.value = 'invalidLocation/test.code-workspace'; + should.equal(await dialog.validateNewWorkspace(false), false, 'Validation should fail because the folder is invalid'); + should.equal(dialog.getErrorMessage().text, constants.WorkspaceParentDirectoryNotExistError(path.dirname(dialog.workspaceInputBox!.value))); + }); + + test('Should validate new workspace location that already exists', async function (): Promise { + // use already existing workspace + const fileExistStub = sinon.stub(utils, 'fileExist'); + fileExistStub.resolves(true); + const existingWorkspaceFilePath = path.join(os.tmpdir(), `${dialog.model.name}.code-workspace`); + dialog.workspaceInputBox!.value = existingWorkspaceFilePath; + should.equal(await dialog.validateNewWorkspace(false), false, 'Validation should fail because the selected workspace file already exists'); + should.equal(dialog.getErrorMessage().text, constants.WorkspaceFileAlreadyExistsError(existingWorkspaceFilePath)); + }); + + test('Should validate new workspace location that is valid', async function (): Promise { + // same folder as the project should be valid even if the project folder isn't created yet + dialog.workspaceInputBox!.value = path.join(dialog.model.location, dialog.model.name, 'test.code-workspace'); + should.equal(await dialog.validateNewWorkspace(true), true, `Validation should pass if the file location is the same folder as the project. Error was: ${dialog.getErrorMessage()?.text}`); + + // a workspace not in the same folder as the project should also be valid + dialog.workspaceInputBox!.value = path.join(os.tmpdir(), `TestWorkspace_${new Date().getTime()}.code-workspace`); + should.equal(await dialog.validateNewWorkspace(false), true, `Validation should pass because the parent directory exists, workspace filepath is unique, and the file extension is correct. Error was: ${dialog.getErrorMessage()?.text}`); + }); +}); + diff --git a/extensions/data-workspace/src/test/dialogs/newProjectDialog.test.ts b/extensions/data-workspace/src/test/dialogs/newProjectDialog.test.ts index 491b266bf9..ec0c395098 100644 --- a/extensions/data-workspace/src/test/dialogs/newProjectDialog.test.ts +++ b/extensions/data-workspace/src/test/dialogs/newProjectDialog.test.ts @@ -15,6 +15,10 @@ import { WorkspaceService } from '../../services/workspaceService'; import { testProjectType } from '../testUtils'; suite('New Project Dialog', function (): void { + this.afterEach(() => { + sinon.restore(); + }); + test('Should validate project location', async function (): Promise { const workspaceServiceMock = TypeMoq.Mock.ofType(); workspaceServiceMock.setup(x => x.getAllProjectTypes()).returns(() => Promise.resolve([testProjectType])); @@ -38,36 +42,6 @@ suite('New Project Dialog', function (): void { should.equal(await dialog.validate(), true, 'Validation should pass because name is unique and parent directory exists'); }); - test('Should validate new workspace location @UNSTABLE@', async function (): Promise { - const workspaceServiceMock = TypeMoq.Mock.ofType(); - workspaceServiceMock.setup(x => x.getAllProjectTypes()).returns(() => Promise.resolve([testProjectType])); - - const dialog = new NewProjectDialog(workspaceServiceMock.object); - await dialog.open(); - - dialog.model.name = `TestProject_${new Date().getTime()}`; - dialog.model.location = os.tmpdir(); - dialog.workspaceInputBox!.value = 'test'; - should.equal(await dialog.validate(), false, 'Validation should fail because workspace does not end in .code-workspace'); - - // use invalid folder - dialog.workspaceInputBox!.value = 'invalidLocation/test.code-workspace'; - should.equal(await dialog.validate(), false, 'Validation should fail because the folder is invalid'); - - // use already existing workspace - const existingWorkspaceFilePath = path.join(os.tmpdir(), `${dialog.model.name}.code-workspace`); - await fs.writeFile(existingWorkspaceFilePath, ''); - dialog.workspaceInputBox!.value = existingWorkspaceFilePath; - should.equal(await dialog.validate(), false, 'Validation should fail because the selected workspace file already exists'); - - // same folder as the project should be valid even if the project folder isn't created yet - dialog.workspaceInputBox!.value = path.join(dialog.model.location, dialog.model.name, 'test.code-workspace'); - should.equal(await dialog.validate(), true, 'Validation should pass if the file location is the same folder as the project'); - - // change workspace name to something that should pass - dialog.workspaceInputBox!.value = path.join(os.tmpdir(), `TestWorkspace_${new Date().getTime()}.code-workspace`); - should.equal(await dialog.validate(), true, 'Validation should pass because the parent directory exists, workspace filepath is unique, and the file extension is correct'); - }); test('Should validate workspace in onComplete', async function (): Promise { const workspaceServiceMock = TypeMoq.Mock.ofType(); diff --git a/extensions/data-workspace/src/test/dialogs/openExistingDialog.test.ts b/extensions/data-workspace/src/test/dialogs/openExistingDialog.test.ts index 8bf936f311..0bad06d985 100644 --- a/extensions/data-workspace/src/test/dialogs/openExistingDialog.test.ts +++ b/extensions/data-workspace/src/test/dialogs/openExistingDialog.test.ts @@ -7,8 +7,6 @@ import * as should from 'should'; import * as TypeMoq from 'typemoq'; import * as sinon from 'sinon'; import * as vscode from 'vscode'; -import * as os from 'os'; -import * as path from 'path'; import * as constants from '../../common/constants'; import { promises as fs } from 'fs'; import { WorkspaceService } from '../../services/workspaceService'; @@ -53,31 +51,6 @@ suite('Open Existing Dialog', function (): void { should.equal(await dialog.validate(), true, 'Validation pass because workspace file exists'); }); - test('Should validate new workspace location @UNSTABLE@', async function (): Promise { - const workspaceServiceMock = TypeMoq.Mock.ofType(); - workspaceServiceMock.setup(x => x.getAllProjectTypes()).returns(() => Promise.resolve([testProjectType])); - - const dialog = new OpenExistingDialog(workspaceServiceMock.object, mockExtensionContext.object); - await dialog.open(); - - dialog._projectFile = await createProjectFile('testproj'); - dialog.workspaceInputBox!.value = 'test'; - should.equal(await dialog.validate(), false, 'Validation should fail because workspace does not end in code-workspace'); - - // use invalid folder - dialog.workspaceInputBox!.value = 'invalidLocation/test.code-workspace'; - should.equal(await dialog.validate(), false, 'Validation should fail because the folder is invalid'); - - // use already existing workspace - const existingWorkspaceFilePath = path.join(os.tmpdir(), `test.code-workspace`); - await fs.writeFile(existingWorkspaceFilePath, ''); - dialog.workspaceInputBox!.value = existingWorkspaceFilePath; - should.equal(await dialog.validate(), false, 'Validation should fail because the selected workspace file already exists'); - - // change workspace name to something that should pass - dialog.workspaceInputBox!.value = path.join(os.tmpdir(), `TestWorkspace_${new Date().getTime()}.code-workspace`); - should.equal(await dialog.validate(), true, 'Validation should pass because the parent directory exists, workspace filepath is unique, and the file extension is correct'); - }); test('Should validate workspace in onComplete when opening project', async function (): Promise { const workspaceServiceMock = TypeMoq.Mock.ofType();