Second batch of tests for notebookEditor class (#10999)

* first few notebookEditor tests

* formating fixes

* unstash previous unfinished work

* saving

* PR feedback

* PR feedback

* copyright fixes

* improve test names and assertion messages

* saving

* 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

* createEditor test and dispose fixes

* remove unneeded import

* rempve unnecessary mock usage in dispose test

* use getContainer() for ['parent']

* use getContainer for ['parent']

* remove launch.json edits

* notebookService needs 9th constructor argument

* Add uploading debugging step

* minor changes

* remove changes to ci.yml

* working version

* formatting fixes

* pr fixes

* hygiene and exception path fixes

* remove minor debug changes

* pr feedback

* pr feedback

Co-authored-by: Arvind Ranasaria MacPro <arvranmac@MININT-OE7B592.fareast.corp.microsoft.com>
Co-authored-by: chgagnon <chgagnon@microsoft.com>

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();
+	findNext(): Promise<void>;
-	findPrevious();
+	findPrevious(): Promise<void>;
 }
 
 export interface IConfigurationChangedEvent {

src/sql/workbench/contrib/notebook/browser/models/notebookFindModel.ts

@@ -17,7 +17,7 @@ export interface INotebookFindModel {
 	findNext(): Promise<NotebookRange>;
 	/** find the previous match */
 	findPrevious(): Promise<NotebookRange>;
-	/** 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<NotebookRange>;
 	/** clear the results of the find */
 	clearFind(): void;

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;

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<void> {
-		let notebookEditorModel = await this.notebookInput.resolve();
+		await this.notebookInput.resolve();
-		if (notebookEditorModel && !this.notebookInput.notebookFindModel.notebookModel) {
+		this._notebookModel = this.notebookInput.notebookFindModel.notebookModel;
-			this._notebookModel = notebookEditorModel.getNotebookModel();
-			this.notebookInput.notebookFindModel.notebookModel = this._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,7 +336,7 @@ 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 => {
+			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),
@@ -347,7 +346,6 @@ export class NotebookEditor extends BaseEditor implements IFindNotebookControlle
 			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 {
+	public async findNext(): Promise<void> {
-		this.notebookFindModel.findNext().then(p => {
+		try {
-			this.updatePosition(p);
+			const p = await this.notebookFindModel.findNext();
+			this.setSelection(p);
 			this._updateFinderMatchState();
 			this._setCurrentFindMatch(p);
-		}, er => { onUnexpectedError(er); });
+		} catch (er) {
+			onUnexpectedError(er);
+		}
 	}
 
-
+	public async findPrevious(): Promise<void> {
-	public findPrevious(): void {
+		try {
-		this.notebookFindModel.findPrevious().then(p => {
+			const p = await this.notebookFindModel.findPrevious();
-			this.updatePosition(p);
+			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);

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<INotebookService>;
 	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);
 

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 * as stubs from 'sql/workbench/contrib/notebook/test/stubs';
-import { TestNotebookEditor } from 'sql/workbench/contrib/notebook/test/testCommon';
+import { NotebookEditorStub } from 'sql/workbench/contrib/notebook/test/testCommon';
-import { ICellModel } from 'sql/workbench/services/notebook/browser/models/modelInterfaces';
+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 * as TypeMoq from 'typemoq';
-import { Emitter } from 'vs/base/common/event';
+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<ICellModel> = [new CellModel(undefined, undefined)];
+	private _contentChangedEmitter = new Emitter<NotebookContentChange>();
+	private _kernelChangedEmitter = new Emitter<nb.IKernelChangedArgs>();
+	private _onActiveCellChanged = new Emitter<ICellModel>();
+	get cells(): ReadonlyArray<ICellModel> {
+		return this._cells;
+	}
+	public get contentChanged(): Event<NotebookContentChange> {
+		return this._contentChangedEmitter.event;
+	}
+
+	get kernelChanged(): Event<nb.IKernelChangedArgs> {
+		return this._kernelChangedEmitter.event;
+	}
+
+	get onActiveCellChanged(): Event<ICellModel> {
+		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 });
+	const notebookEditorStub = new NotebookEditorStub({ cellGuid: cellTextEditorGuid, editor: queryTextEditor, model: new NotebookModelStub() });
-	testNotebookEditor.id = untitledNotebookInput.notebookUri.toString();
+	notebookEditorStub.id = untitledNotebookInput.notebookUri.toString();
-	testNotebookEditor.model = new NotebookModelStub();
+	notebookService.addNotebookEditor(notebookEditorStub);
-	notebookService.addNotebookEditor(testNotebookEditor);
 
 	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 = (<DisposableStore>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.
+	for (const input of ['', undefined, null, 'unknown string', /*unknown guid*/generateUuid()]) {
-	['', undefined, null, 'unknown string', /*unknown guid*/generateUuid()].forEach(input => {
+		test(`Verifies that getCellEditor() returns undefined for invalid or unknown guid:'${input}'`, async () => {
-		test(`NotebookEditor: Negative Test for getCellEditor() - returns undefined for invalid or unknown guid:'${input}'`, async () => {
+			await setupNotebookEditor(notebookEditor, untitledNotebookInput);
-			const result = notebookEditor.getCellEditor(input);
+			const inputGuid = <string>input;
-			assert.strictEqual(result, undefined, `notebookEditor.getCellEditor() should return undefined when invalid guid:'${input}' is passed in for a notebookEditor of an empty document.`);
+			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 [<NotebookRange>{}, new NotebookRange(<ICellModel>{}, 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(<ICellModel>{}, 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<void> {
+	createEditor(notebookEditor);
+	await setInputDocument(notebookEditor, untitledNotebookInput);
+}
+
+async function setInputDocument(notebookEditor: NotebookEditor, untitledNotebookInput: UntitledNotebookInput): Promise<void> {
+	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
+}
+

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');

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<INotebookModel> {
+		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; }) {