From f02e2a4b72e66a612bcb8fe34f1822d633d6460e Mon Sep 17 00:00:00 2001 From: Arvind Ranasaria Date: Mon, 15 Jun 2020 15:27:42 -0700 Subject: [PATCH] Initial unit tests for class NotebookEditor (#10926) * first few notebookEditor tests * formating fixes * PR feedback * PR feedback * copyright fixes * improve test names and assertion messages * PR feedback * improve test names. * test name change * test name change * remove unneeded cast * remove spurious comment * fix misplaced paranthesis - thanks hygiene checker! * remove unused code * remove deferredPromise * rempve unnecessary mock usage in dispose test * use getContainer for ['parent'] * notebookService needs 9th constructor argument * Add uploading debugging step * remove changes to ci.yml Co-authored-by: Arvind Ranasaria MacPro Co-authored-by: chgagnon --- src/sql/base/common/promise.ts | 2 +- .../notebook/browser/notebookEditor.ts | 7 +- .../browser/MarkdownTextTransformer.test.ts | 4 +- .../test/browser/notebookEditor.test.ts | 173 ++++++++++++++++++ .../browser/testEditorsAndProviders.test.ts | 50 ----- .../contrib/notebook/test/testCommon.ts | 61 ++++++ 6 files changed, 241 insertions(+), 56 deletions(-) create mode 100644 src/sql/workbench/contrib/notebook/test/browser/notebookEditor.test.ts delete mode 100644 src/sql/workbench/contrib/notebook/test/browser/testEditorsAndProviders.test.ts create mode 100644 src/sql/workbench/contrib/notebook/test/testCommon.ts diff --git a/src/sql/base/common/promise.ts b/src/sql/base/common/promise.ts index 4185b4d55a..74fcf82f03 100644 --- a/src/sql/base/common/promise.ts +++ b/src/sql/base/common/promise.ts @@ -35,6 +35,6 @@ export class Deferred implements Promise { } get [Symbol.toStringTag](): string { - return this.toString(); + return this.promise[Symbol.toStringTag]; // symbol tag same as that of underlying promise object } } diff --git a/src/sql/workbench/contrib/notebook/browser/notebookEditor.ts b/src/sql/workbench/contrib/notebook/browser/notebookEditor.ts index f975d47c75..a0fe5df231 100644 --- a/src/sql/workbench/contrib/notebook/browser/notebookEditor.ts +++ b/src/sql/workbench/contrib/notebook/browser/notebookEditor.ts @@ -28,7 +28,7 @@ import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey'; import { IWorkbenchThemeService } from 'vs/workbench/services/themes/common/workbenchThemeService'; import { INotebookModel } from 'sql/workbench/services/notebook/browser/models/modelInterfaces'; import { INotebookFindModel } from 'sql/workbench/contrib/notebook/browser/models/notebookFindModel'; -import { IDisposable, DisposableStore, dispose } from 'vs/base/common/lifecycle'; +import { IDisposable, DisposableStore } from 'vs/base/common/lifecycle'; import { IModelDecorationsChangeAccessor, IModelDeltaDecoration } from 'vs/editor/common/model'; import { NotebookFindDecorations } from 'sql/workbench/contrib/notebook/browser/find/notebookFindDecorations'; import { TimeoutTimer } from 'vs/base/common/async'; @@ -66,12 +66,12 @@ export class NotebookEditor extends BaseEditor implements IFindNotebookControlle ) { super(NotebookEditor.ID, telemetryService, themeService, storageService); this._startSearchingTimer = new TimeoutTimer(); + this._toDispose.add(this._startSearchingTimer); this._actionMap[ACTION_IDS.FIND_NEXT] = this._instantiationService.createInstance(NotebookFindNextAction, this); this._actionMap[ACTION_IDS.FIND_PREVIOUS] = this._instantiationService.createInstance(NotebookFindPreviousAction, this); } public dispose(): void { - dispose(this._startSearchingTimer); this._toDispose.dispose(); } @@ -192,7 +192,7 @@ export class NotebookEditor extends BaseEditor implements IFindNotebookControlle const parentElement = this.getContainer(); - super.setInput(input, options, CancellationToken.None); + await super.setInput(input, options, CancellationToken.None); DOM.clearNode(parentElement); await this.setFindInput(parentElement); if (!input.hasBootstrapped) { @@ -271,6 +271,7 @@ export class NotebookEditor extends BaseEditor implements IFindNotebookControlle } if (this._findCountChangeListener === undefined && this._notebookModel) { this._findCountChangeListener = this.notebookInput.notebookFindModel.onFindCountChange(() => this._updateFinderMatchState()); + this._toDispose.add(this._findCountChangeListener); this.registerModelChanges(); } if (e.isRevealed) { diff --git a/src/sql/workbench/contrib/notebook/test/browser/MarkdownTextTransformer.test.ts b/src/sql/workbench/contrib/notebook/test/browser/MarkdownTextTransformer.test.ts index 9353b61bcb..89997397c3 100644 --- a/src/sql/workbench/contrib/notebook/test/browser/MarkdownTextTransformer.test.ts +++ b/src/sql/workbench/contrib/notebook/test/browser/MarkdownTextTransformer.test.ts @@ -29,7 +29,7 @@ import { TestStorageService } from 'vs/workbench/test/common/workbenchTestServic import { IEnvironmentService } from 'vs/platform/environment/common/environment'; import { IStorageService } from 'vs/platform/storage/common/storage'; import { IEditor } from 'vs/editor/common/editorCommon'; -import { TestNotebookEditor } from 'sql/workbench/contrib/notebook/test/browser/testEditorsAndProviders.test'; +import { TestNotebookEditor } from 'sql/workbench/contrib/notebook/test/testCommon'; @@ -58,7 +58,7 @@ suite('MarkdownTextTransformer', () => { undefined, undefined, undefined, undefined, undefined, undefined, TestEnvironmentService); cellModel = new CellModel(undefined, undefined, mockNotebookService.object); - notebookEditor = new TestNotebookEditor(cellModel.cellGuid, instantiationService); + notebookEditor = new TestNotebookEditor({ cellGuid: cellModel.cellGuid, instantiationService: instantiationService }); markdownTextTransformer = new MarkdownTextTransformer(mockNotebookService.object, cellModel, notebookEditor); mockNotebookService.setup(s => s.findNotebookEditor(TypeMoq.It.isAny())).returns(() => notebookEditor); diff --git a/src/sql/workbench/contrib/notebook/test/browser/notebookEditor.test.ts b/src/sql/workbench/contrib/notebook/test/browser/notebookEditor.test.ts new file mode 100644 index 0000000000..2875762e57 --- /dev/null +++ b/src/sql/workbench/contrib/notebook/test/browser/notebookEditor.test.ts @@ -0,0 +1,173 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the Source EULA. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as assert from 'assert'; +import { QueryTextEditor } from 'sql/workbench/browser/modelComponents/queryTextEditor'; +import { UntitledNotebookInput } from 'sql/workbench/contrib/notebook/browser/models/untitledNotebookInput'; +import { NotebookEditor } from 'sql/workbench/contrib/notebook/browser/notebookEditor'; +import { NBTestQueryManagementService } from 'sql/workbench/contrib/notebook/test/nbTestQueryManagementService'; +import { NotebookModelStub } from 'sql/workbench/contrib/notebook/test/stubs'; +import { TestNotebookEditor } from 'sql/workbench/contrib/notebook/test/testCommon'; +import { ICellModel } from 'sql/workbench/services/notebook/browser/models/modelInterfaces'; +import { INotebookService, NotebookRange } from 'sql/workbench/services/notebook/browser/notebookService'; +import { NotebookService } from 'sql/workbench/services/notebook/browser/notebookServiceImpl'; +import * as dom from 'vs/base/browser/dom'; +import { Emitter } from 'vs/base/common/event'; +import { DisposableStore } from 'vs/base/common/lifecycle'; +import { Schemas } from 'vs/base/common/network'; +import { URI } from 'vs/base/common/uri'; +import { generateUuid } from 'vs/base/common/uuid'; +import { ITextResourceConfigurationService } from 'vs/editor/common/services/textResourceConfigurationService'; +import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; +import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey'; +import { ContextViewService } from 'vs/platform/contextview/browser/contextViewService'; +import { DidInstallExtensionEvent, DidUninstallExtensionEvent, IExtensionManagementService, InstallExtensionEvent } from 'vs/platform/extensionManagement/common/extensionManagement'; +import { IExtensionIdentifier } from 'vs/platform/extensions/common/extensions'; +import { IFileService } from 'vs/platform/files/common/files'; +import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock'; +import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding'; +import { ILayoutService } from 'vs/platform/layout/browser/layoutService'; +import { ILifecycleService } from 'vs/platform/lifecycle/common/lifecycle'; +import { ILogService } from 'vs/platform/log/common/log'; +import { IStorageService } from 'vs/platform/storage/common/storage'; +import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry'; +import { IThemeService } from 'vs/platform/theme/common/themeService'; +import { EditorOptions } from 'vs/workbench/common/editor'; +import { IEditorGroupsService } from 'vs/workbench/services/editor/common/editorGroupsService'; +import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; +import { ExtensionManagementService } from 'vs/workbench/services/extensionManagement/common/extensionManagementService'; +import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions'; +import { WorkbenchThemeService } from 'vs/workbench/services/themes/browser/workbenchThemeService'; +import { IWorkbenchThemeService } from 'vs/workbench/services/themes/common/workbenchThemeService'; +import { UntitledTextEditorInput } from 'vs/workbench/services/untitled/common/untitledTextEditorInput'; +import { IUntitledTextEditorService } from 'vs/workbench/services/untitled/common/untitledTextEditorService'; +import { workbenchInstantiationService } from 'vs/workbench/test/browser/workbenchTestServices'; + + +suite('Test class NotebookEditor', () => { + + let notebookEditor: NotebookEditor; + + const installEvent: Emitter = new Emitter(); + const didInstallEvent = new Emitter(); + const uninstallEvent = new Emitter(); + const didUninstallEvent = new Emitter(); + + const instantiationService = workbenchInstantiationService(); + const workbenchThemeService = instantiationService.createInstance(WorkbenchThemeService); + instantiationService.stub(IWorkbenchThemeService, workbenchThemeService); + + const queryManagementService = new NBTestQueryManagementService(); + + instantiationService.stub(IExtensionManagementService, ExtensionManagementService); + instantiationService.stub(IExtensionManagementService, 'onInstallExtension', installEvent.event); + instantiationService.stub(IExtensionManagementService, 'onDidInstallExtension', didInstallEvent.event); + instantiationService.stub(IExtensionManagementService, 'onUninstallExtension', uninstallEvent.event); + instantiationService.stub(IExtensionManagementService, 'onDidUninstallExtension', didUninstallEvent.event); + + const extensionService = instantiationService.get(IExtensionService); + const notebookService = new NotebookService( + instantiationService.get(ILifecycleService), + instantiationService.get(IStorageService), + extensionService, + instantiationService.get(IExtensionManagementService), + instantiationService, + instantiationService.get(IFileService), + instantiationService.get(ILogService), + queryManagementService, + instantiationService.get(IContextKeyService) + ); + + instantiationService.stub(INotebookService, notebookService); + + const testTitle = 'NotebookEditor.Test-Title'; + const untitledUri = URI.from({ scheme: Schemas.untitled, path: 'NotebookEditor.Test-TestPath' }); + const untitledTextEditorService = instantiationService.get(IUntitledTextEditorService); + const untitledTextInput = instantiationService.createInstance(UntitledTextEditorInput, untitledTextEditorService.create({ associatedResource: untitledUri })); + const untitledNotebookInput = new UntitledNotebookInput( + testTitle, untitledUri, untitledTextInput, + undefined, instantiationService, notebookService, extensionService + ); + + const cellTextEditorGuid = generateUuid(); + const queryTextEditor = new QueryTextEditor( + instantiationService.get(ITelemetryService), + instantiationService, + instantiationService.get(IStorageService), + instantiationService.get(ITextResourceConfigurationService), + instantiationService.get(IThemeService), + instantiationService.get(IEditorGroupsService), + instantiationService.get(IEditorService), + instantiationService.get(IConfigurationService) + ); + const testNotebookEditor = new TestNotebookEditor({ cellGuid: cellTextEditorGuid, editor: queryTextEditor }); + testNotebookEditor.id = untitledNotebookInput.notebookUri.toString(); + testNotebookEditor.model = new NotebookModelStub(); + notebookService.addNotebookEditor(testNotebookEditor); + + setup(async () => { + // Create notebookEditor + notebookEditor = new NotebookEditor( + instantiationService.get(ITelemetryService), + instantiationService.get(IThemeService), + instantiationService, + instantiationService.get(IStorageService), + new ContextViewService(instantiationService.get(ILayoutService)), + instantiationService.get(IKeybindingService), + instantiationService.get(IContextKeyService), + workbenchThemeService, + notebookService + ); + let div = dom.$('div', undefined, dom.$('span', { id: 'demospan' })); + let parentHtmlElement = div.firstChild as HTMLElement; + notebookEditor.create(parentHtmlElement); // adds notebookEditor to new htmlElement as parent + assert.notStrictEqual(notebookEditor['_overlay'], undefined, `The overlay must be defined for notebookEditor once create() has been called on it`); + assert.strictEqual(notebookEditor.getContainer(), parentHtmlElement, 'parent of notebookEditor was not the one that was expected'); + await notebookEditor.setInput(untitledNotebookInput, EditorOptions.create({ pinned: true })); + }); + + test('NotebookEditor: Tests that dispose() disposes all objects in its disposable store', async () => { + let isDisposed = (notebookEditor['_toDispose'])['_isDisposed']; + assert.ok(!isDisposed, 'initially notebookEditor\'s disposable store must not be disposed'); + notebookEditor.dispose(); + isDisposed = (notebookEditor['_toDispose'])['_isDisposed']; + assert.ok(isDisposed, 'notebookEditor\'s disposable store must be disposed'); + }); + + test('NotebookEditor: Tests that getPosition and getLastPosition correctly return the range set by setSelection', async () => { + let currentPosition = notebookEditor.getPosition(); + let lastPosition = notebookEditor.getLastPosition(); + assert.strictEqual(currentPosition, undefined, 'notebookEditor.getPosition() should return an undefined range with no selected range'); + assert.strictEqual(lastPosition, undefined, 'notebookEditor.getLastPosition() should return an undefined range with no previously selected range'); + let selectedRange = new NotebookRange({}, 0, 0, 0, 0); + notebookEditor.setSelection(selectedRange); + lastPosition = notebookEditor.getLastPosition(); + assert.strictEqual(lastPosition, currentPosition, 'notebookEditor.getLastPosition() should return the value that was the \'currentPosition before the most recent range selection'); + currentPosition = notebookEditor.getPosition(); + assert.strictEqual(currentPosition, selectedRange, 'notebookEditor.getPosition() should return the range that was selected'); + selectedRange = new NotebookRange({}, 0, 1, 0, 1); + notebookEditor.setSelection(selectedRange); + lastPosition = notebookEditor.getLastPosition(); + assert.strictEqual(lastPosition, currentPosition, 'notebookEditor.getLastPosition() should return the value that was the \'currentPosition before the most recent range selection'); + currentPosition = notebookEditor.getPosition(); + assert.strictEqual(currentPosition, selectedRange, 'notebookEditor.getPosition() should return the range that was selected'); + }); + + // NotebookEditor-getCellEditor tests. + ['', undefined, null, 'unknown string', /*unknown guid*/generateUuid()].forEach(input => { + test(`NotebookEditor: Negative Test for getCellEditor() - returns undefined for invalid or unknown guid:'${input}'`, async () => { + const result = notebookEditor.getCellEditor(input); + assert.strictEqual(result, undefined, `notebookEditor.getCellEditor() should return undefined when invalid guid:'${input}' is passed in for a notebookEditor of an empty document.`); + }); + }); + + test('NotebookEditor: Positive Test for getCellEditor() - returns a valid text editor object for valid guid input', async () => { + const result = notebookEditor.getCellEditor(cellTextEditorGuid); + assert.strictEqual(result, queryTextEditor, 'notebookEditor.getCellEditor() should return an expected QueryTextEditor when a guid corresponding to that editor is passed in.'); + + }); + +}); + diff --git a/src/sql/workbench/contrib/notebook/test/browser/testEditorsAndProviders.test.ts b/src/sql/workbench/contrib/notebook/test/browser/testEditorsAndProviders.test.ts deleted file mode 100644 index 436ab782eb..0000000000 --- a/src/sql/workbench/contrib/notebook/test/browser/testEditorsAndProviders.test.ts +++ /dev/null @@ -1,50 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the Source EULA. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -import { NotebookEditorStub, CellEditorProviderStub } from 'sql/workbench/contrib/notebook/test/stubs'; -import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; -import { QueryTextEditor } from 'sql/workbench/browser/modelComponents/queryTextEditor'; -import { NullTelemetryService } from 'vs/platform/telemetry/common/telemetryUtils'; -import { TestStorageService } from 'vs/workbench/test/common/workbenchTestServices'; -import { TestTextResourceConfigurationService, TestEditorGroupsService, TestEditorService } from 'vs/workbench/test/browser/workbenchTestServices'; -import { TestThemeService } from 'vs/platform/theme/test/common/testThemeService'; -import { TestConfigurationService } from 'vs/platform/configuration/test/common/testConfigurationService'; -import * as dom from 'vs/base/browser/dom'; - -export class TestNotebookEditor extends NotebookEditorStub { - constructor(private _cellGuid?: string, private _instantiationService?: IInstantiationService) { - super(); - } - cellEditors: CellEditorProviderStub[] = [new TestCellEditorProvider(this._cellGuid, this._instantiationService)]; -} - -class TestCellEditorProvider extends CellEditorProviderStub { - private _editor: QueryTextEditor; - private _cellGuid: string; - constructor(cellGuid: string, instantiationService?: IInstantiationService) { - super(); - let div = dom.$('div', undefined, dom.$('span', { id: 'demospan' })); - let firstChild = div.firstChild as HTMLElement; - - this._editor = new QueryTextEditor( - NullTelemetryService, - instantiationService, - new TestStorageService(), - new TestTextResourceConfigurationService(), - new TestThemeService(), - new TestEditorGroupsService(), - new TestEditorService(), - new TestConfigurationService() - ); - this._editor.create(firstChild); - this._cellGuid = cellGuid; - } - cellGuid(): string { - return this._cellGuid; - } - getEditor(): QueryTextEditor { - return this._editor; - } -} diff --git a/src/sql/workbench/contrib/notebook/test/testCommon.ts b/src/sql/workbench/contrib/notebook/test/testCommon.ts new file mode 100644 index 0000000000..f01faf8eb6 --- /dev/null +++ b/src/sql/workbench/contrib/notebook/test/testCommon.ts @@ -0,0 +1,61 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the Source EULA. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { QueryTextEditor } from 'sql/workbench/browser/modelComponents/queryTextEditor'; +import { CellEditorProviderStub, NotebookEditorStub } from 'sql/workbench/contrib/notebook/test/stubs'; +import * as dom from 'vs/base/browser/dom'; +import { TestConfigurationService } from 'vs/platform/configuration/test/common/testConfigurationService'; +import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; +import { NullTelemetryService } from 'vs/platform/telemetry/common/telemetryUtils'; +import { TestThemeService } from 'vs/platform/theme/test/common/testThemeService'; +import { TestEditorGroupsService, TestEditorService, TestTextResourceConfigurationService } from 'vs/workbench/test/browser/workbenchTestServices'; +import { TestStorageService } from 'vs/workbench/test/common/workbenchTestServices'; + +// Typically you will pass in either editor or the instantiationService parameter. +// Leave both undefined when you want the underlying object(s) to have an undefined editor. +export class TestNotebookEditor extends NotebookEditorStub { + cellEditors: CellEditorProviderStub[]; + // Normally one needs to provide either the editor or the instantiationService as the constructor parameter + constructor({ cellGuid, instantiationService, editor }: { cellGuid?: string; instantiationService?: IInstantiationService; editor?: QueryTextEditor; } = {}) { + super(); + this.cellEditors = [new TestCellEditorProvider({ cellGuid: cellGuid, instantiationService: instantiationService, editor: editor })]; + } +} + +// Typically you will pass in either editor or the instantiationService parameter. +// Leave both undefined when you want the underlying object to have an undefined editor. +class TestCellEditorProvider extends CellEditorProviderStub { + private _editor: QueryTextEditor; + private _cellGuid: string; + constructor({ cellGuid, instantiationService, editor }: { cellGuid: string; instantiationService?: IInstantiationService; editor?: QueryTextEditor; }) { + super(); + if (editor) { + this._editor = editor; + } else if (instantiationService) { + this._editor = new QueryTextEditor( + NullTelemetryService, + instantiationService, + new TestStorageService(), + new TestTextResourceConfigurationService(), + new TestThemeService(), + new TestEditorGroupsService(), + new TestEditorService(), + new TestConfigurationService() + ); + } + if (this._editor) { + let div = dom.$('div', undefined, dom.$('span', { id: 'demospan' })); + let firstChild = div.firstChild as HTMLElement; + this._editor.create(firstChild); + } + this._cellGuid = cellGuid; + } + cellGuid(): string { + return this._cellGuid; + } + getEditor(): QueryTextEditor { + return this._editor; + } +}