diff --git a/src/sql/workbench/contrib/notebook/browser/find/notebookFindWidget.ts b/src/sql/workbench/contrib/notebook/browser/find/notebookFindWidget.ts index cc7615ea8a..cf2e622c23 100644 --- a/src/sql/workbench/contrib/notebook/browser/find/notebookFindWidget.ts +++ b/src/sql/workbench/contrib/notebook/browser/find/notebookFindWidget.ts @@ -55,8 +55,8 @@ export interface IFindNotebookController { addOverlayWidget(widget: IOverlayWidget): void; getAction(id: string): IEditorAction; onDidChangeConfiguration(fn: (e: IConfigurationChangedEvent) => void): IDisposable; - findNext(); - findPrevious(); + findNext(): Promise; + findPrevious(): Promise; } export interface IConfigurationChangedEvent { diff --git a/src/sql/workbench/contrib/notebook/browser/models/notebookFindModel.ts b/src/sql/workbench/contrib/notebook/browser/models/notebookFindModel.ts index a1033d184c..4222534ff9 100644 --- a/src/sql/workbench/contrib/notebook/browser/models/notebookFindModel.ts +++ b/src/sql/workbench/contrib/notebook/browser/models/notebookFindModel.ts @@ -17,7 +17,7 @@ export interface INotebookFindModel { findNext(): Promise; /** find the previous match */ findPrevious(): Promise; - /** search the notebook model for the given exp up to maxMatch occurances */ + /** search the notebook model for the given exp up to maxMatch occurrences */ find(exp: string, matchCase?: boolean, wholeWord?: boolean, maxMatches?: number): Promise; /** clear the results of the find */ clearFind(): void; diff --git a/src/sql/workbench/contrib/notebook/browser/models/notebookInput.ts b/src/sql/workbench/contrib/notebook/browser/models/notebookInput.ts index 6406ba5db0..66348e602f 100644 --- a/src/sql/workbench/contrib/notebook/browser/models/notebookInput.ts +++ b/src/sql/workbench/contrib/notebook/browser/models/notebookInput.ts @@ -175,7 +175,7 @@ export class NotebookEditorModel extends EditorModel { return this.getNotebookModel() !== undefined; } - public getNotebookModel(): INotebookModel { + public getNotebookModel(): INotebookModel | undefined { let editor = this.notebookService.findNotebookEditor(this.notebookUri); if (editor) { return editor.model; diff --git a/src/sql/workbench/contrib/notebook/browser/notebookEditor.ts b/src/sql/workbench/contrib/notebook/browser/notebookEditor.ts index a0fe5df231..de5638ec37 100644 --- a/src/sql/workbench/contrib/notebook/browser/notebookEditor.ts +++ b/src/sql/workbench/contrib/notebook/browser/notebookEditor.ts @@ -111,7 +111,7 @@ export class NotebookEditor extends BaseEditor implements IFindNotebookControlle } public changeDecorations(callback: (changeAccessor: IModelDecorationsChangeAccessor) => any): any { - if (!this.notebookInput.notebookFindModel) { + if (!this.notebookInput?.notebookFindModel) { // callback will not be called return null; } @@ -123,11 +123,8 @@ export class NotebookEditor extends BaseEditor implements IFindNotebookControlle } async setNotebookModel(): Promise { - let notebookEditorModel = await this.notebookInput.resolve(); - if (notebookEditorModel && !this.notebookInput.notebookFindModel.notebookModel) { - this._notebookModel = notebookEditorModel.getNotebookModel(); - this.notebookInput.notebookFindModel.notebookModel = this._notebookModel; - } + await this.notebookInput.resolve(); + this._notebookModel = this.notebookInput.notebookFindModel.notebookModel; if (!this.notebookInput.notebookFindModel.findDecorations) { this.notebookInput.notebookFindModel.setNotebookFindDecorations(this); } @@ -170,8 +167,10 @@ export class NotebookEditor extends BaseEditor implements IFindNotebookControlle /** * Sets focus on this editor. Specifically, it sets the focus on the hosted text editor. + * An implementation provided here for IFindNotebookController interface. */ public focus(): void { + //no-op } /** @@ -312,9 +311,9 @@ export class NotebookEditor extends BaseEditor implements IFindNotebookControlle this.notebookFindModel.findExpression = this._findState.searchString; this.notebookInput.notebookFindModel.find(this._findState.searchString, this._findState.matchCase, this._findState.wholeWord, NOTEBOOK_MAX_MATCHES).then(findRange => { if (findRange) { - this.updatePosition(findRange); + this.setSelection(findRange); } else if (this.notebookFindModel.findMatches.length > 0) { - this.updatePosition(this.notebookFindModel.findMatches[0].range); + this.setSelection(this.notebookFindModel.findMatches[0].range); } else { this.notebookInput.notebookFindModel.clearFind(); this._updateFinderMatchState(); @@ -337,17 +336,16 @@ export class NotebookEditor extends BaseEditor implements IFindNotebookControlle } } if (e.searchScope) { - await this.notebookInput.notebookFindModel.find(this._findState.searchString, this._findState.matchCase, this._findState.wholeWord, NOTEBOOK_MAX_MATCHES).then(findRange => { - this._findDecorations.set(this.notebookFindModel.findMatches, this._currentMatch); - this._findState.changeMatchInfo( - this.notebookFindModel.getIndexByRange(this._currentMatch), - this._findDecorations.getCount(), - this._currentMatch - ); - if (this._finder.getDomNode().style.visibility === 'visible') { - this._setCurrentFindMatch(this._currentMatch); - } - }); + await this.notebookInput.notebookFindModel.find(this._findState.searchString, this._findState.matchCase, this._findState.wholeWord, NOTEBOOK_MAX_MATCHES); + this._findDecorations.set(this.notebookFindModel.findMatches, this._currentMatch); + this._findState.changeMatchInfo( + this.notebookFindModel.getIndexByRange(this._currentMatch), + this._findDecorations.getCount(), + this._currentMatch + ); + if (this._finder.getDomNode().style.visibility === 'visible') { + this._setCurrentFindMatch(this._currentMatch); + } } } @@ -388,6 +386,7 @@ export class NotebookEditor extends BaseEditor implements IFindNotebookControlle this._previousMatch = this._currentMatch; this._currentMatch = range; } + public toggleSearch(): void { // reveal only when the model is loaded let isRevealed: boolean = !this._findState.isRevealed && this._notebookModel ? !this._findState.isRevealed : false; @@ -399,21 +398,26 @@ export class NotebookEditor extends BaseEditor implements IFindNotebookControlle } } - public findNext(): void { - this.notebookFindModel.findNext().then(p => { - this.updatePosition(p); + public async findNext(): Promise { + try { + const p = await this.notebookFindModel.findNext(); + this.setSelection(p); this._updateFinderMatchState(); this._setCurrentFindMatch(p); - }, er => { onUnexpectedError(er); }); + } catch (er) { + onUnexpectedError(er); + } } - - public findPrevious(): void { - this.notebookFindModel.findPrevious().then(p => { - this.updatePosition(p); + public async findPrevious(): Promise { + try { + const p = await this.notebookFindModel.findPrevious(); + this.setSelection(p); this._updateFinderMatchState(); this._setCurrentFindMatch(p); - }, er => { onUnexpectedError(er); }); + } catch (er) { + onUnexpectedError(er); + } } private _updateFinderMatchState(): void { @@ -424,11 +428,6 @@ export class NotebookEditor extends BaseEditor implements IFindNotebookControlle } } - private updatePosition(range: NotebookRange): void { - this._previousMatch = this._currentMatch; - this._currentMatch = range; - } - private _setCurrentFindMatch(match: NotebookRange): void { if (match) { this._notebookModel.updateActiveCell(match.cell); 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 89997397c3..3e54b1ae28 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/testCommon'; +import { NotebookEditorStub } from 'sql/workbench/contrib/notebook/test/testCommon'; @@ -37,7 +37,7 @@ suite('MarkdownTextTransformer', () => { let markdownTextTransformer: MarkdownTextTransformer; let widget: IEditor; let textModel: TextModel; - let notebookEditor: TestNotebookEditor; + let notebookEditor: NotebookEditorStub; let mockNotebookService: TypeMoq.Mock; let cellModel: CellModel; @@ -58,7 +58,7 @@ suite('MarkdownTextTransformer', () => { undefined, undefined, undefined, undefined, undefined, undefined, TestEnvironmentService); cellModel = new CellModel(undefined, undefined, mockNotebookService.object); - notebookEditor = new TestNotebookEditor({ cellGuid: cellModel.cellGuid, instantiationService: instantiationService }); + notebookEditor = new NotebookEditorStub({ 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 index 2875762e57..5a8c9081ca 100644 --- a/src/sql/workbench/contrib/notebook/test/browser/notebookEditor.test.ts +++ b/src/sql/workbench/contrib/notebook/test/browser/notebookEditor.test.ts @@ -4,22 +4,30 @@ *--------------------------------------------------------------------------------------------*/ import * as assert from 'assert'; +import { nb } from 'azdata'; import { QueryTextEditor } from 'sql/workbench/browser/modelComponents/queryTextEditor'; +import { ACTION_IDS } from 'sql/workbench/contrib/notebook/browser/find/notebookFindWidget'; import { UntitledNotebookInput } from 'sql/workbench/contrib/notebook/browser/models/untitledNotebookInput'; +import { NotebookFindNextAction, NotebookFindPreviousAction } from 'sql/workbench/contrib/notebook/browser/notebookActions'; 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 * as stubs from 'sql/workbench/contrib/notebook/test/stubs'; +import { NotebookEditorStub } from 'sql/workbench/contrib/notebook/test/testCommon'; +import { CellModel } from 'sql/workbench/services/notebook/browser/models/cell'; +import { ICellModel, NotebookContentChange } 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 * as TypeMoq from 'typemoq'; +import * as DOM from 'vs/base/browser/dom'; +import { errorHandler, onUnexpectedError } from 'vs/base/common/errors'; +import { Emitter, Event } 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 { IOverlayWidget, IOverlayWidgetPosition } from 'vs/editor/browser/editorBrowser'; import { ITextResourceConfigurationService } from 'vs/editor/common/services/textResourceConfigurationService'; +import { INewFindReplaceState } from 'vs/editor/contrib/find/findState'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey'; import { ContextViewService } from 'vs/platform/contextview/browser/contextViewService'; @@ -35,6 +43,7 @@ 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 { ICell } from 'vs/workbench/contrib/notebook/common/notebookCommon'; 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'; @@ -45,6 +54,32 @@ import { UntitledTextEditorInput } from 'vs/workbench/services/untitled/common/u import { IUntitledTextEditorService } from 'vs/workbench/services/untitled/common/untitledTextEditorService'; import { workbenchInstantiationService } from 'vs/workbench/test/browser/workbenchTestServices'; +class NotebookModelStub extends stubs.NotebookModelStub { + private _cells: Array = [new CellModel(undefined, undefined)]; + private _contentChangedEmitter = new Emitter(); + private _kernelChangedEmitter = new Emitter(); + private _onActiveCellChanged = new Emitter(); + get cells(): ReadonlyArray { + return this._cells; + } + public get contentChanged(): Event { + return this._contentChangedEmitter.event; + } + + get kernelChanged(): Event { + return this._kernelChangedEmitter.event; + } + + get onActiveCellChanged(): Event { + return this._onActiveCellChanged.event; + } + + updateActiveCell(cell: ICellModel) { + // do nothing. + // When relevant a mock is used to intercept this call to do any verifications or run + // any code relevant for testing in the context of the test. + } +} suite('Test class NotebookEditor', () => { @@ -102,10 +137,9 @@ suite('Test class NotebookEditor', () => { 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); + const notebookEditorStub = new NotebookEditorStub({ cellGuid: cellTextEditorGuid, editor: queryTextEditor, model: new NotebookModelStub() }); + notebookEditorStub.id = untitledNotebookInput.notebookUri.toString(); + notebookService.addNotebookEditor(notebookEditorStub); setup(async () => { // Create notebookEditor @@ -120,15 +154,38 @@ suite('Test class NotebookEditor', () => { 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 () => { + test('Verifies that create() calls createEditor() and sets the provided parent object as the \'_overlay\' field', () => { + assert.strictEqual(notebookEditor['_overlay'], undefined, `The overlay must be undefined for notebookEditor when create() has not been called on it`); + let parentHtmlElement = document.createElement('div'); + notebookEditor.create(parentHtmlElement); + 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'); + }); + + for (const notebookModel of [new NotebookModelStub(), undefined]) { + test(`Tests that notebookModel='${notebookModel}' set indirectly by setInput -> setNotebookModel is returned by getNotebookModel()`, async () => { + createEditor(notebookEditor); + const untitledUri = URI.from({ scheme: Schemas.untitled, path: `NotebookEditor.Test-TestPath-${notebookModel}` }); + 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 testNotebookEditor = new NotebookEditorStub({ cellGuid: cellTextEditorGuid, editor: queryTextEditor, model: notebookModel }); + testNotebookEditor.id = untitledNotebookInput.notebookUri.toString(); + notebookService.addNotebookEditor(testNotebookEditor); + notebookEditor.clearInput(); + await notebookEditor.setInput(untitledNotebookInput, EditorOptions.create({ pinned: true })); + const result = await notebookEditor.getNotebookModel(); + assert.strictEqual(result, notebookModel, `getNotebookModel() should return the model set in the INotebookEditor object`); + }); + } + + test('Tests that dispose() disposes all objects in its disposable store', async () => { + await setupNotebookEditor(notebookEditor, untitledNotebookInput); let isDisposed = (notebookEditor['_toDispose'])['_isDisposed']; assert.ok(!isDisposed, 'initially notebookEditor\'s disposable store must not be disposed'); notebookEditor.dispose(); @@ -136,7 +193,8 @@ suite('Test class NotebookEditor', () => { 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 () => { + test('Tests that getPosition and getLastPosition correctly return the range set by setSelection', async () => { + await setupNotebookEditor(notebookEditor, untitledNotebookInput); let currentPosition = notebookEditor.getPosition(); let lastPosition = notebookEditor.getLastPosition(); assert.strictEqual(currentPosition, undefined, 'notebookEditor.getPosition() should return an undefined range with no selected range'); @@ -155,19 +213,233 @@ suite('Test class NotebookEditor', () => { 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.`); + for (const input of ['', undefined, null, 'unknown string', /*unknown guid*/generateUuid()]) { + test(`Verifies that getCellEditor() returns undefined for invalid or unknown guid:'${input}'`, async () => { + await setupNotebookEditor(notebookEditor, untitledNotebookInput); + const inputGuid = input; + const result = notebookEditor.getCellEditor(inputGuid); + assert.strictEqual(result, undefined, `notebookEditor.getCellEditor() should return undefined when invalid guid:'${inputGuid}' 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 () => { + test('Verifies that getCellEditor() returns a valid text editor object for valid guid input', async () => { + await setupNotebookEditor(notebookEditor, untitledNotebookInput); 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.'); }); + test('Verifies that domNode passed in via addOverlayWidget() call gets attached to the root HtmlElement of notebookEditor', async () => { + await setupNotebookEditor(notebookEditor, untitledNotebookInput); + const domNode: HTMLElement = document.createElement('div'); + const widget: IOverlayWidget = { + getId(): string { return ''; }, + getDomNode(): HTMLElement { return domNode; }, + getPosition(): IOverlayWidgetPosition | null { return null; } + }; + notebookEditor.addOverlayWidget(widget); + const rootElement: HTMLElement = notebookEditor['_overlay']; + assert.ok(domNode.parentElement === rootElement, `parent of the passed in domNode must be the root element of notebookEditor:${notebookEditor}`); + }); + + test('Noop methods do not throw', async () => { + // Just calling the no-op methods, test will fail if they throw + notebookEditor.focus(); + notebookEditor.layoutOverlayWidget(undefined); + assert.strictEqual(notebookEditor.deltaDecorations(undefined, undefined), undefined, 'deltaDecorations is not implemented and returns undefined'); + }); + + + test('Tests that getConfiguration returns the information set by layout() call', async () => { + await setupNotebookEditor(notebookEditor, untitledNotebookInput); + const dimension: DOM.Dimension = new DOM.Dimension(Math.random(), Math.random()); + notebookEditor.layout(dimension); + const config = notebookEditor.getConfiguration(); + assert.ok(config.layoutInfo.width === dimension.width && config.layoutInfo.height === dimension.height, `width and height returned by getConfiguration() must be same as the dimension set by layout() call`); + }); + + test('Tests setInput call with various states of input on a notebookEditor object', async () => { + createEditor(notebookEditor); + const editorOptions = EditorOptions.create({ pinned: true }); + for (const input of [ + untitledNotebookInput /* set to a known input */, + untitledNotebookInput /* tries to set the same input that was previously set */ + ]) { + await notebookEditor.setInput(input, editorOptions); + assert.strictEqual(notebookEditor.input, input, `notebookEditor.input should be the one that we set`); + } + }); + + test('Tests setInput call with various states of findState.isRevealed on a notebookEditor object', async () => { + createEditor(notebookEditor); + const editorOptions = EditorOptions.create({ pinned: true }); + for (const isRevealed of [true, false]) { + notebookEditor['_findState']['_isRevealed'] = isRevealed; + notebookEditor.clearInput(); + await notebookEditor.setInput(untitledNotebookInput, editorOptions); + assert.strictEqual(notebookEditor.input, untitledNotebookInput, `notebookEditor.input should be the one that we set`); + } + }); + + test('Verifies that call updateDecorations calls deltaDecorations() on the underlying INotebookEditor object with arguments passed to it', async () => { + await setupNotebookEditor(notebookEditor, untitledNotebookInput); + + const newRange = {} as NotebookRange; + const oldRange = {} as NotebookRange; + const iNotebookEditorMock = TypeMoq.Mock.ofInstance(notebookEditorStub); + iNotebookEditorMock.callBase = true; //by default forward all call call to the underlying object + iNotebookEditorMock.object.id = untitledNotebookInput.notebookUri.toString(); + iNotebookEditorMock + .setup(x => x.deltaDecorations(TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .callback((newDecorationRange: NotebookRange, oldDecorationRange: NotebookRange) => { + //Verify the parameters passed into the INotebookEditor.deltaDecorations() call + assert.strictEqual(newDecorationRange, newRange, 'newDecorationsRange passed to INotebookEditor.deltaDecorations() must be the one that was provided to NotebookEditor.updateDecorations() call'); + assert.strictEqual(oldDecorationRange, oldRange, 'oldDecorationsRange passed to INotebookEditor.deltaDecorations() must be the one that was provided to NotebookEditor.updateDecorations() call'); + }); + notebookService.addNotebookEditor(iNotebookEditorMock.object); + notebookEditor.updateDecorations(newRange, oldRange); + // Ensure that INotebookEditor.deltaDecorations() was called exactly once. + iNotebookEditorMock.verify(x => x.deltaDecorations(TypeMoq.It.isAny(), TypeMoq.It.isAny()), TypeMoq.Times.exactly(1)); + }); + + test('Verifies that changeDecorations call returns null when no input is set on the notebookEditor object', async () => { + const returnObject = {}; + let changeDecorationsCalled = false; + const result = notebookEditor.changeDecorations(() => { + changeDecorationsCalled = true; + return returnObject; + }); + assert.notEqual(changeDecorationsCalled, true, `changeDecorations callback should not have been called`); + assert.notStrictEqual(result, returnObject, 'object returned by the callback given to changeDecorations() call must not be returned by it'); + assert.strictEqual(result, null, 'return value of changeDecorations() call must be null when no input is set on notebookEditor object'); + }); + + test('Verifies that changeDecorations calls the callback provided to it and returns the object returned by that callback', async () => { + await setupNotebookEditor(notebookEditor, untitledNotebookInput); + const returnObject = {}; + let changeDecorationsCalled = false; + const result = notebookEditor.changeDecorations(() => { + changeDecorationsCalled = true; + return returnObject; + }); + assert.ok(changeDecorationsCalled, `changeDecorations callback should have been called`); + assert.strictEqual(result, returnObject, 'object returned by the callback given to changeDecorations() call must be returned by it'); + }); + + + test('Verifies getAction called for findNext and findPrevious returns objects of correct type', () => { + assert.ok(notebookEditor.getAction(ACTION_IDS.FIND_NEXT) instanceof NotebookFindNextAction, `getAction called for '${ACTION_IDS.FIND_NEXT}' should return an instance of ${NotebookFindNextAction}`); + assert.ok(notebookEditor.getAction(ACTION_IDS.FIND_PREVIOUS) instanceof NotebookFindPreviousAction, `getAction called for '${ACTION_IDS.FIND_PREVIOUS}' should return an instance of ${NotebookFindPreviousAction}`); + }); + + test('Verifies toggleSearch changes isRevealed state with and without a notebookModel', async () => { + await setupNotebookEditor(notebookEditor, untitledNotebookInput); + const notebookModel = await notebookEditor.getNotebookModel(); + for (const model of [notebookModel, undefined]) { + notebookEditor['_notebookModel'] = model; + for (let i: number = 1; i <= 2; i++) { //Do it twice so that two toggles return back to original state verifying both transitions + let isRevealed = notebookEditor['_findState']['_isRevealed']; + notebookEditor.toggleSearch(); + assert.strictEqual(notebookEditor['_findState']['_isRevealed'], !isRevealed && !!model, 'new isRevealed state should be false if model is undefined and should be opposite of previous state otherwise'); + } + } + }); + + for (const action of [ACTION_IDS.FIND_NEXT, ACTION_IDS.FIND_PREVIOUS]) { + test(`Tests that ${action} raises 'no search running' error when findArray is empty`, async () => { + await setupNotebookEditor(notebookEditor, untitledNotebookInput); + let unexpectedErrorCalled = false; + const onUnexpectedErrorVerifier = (error: any) => { + //console.log(`Verifies that: ${onUnexpectedError} is passed an instance of ${Error}`); + unexpectedErrorCalled = true; + assert.ok(error instanceof Error, `${onUnexpectedError} must be passed an instance of ${Error}`); + assert.strictEqual((error as Error).message, 'no search running', `Error text must be 'no search running' when findArray is empty`); + }; + errorHandler.setUnexpectedErrorHandler(onUnexpectedErrorVerifier); + notebookEditor.notebookFindModel['_findArray'] = []; //empty out the findArray. + action === ACTION_IDS.FIND_NEXT ? await notebookEditor.findNext() : await notebookEditor.findPrevious(); + let result = notebookEditor.getPosition(); + assert.strictEqual(result, undefined, 'the notebook cell range returned by find operation must be undefined with no pending finds'); + assert.strictEqual(unexpectedErrorCalled, true, `${onUnexpectedError} must be have been raised with no pending finds`); + }); + } + + for (const action of [ACTION_IDS.FIND_NEXT, ACTION_IDS.FIND_PREVIOUS]) { + for (const range of [{}, new NotebookRange({}, 0, 0, 0, 0)]) { + test(`Tests ${action} returns the NotebookRange with cell: '${JSON.stringify(range.cell)}' that is as expected given the findArray`, async () => { + await setupNotebookEditor(notebookEditor, untitledNotebookInput); + const notebookModel = await notebookEditor.getNotebookModel(); + const mockModel = TypeMoq.Mock.ofInstance(notebookModel); + mockModel.callBase = true; //forward calls to the base object + let updateActiveCellCalled = false; + mockModel.setup(x => x.updateActiveCell(TypeMoq.It.isAny())).callback((cell: ICell) => { + updateActiveCellCalled = true; + assert.strictEqual(cell, range.cell, `updateActiveCell must get called with cell property of the range object that was set in findArray\n\tactual cell:${JSON.stringify(cell, undefined, '\t')}, expected cell:${JSON.stringify(range.cell, undefined, '\t')}`); + }); + //a method call with value undefined needs a separate setup as TypeMoq.It.isAny() does not seem to match undefined parameter value. + mockModel.setup(x => x.updateActiveCell(undefined)).callback((cell: ICell) => { + updateActiveCellCalled = true; + assert.strictEqual(cell, range.cell, `updateActiveCell must get called with cell property of the range object that was set in findArray\n\tactual cell:${JSON.stringify(cell, undefined, '\t')}, expected cell:${JSON.stringify(range.cell, undefined, '\t')}`); + }); + notebookEditor.notebookFindModel['_findArray'] = [range]; //set the findArray to have the expected range + notebookEditor['_notebookModel'] = mockModel.object; + action === ACTION_IDS.FIND_NEXT ? await notebookEditor.findNext() : await notebookEditor.findPrevious(); + const result = notebookEditor.getPosition(); + assert.strictEqual(result, range, 'the notebook cell range returned by find operation must be the one that we set'); + mockModel.verify(x => x.updateActiveCell(TypeMoq.It.isAny()), TypeMoq.Times.atMostOnce()); + mockModel.verify(x => x.updateActiveCell(undefined), TypeMoq.Times.atMostOnce()); + assert.strictEqual(updateActiveCellCalled, true, 'the updateActiveCell should have gotten called'); + }); + } + } + + test(`Verifies visibility and decorations are set correctly when FindStateChange callbacks happen`, async () => { + await setupNotebookEditor(notebookEditor, untitledNotebookInput); + let currentPosition = new NotebookRange({}, 0, 0, 0, 0); + notebookEditor.setSelection(currentPosition); + notebookEditor.notebookFindModel['_findArray'] = [currentPosition]; //set some pending finds. + await notebookEditor.findNext(); + const findState = notebookEditor['_findState']; + const finder = notebookEditor['_finder']; + const newState: INewFindReplaceState = {}; + const findDecorations = notebookEditor.notebookInput.notebookFindModel.findDecorations; + const findDecorationsMock = TypeMoq.Mock.ofInstance(findDecorations); + findDecorationsMock.callBase = true; //forward to base object by default. + let clearDecorationsCalled = false; + findDecorationsMock.setup(x => x.clearDecorations()).callback(() => { + findDecorations.clearDecorations(); + clearDecorationsCalled = true; + }); + notebookEditor.notebookInput.notebookFindModel['_findDecorations'] = findDecorationsMock.object; + for (const newStateIsRevealed of [true, false]) { + newState.isRevealed = newStateIsRevealed; + clearDecorationsCalled = false; + findState.change(newState, false); + if (newStateIsRevealed) { + assert.strictEqual(finder.getDomNode().style.visibility, 'visible', 'finder node should be visible when newState.isRevealed'); + assert.ok(!clearDecorationsCalled, 'decorations are not cleared when finder isRevealed'); + assert.strictEqual(findDecorations.getStartPosition(), currentPosition, 'when finder isRevealed, decorations startPosition is set to currentPosition'); + } else { + assert.strictEqual(finder.getDomNode().style.visibility, 'hidden', 'finder node should be hidden when newState.isNotRevealed'); + assert.ok(clearDecorationsCalled, 'decorations are cleared when finder isNotRevealed'); + } + } + }); }); +async function setupNotebookEditor(notebookEditor: NotebookEditor, untitledNotebookInput: UntitledNotebookInput): Promise { + createEditor(notebookEditor); + await setInputDocument(notebookEditor, untitledNotebookInput); +} + +async function setInputDocument(notebookEditor: NotebookEditor, untitledNotebookInput: UntitledNotebookInput): Promise { + const editorOptions = EditorOptions.create({ pinned: true }); + await notebookEditor.setInput(untitledNotebookInput, editorOptions); + assert.strictEqual(notebookEditor.options, editorOptions, 'NotebookEditor options must be the ones that we set'); +} + +function createEditor(notebookEditor: NotebookEditor) { + let parentHtmlElement = document.createElement('div'); + notebookEditor.create(parentHtmlElement); // adds notebookEditor to new htmlElement as parent +} + diff --git a/src/sql/workbench/contrib/notebook/test/electron-browser/notebookFindModel.test.ts b/src/sql/workbench/contrib/notebook/test/electron-browser/notebookFindModel.test.ts index 9402210fea..e0ffd693e3 100644 --- a/src/sql/workbench/contrib/notebook/test/electron-browser/notebookFindModel.test.ts +++ b/src/sql/workbench/contrib/notebook/test/electron-browser/notebookFindModel.test.ts @@ -274,7 +274,7 @@ suite('Notebook Find Model', function (): void { await notebookFindModel.find('insert', false, false, max_find_count); assert(notebookFindModel.findMatches, 'Find in notebook failed.'); - assert.equal(notebookFindModel.findMatches.length, 3, 'Find couldnt find all occurrences'); + assert.equal(notebookFindModel.findMatches.length, 3, 'Find couldn\'t find all occurrences'); await notebookFindModel.find('insert', true, false, max_find_count); assert.equal(notebookFindModel.findMatches.length, 2, 'Find failed to apply match case while searching'); diff --git a/src/sql/workbench/contrib/notebook/test/testCommon.ts b/src/sql/workbench/contrib/notebook/test/testCommon.ts index f01faf8eb6..bff9560662 100644 --- a/src/sql/workbench/contrib/notebook/test/testCommon.ts +++ b/src/sql/workbench/contrib/notebook/test/testCommon.ts @@ -4,7 +4,8 @@ *--------------------------------------------------------------------------------------------*/ import { QueryTextEditor } from 'sql/workbench/browser/modelComponents/queryTextEditor'; -import { CellEditorProviderStub, NotebookEditorStub } from 'sql/workbench/contrib/notebook/test/stubs'; +import * as stubs from 'sql/workbench/contrib/notebook/test/stubs'; +import { INotebookModel } from 'sql/workbench/services/notebook/browser/models/modelInterfaces'; 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'; @@ -15,18 +16,29 @@ import { TestStorageService } from 'vs/workbench/test/common/workbenchTestServic // 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 { +export class NotebookEditorStub extends stubs.NotebookEditorStub { cellEditors: CellEditorProviderStub[]; + private _model: INotebookModel | undefined; + + get model(): INotebookModel | undefined { + return this._model; + } + + get modelReady(): Promise { + return Promise.resolve(this._model); + } + // 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; } = {}) { + constructor({ cellGuid, instantiationService, editor, model }: { cellGuid?: string; instantiationService?: IInstantiationService; editor?: QueryTextEditor; model?: INotebookModel } = {}) { super(); - this.cellEditors = [new TestCellEditorProvider({ cellGuid: cellGuid, instantiationService: instantiationService, editor: editor })]; + this._model = model; + this.cellEditors = [new CellEditorProviderStub({ 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 { +class CellEditorProviderStub extends stubs.CellEditorProviderStub { private _editor: QueryTextEditor; private _cellGuid: string; constructor({ cellGuid, instantiationService, editor }: { cellGuid: string; instantiationService?: IInstantiationService; editor?: QueryTextEditor; }) {