tests for KernelsDropdown class (#11476)

* add return type for a function * tests for KernelsDropdown class * remove inadvertent change * remove inadvertent change * formatting changes * pr feedback * pr feedback
2026-01-29 09:35:38 -05:00 · 2020-07-22 17:49:01 -07:00
parent b53d1cee4a
commit 9b4931bb0e
4 changed files with 328 additions and 20 deletions
--- a/src/sql/workbench/contrib/notebook/browser/notebookActions.ts
+++ b/src/sql/workbench/contrib/notebook/browser/notebookActions.ts
@@ -31,7 +31,7 @@ import { IConfigurationService } from 'vs/platform/configuration/common/configur
 import { CellContext } from 'sql/workbench/contrib/notebook/browser/cellViews/codeActions';

 const msgLoading = localize('loading', "Loading kernels...");
-const msgChanging = localize('changing', "Changing kernel...");
+export const msgChanging = localize('changing', "Changing kernel...");
 const attachToLabel: string = localize('AttachTo', "Attach to ");
 const kernelLabel: string = localize('Kernel', "Kernel ");
 const msgLoadingContexts = localize('loadingContexts', "Loading contexts...");
@@ -281,8 +281,9 @@ export class CollapseCellsAction extends ToggleableAction {
 	}
 }

-const ShowAllKernelsConfigName = 'notebook.showAllKernels';
-const WorkbenchPreviewConfigName = 'workbench.enablePreviewFeatures';
+const showAllKernelsConfigName = 'notebook.showAllKernels';
+const workbenchPreviewConfigName = 'workbench.enablePreviewFeatures';
+export const noKernelName = localize('noKernel', "No Kernel");
 export class KernelsDropdown extends SelectBox {
 	private model: NotebookModel;
 	private _showAllKernels: boolean = false;
@@ -300,7 +301,7 @@ export class KernelsDropdown extends SelectBox {
 		this.onDidSelect(e => this.doChangeKernel(e.selected));
 		this.getAllKernelConfigValue();
 		this._register(this._configurationService.onDidChangeConfiguration(e => {
-			if (e.affectsConfiguration(ShowAllKernelsConfigName) || e.affectsConfiguration(WorkbenchPreviewConfigName)) {
+			if (e.affectsConfiguration(showAllKernelsConfigName) || e.affectsConfiguration(workbenchPreviewConfigName)) {
 				this.getAllKernelConfigValue();
 			}
 		}));
@@ -327,7 +328,7 @@ export class KernelsDropdown extends SelectBox {
 					index = firstIndex(kernels, kernel => kernel === standardKernel.displayName);
 				} else {
 					let kernelSpec = this.model.specs.kernels.find(k => k.name === kernel.name);
-					index = firstIndex(kernels, k => k === kernelSpec.display_name);
+					index = firstIndex(kernels, k => k === kernelSpec?.display_name);
 				}
 				// This is an error case that should never happen
 				// Just in case, setting index to 0
@@ -337,7 +338,6 @@ export class KernelsDropdown extends SelectBox {
 				this.setOptions(kernels, index);
 			}
 		} else if (this.model.clientSession.isInErrorState) {
-			let noKernelName = localize('noKernel', "No Kernel");
 			kernels.unshift(noKernelName);
 			this.setOptions(kernels, 0);
 		}
@@ -349,7 +349,7 @@ export class KernelsDropdown extends SelectBox {
 	}

 	private getAllKernelConfigValue(): void {
-		this._showAllKernels = !!this._configurationService.getValue(ShowAllKernelsConfigName) && !!this._configurationService.getValue(WorkbenchPreviewConfigName);
+		this._showAllKernels = !!this._configurationService.getValue(showAllKernelsConfigName) && !!this._configurationService.getValue(workbenchPreviewConfigName);
 	}
 }

--- a/src/sql/workbench/contrib/notebook/test/browser/notebookActions.test.ts
+++ b/src/sql/workbench/contrib/notebook/test/browser/notebookActions.test.ts
@@ -3,18 +3,104 @@
 *  Licensed under the Source EULA. See License.txt in the project root for license information.
 *--------------------------------------------------------------------------------------------*/

-import * as TypeMoq from 'typemoq';
 import * as assert from 'assert';
-
-import { AddCellAction, ClearAllOutputsAction, CollapseCellsAction, TrustedAction, RunAllCellsAction, NewNotebookAction } from 'sql/workbench/contrib/notebook/browser/notebookActions';
-import { CellType } from 'sql/workbench/services/notebook/common/contracts';
-import { INotebookEditor } from 'sql/workbench/services/notebook/browser/notebookService';
+import * as azdata from 'azdata';
+import * as sinon from 'sinon';
+import { TestConfigurationService } from 'sql/platform/connection/test/common/testConfigurationService';
+import { AddCellAction, ClearAllOutputsAction, CollapseCellsAction, KernelsDropdown, msgChanging, NewNotebookAction, noKernelName, RunAllCellsAction, TrustedAction } from 'sql/workbench/contrib/notebook/browser/notebookActions';
+import { ClientSessionStub, ContextViewProviderStub, NotebookComponentStub, NotebookModelStub } from 'sql/workbench/contrib/notebook/test/stubs';
+import { NotebookEditorStub } from 'sql/workbench/contrib/notebook/test/testCommon';
 import { ICellModel, INotebookModel } from 'sql/workbench/services/notebook/browser/models/modelInterfaces';
+import { IStandardKernelWithProvider } from 'sql/workbench/services/notebook/browser/models/notebookUtils';
+import { INotebookEditor } from 'sql/workbench/services/notebook/browser/notebookService';
+import { CellType } from 'sql/workbench/services/notebook/common/contracts';
+import * as TypeMoq from 'typemoq';
+import { Emitter } from 'vs/base/common/event';
+import { TestCommandService } from 'vs/editor/test/browser/editorTestServices';
+import { ICommandService } from 'vs/platform/commands/common/commands';
+import { IConfigurationChangeEvent, IConfigurationOverrides, IConfigurationService } from 'vs/platform/configuration/common/configuration';
+import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock';
 import { INotificationService } from 'vs/platform/notification/common/notification';
 import { TestNotificationService } from 'vs/platform/notification/test/common/testNotificationService';
-import { ICommandService } from 'vs/platform/commands/common/commands';
-import { TestCommandService } from 'vs/editor/test/browser/editorTestServices';
-import { NotebookComponentStub } from 'sql/workbench/contrib/notebook/test/stubs';
+import { workbenchInstantiationService } from 'vs/workbench/test/browser/workbenchTestServices';
+
+class TestClientSession extends ClientSessionStub {
+	private _errorState: boolean = false;
+	setErrorState = (value: boolean) => this._errorState = value;
+	get isInErrorState(): boolean {
+		return this._errorState;
+	}
+	get kernel(): azdata.nb.IKernel {
+		return <azdata.nb.IKernel>{
+			name: 'StandardKernel1'
+		};
+	}
+}
+class TestNotebookModel extends NotebookModelStub {
+	private _clientSession: TestClientSession = new TestClientSession();
+	public kernelChangedEmitter: Emitter<azdata.nb.IKernelChangedArgs> = new Emitter<azdata.nb.IKernelChangedArgs>();
+
+	public get kernelChanged() {
+		return this.kernelChangedEmitter.event;
+	}
+
+	public get clientSession(): TestClientSession {
+		return this._clientSession;
+	}
+
+	private _standardKernelsMap: Map<string, IStandardKernelWithProvider> = new Map<string, IStandardKernelWithProvider>(
+		[
+			// The name and displayName are set to same value
+			// for ease of expected result calculation for kernelDropdown.updateKernel tests.
+			[
+				'StandardKernel1',
+				{
+					name: 'StandardKernel1',
+					displayName: 'StandardKernel1',
+					connectionProviderIds: ['Kernel1 connection 1', 'Kernel1 connection2'],
+					notebookProvider: 'kernel provider1'
+				}
+			],
+			[
+				'StandardKernel2',
+				{
+					name: 'StandardKernel2',
+					displayName: 'StandardKernel2',
+					connectionProviderIds: ['Kernel1 connection 2', 'Kernel1 connection2'],
+					notebookProvider: 'kernel provider2'
+				}
+			]
+		]
+	);
+
+	public standardKernelsDisplayName(): string[] {
+		return [...this._standardKernelsMap.values()].map(x => x.displayName);
+	}
+
+	public get specs(): azdata.nb.IAllKernels | undefined {
+		return {
+			defaultKernel: 'SpecKernel1',
+			// The name and displayName are set to same value
+			// for ease of expected result calculation for kernelDropdown.updateKernel tests.
+			kernels: [
+				{
+					name: 'SpecKernel1',
+					language: 'SpecLanguage1',
+					display_name: 'SpecKernel1'
+				},
+				{
+					name: 'SpecKernel2',
+					language: 'SpecLanguage2',
+					display_name: 'SpecKernel2'
+				}
+			]
+		};
+	}
+
+	public getStandardKernelFromName(name: string): IStandardKernelWithProvider {
+		return this._standardKernelsMap.get(name);
+	}
+}

 suite('Notebook Actions', function (): void {
 	test('Add Cell Action', async function (): Promise<void> {
@@ -152,4 +238,208 @@ suite('Notebook Actions', function (): void {

 		assert.strictEqual(actualCmdId, NewNotebookAction.INTERNAL_NEW_NOTEBOOK_CMD_ID);
 	});
+
+
+	suite('Kernels dropdown', async () => {
+		let kernelsDropdown: KernelsDropdown;
+		let contextViewProvider: ContextViewProviderStub;
+		let container: HTMLElement;
+		let notebookModel: TestNotebookModel;
+		let configurationService: TestConfigurationService;
+		let notebookEditor: NotebookEditorStub;
+		let sandbox: sinon.SinonSandbox;
+		let setOptionsSpy: sinon.SinonSpy;
+
+		setup(async () => {
+			sandbox = sinon.sandbox.create();
+			container = document.createElement('div');
+			contextViewProvider = new ContextViewProviderStub();
+			const instantiationService = <TestInstantiationService>workbenchInstantiationService();
+			configurationService = new TestConfigurationService();
+			instantiationService.set(IConfigurationService, configurationService);
+			notebookModel = new TestNotebookModel();
+			notebookEditor = new NotebookEditorStub({ model: notebookModel });
+			await notebookEditor.modelReady;
+			kernelsDropdown = new KernelsDropdown(container, contextViewProvider, notebookEditor.modelReady, configurationService);
+			setOptionsSpy = sandbox.spy(kernelsDropdown, 'setOptions');
+		});
+
+		teardown(() => {
+			sandbox.restore();
+		});
+
+		suite('updateKernel', () => {
+			suite(`kernel not defined or ready and showAllKernels is true`, () => {
+				for (const kernel of [undefined, { isReady: false }] as azdata.nb.IKernel[]) {
+					for (const clientSessionErrorState of [true, false]) {
+						test(`verify for kernel:${JSON.stringify(kernel)} and notebookModel's clientSession error state: ${clientSessionErrorState}`, () => {
+							sandbox.stub(configurationService, 'getValue').returns(true); // returns true for all configuration values.
+							const e: IConfigurationChangeEvent = <IConfigurationChangeEvent>{
+								affectsConfiguration(_configuration: string, _overrides?: IConfigurationOverrides) {
+									return true;
+								}
+							};
+							configurationService.onDidChangeConfigurationEmitter.fire(e); //reconfigure kernelDropdown object based on config changes
+							const expectedSetOptionsArgs = {
+								kernels: [noKernelName, ...notebookModel.specs.kernels.map(x => x.display_name), ...notebookModel.standardKernelsDisplayName()], // these are the kernels fed into the update method via the testNotebookModel object
+								selected: 0 // the selected value is NoKernelName value when no kernel is defined or is ready.
+							};
+							verifyUpdateKernelForNoKernelCase(notebookModel, kernelsDropdown, kernel, setOptionsSpy, expectedSetOptionsArgs, clientSessionErrorState);
+						});
+					}
+				}
+			});
+
+			suite(`kernel not defined or ready and showAllKernels is false`, () => {
+				for (const kernel of [undefined, { isReady: false }] as azdata.nb.IKernel[]) {
+					for (const clientSessionErrorState of [true, false]) {
+						test(`verify for kernel:${JSON.stringify(kernel)} and notebookModel's clientSession error state: ${clientSessionErrorState}`, () => {
+							const expectedSetOptionsArgs = {
+								kernels: [noKernelName, ...notebookModel.standardKernelsDisplayName()], // these are the kernels fed into the update method via the testNotebookModel object
+								selected: 0 // the selected value is NoKernelName value when no kernel is defined or is ready.
+							};
+							verifyUpdateKernelForNoKernelCase(notebookModel, kernelsDropdown, kernel, setOptionsSpy, expectedSetOptionsArgs, clientSessionErrorState);
+						});
+					}
+				}
+			});
+
+			suite(`kernel defined and ready and showAllKernels is true`, () => {
+				for (const kernel of [{ name: 'StandardKernel1', isReady: true }, { name: 'SpecKernel1', isReady: true }, { name: 'Unknown', isReady: true }] as azdata.nb.IKernel[]) {
+					test(`verify for kernel: '${kernel.name}'`, () => {
+						sandbox.stub(configurationService, 'getValue').returns(true); // returns true for all configuration values.
+						const e: IConfigurationChangeEvent = <IConfigurationChangeEvent>{
+							affectsConfiguration(_configuration: string, _overrides?: IConfigurationOverrides) {
+								return true;
+							}
+						};
+						configurationService.onDidChangeConfigurationEmitter.fire(e); //reconfigure kernelDropdown object based on config changes
+						testDefinedAndReadyKernelForTrueShowKernels(notebookModel, kernel, kernelsDropdown, setOptionsSpy);
+					});
+				}
+			});
+
+			suite(`kernel defined and ready and showAllKernels is false`, () => {
+				for (const kernel of [{ name: 'StandardKernel1', isReady: true }, { name: 'SpecKernel1', isReady: true }, { name: undefined, isReady: true }] as azdata.nb.IKernel[]) {
+					test(`verify for kernel with name: '${kernel.name}'`, () => {
+						sandbox.stub(configurationService, 'getValue').returns(false); // returns false for all configuration values.
+						const e: IConfigurationChangeEvent = <IConfigurationChangeEvent>{
+							affectsConfiguration(_configuration: string, _overrides?: IConfigurationOverrides) {
+								return true;
+							}
+						};
+						configurationService.onDidChangeConfigurationEmitter.fire(e); //reconfigure kernelDropdown object based on config changes
+						testDefinedAndReadyKernelForFalseShowKernels(notebookModel, kernel, kernelsDropdown, setOptionsSpy);
+					});
+				}
+
+				test(`verify showAllKernels is not affected when onDidChangeConfigurationEmitter fires with both ShowAllKernelConfigName and WorkbenchPreviewConfigName not changed`, () => {
+					const kernel = <azdata.nb.IKernel>{ name: 'StandardKernel1', isReady: true };
+					const getValueStub = sandbox.stub(configurationService, 'getValue').returns(false); // returns false for all configuration values.
+					let e: IConfigurationChangeEvent = <IConfigurationChangeEvent>{
+						affectsConfiguration(_configuration: string, _overrides?: IConfigurationOverrides) {
+							return true;
+						}
+					};
+					configurationService.onDidChangeConfigurationEmitter.fire(e); //reconfigure kernelDropdown object based on config changes
+					//showAllKernels should now be set to false
+
+					//Now fire another changeConfiguration but with affectsConfiguration returning false for all values. Even though configuration service returns true for the config values that affect showAllKernels, the test that follows proves that showAllKernels remained false.
+					getValueStub.restore();
+					sandbox.stub(configurationService, 'getValue').returns(true); // returns false for all configuration values.
+					e = <IConfigurationChangeEvent>{
+						// the following fake of returning false, simulates the scenario where config changes have occurred but none that affect should affect 'showAllKernels'
+						affectsConfiguration(_configuration: string, _overrides?: IConfigurationOverrides) {
+							return false;
+						}
+					};
+					configurationService.onDidChangeConfigurationEmitter.fire(e); //reconfigure kernelDropdown object based on config changes
+
+					// test for showKernels = false
+					testDefinedAndReadyKernelForFalseShowKernels(notebookModel, kernel, kernelsDropdown, setOptionsSpy);
+				});
+			});
+		});
+
+		suite(`doChangeKernel`, () => {
+			for (const displayName of [undefined, '', 'Arbitrary Kernel Name']) {
+				test(`verify for kernel displayName='${displayName}'`, () => {
+					const changeKernelStub = sandbox.stub(notebookModel, 'changeKernel');
+					kernelsDropdown.doChangeKernel(displayName);
+					assert.ok(setOptionsSpy.calledOnce, `setOptions should be called exactly once`);
+					assert.ok(setOptionsSpy.calledWithExactly([msgChanging], 0), `setOptions should be called with a options value of ${[msgChanging]} and selected value of 0`);
+					assert.ok(changeKernelStub.calledOnce, `notebookModel.changeKernel should be called exactly once`);
+					assert.ok(changeKernelStub.calledWithExactly(displayName), `notebookModel.changeKernel should be called with the kernel displayName that was passed to doChangeKernel`);
+				});
+			}
+		});
+
+		test(`verify that firing of notebookModel.kernelChanged event calls updateKernel`, () => {
+			const updateKernelStub = sandbox.stub(kernelsDropdown, 'updateKernel');
+			const e: azdata.nb.IKernelChangedArgs = <azdata.nb.IKernelChangedArgs>{
+				newValue: <azdata.nb.IKernel>{
+					name: 'StandardKernel2'
+				}
+			};
+			notebookModel.kernelChangedEmitter.fire(e);
+			assert.ok(updateKernelStub.calledOnce, `updateKernel should be called exactly once`);
+			assert.ok(updateKernelStub.calledWithExactly(e.newValue), `updateKernel should be called with the parameter: ${JSON.stringify(e.newValue)}`);
+		});
+
+	});
 });
+
+
+function testDefinedAndReadyKernelForTrueShowKernels(notebookModel: TestNotebookModel, kernel: azdata.nb.IKernel, kernelsDropdown: KernelsDropdown, setOptionsSpy: sinon.SinonSpy) {
+	// these are the kernels fed into the update method via the testNotebookModel object
+	const kernels = [...notebookModel.specs.kernels.map(x => x.display_name), ...notebookModel.standardKernelsDisplayName()];
+	let index = kernels.findIndex(x => x === kernel.name);
+	if (index === -1) {
+		index = 0;
+	}
+	const expectedSetOptionsArgs = {
+		kernels: kernels,
+		selected: index // selected value from the kernelDropdown options must point to the index within 'kernels' corresponding to kernel.name
+	};
+	verifyUpdateKernelForKernelDefinedAndReadyCase(notebookModel, kernelsDropdown, kernel, setOptionsSpy, expectedSetOptionsArgs);
+}
+
+function testDefinedAndReadyKernelForFalseShowKernels(notebookModel: TestNotebookModel, kernel: azdata.nb.IKernel, kernelsDropdown: KernelsDropdown, setOptionsSpy: sinon.SinonSpy) {
+	// these are the kernels fed into the update method via the testNotebookModel object
+	const kernels = [...notebookModel.standardKernelsDisplayName()];
+	let index = kernels.findIndex(x => x === kernel.name);
+	if (index === -1) {
+		index = 0;
+	}
+	const expectedSetOptionsArgs = {
+		kernels: kernels,
+		selected: index // selected value from the kernelDropdown options must point to the index within 'kernels' corresponding to kernel.name
+	};
+	verifyUpdateKernelForKernelDefinedAndReadyCase(notebookModel, kernelsDropdown, kernel, setOptionsSpy, expectedSetOptionsArgs);
+}
+
+function verifyUpdateKernelForNoKernelCase(notebookModel: TestNotebookModel, kernelsDropdown: KernelsDropdown, kernel: azdata.nb.IKernel, setOptionsSpy: sinon.SinonSpy, expectedSetOptionsArgs: {
+	kernels: string[]; // these are the kernels fed into the update method via the testNotebookModel object
+	selected: number; // the selected value is NoKernelName value when no kernel is defined or is ready.
+}, clientSessionErrorState: boolean) {
+	notebookModel.clientSession.setErrorState(clientSessionErrorState);
+	kernelsDropdown.updateKernel(kernel);
+	// setOptions is expected to get called only when clientSession is in error state
+	if (notebookModel.clientSession.isInErrorState) {
+		assert.ok(setOptionsSpy.calledOnce, `setOptions should be be called exactly once when kernel is not defined or ready and clientSession is in error state`);
+		assert.ok(setOptionsSpy.calledWithExactly(expectedSetOptionsArgs.kernels, expectedSetOptionsArgs.selected), `setOptions should be called with a options value of ${JSON.stringify(expectedSetOptionsArgs.kernels, undefined, '\t')} and selected value of ${expectedSetOptionsArgs.selected}`);
+	}
+	else {
+		assert.ok(setOptionsSpy.notCalled, `setOptions should be not be called when kernel is not defined or ready and clientSession is not in error state`);
+	}
+}
+
+function verifyUpdateKernelForKernelDefinedAndReadyCase(notebookModel: TestNotebookModel, kernelsDropdown: KernelsDropdown, kernel: azdata.nb.IKernel, setOptionsSpy: sinon.SinonSpy, expectedSetOptionsArgs: {
+	kernels: string[]; // these are the kernels fed into the update method via the testNotebookModel object
+	selected: number; // the selected value is NoKernelName value when no kernel is defined or is ready.
+}) {
+	kernelsDropdown.updateKernel(kernel);
+	assert.ok(setOptionsSpy.calledOnce, `setOptions should be be called exactly once when kernel is not defined or ready and clientSession is in error state`);
+	assert.ok(setOptionsSpy.calledWithExactly(expectedSetOptionsArgs.kernels, expectedSetOptionsArgs.selected), `setOptions should be called with a options value of ${JSON.stringify(expectedSetOptionsArgs.kernels)} and selected value of ${expectedSetOptionsArgs.selected}`);
+}
+
--- a/src/sql/workbench/contrib/notebook/test/stubs.ts
+++ b/src/sql/workbench/contrib/notebook/test/stubs.ts
@@ -16,6 +16,7 @@ import { RenderMimeRegistry } from 'sql/workbench/services/notebook/browser/outp
 import { ConnectionProfile } from 'sql/platform/connection/common/connectionProfile';
 import { URI } from 'vs/base/common/uri';
 import { QueryTextEditor } from 'sql/workbench/browser/modelComponents/queryTextEditor';
+import { IContextViewProvider, IDelegate } from 'vs/base/browser/ui/contextview/contextview';

 export class NotebookModelStub implements INotebookModel {
 	constructor(private _languageInfo?: nb.ILanguageInfo) {
@@ -693,3 +694,20 @@ export class CellEditorProviderStub implements ICellEditorProvider {
 		throw new Error('Method not implemented.');
 	}
 }
+
+export interface IContextViewEmitterArgs {
+	delegate: IDelegate,
+	container?: HTMLElement
+}
+
+export class ContextViewProviderStub implements IContextViewProvider {
+	showContextView(delegate: IDelegate, container?: HTMLElement): void {
+		throw new Error('Method not implemented.');
+	}
+	hideContextView(): void {
+		throw new Error('Method not implemented.');
+	}
+	layout(): void {
+		throw new Error('Method not implemented.');
+	}
+}