Add a few more notebook model unit tests (#9859)

* Add a few more notebook model tests

* Add tests for nb managers, active cell

* Add assert messages
This commit is contained in:
Chris LaFreniere 2020-04-06 16:39:02 -07:00 коммит произвёл GitHub
Родитель 4c01482a8f
Коммит 1d22e23c2d
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
2 изменённых файлов: 270 добавлений и 35 удалений

Просмотреть файл

@ -14,7 +14,7 @@ import { URI } from 'vs/base/common/uri';
import { NotebookManagerStub } from 'sql/workbench/contrib/notebook/test/stubs';
import { NotebookModel } from 'sql/workbench/services/notebook/browser/models/notebookModel';
import { ModelFactory } from 'sql/workbench/services/notebook/browser/models/modelFactory';
import { IClientSession, INotebookModelOptions, NotebookContentChange, IClientSessionOptions } from 'sql/workbench/services/notebook/browser/models/modelInterfaces';
import { IClientSession, INotebookModelOptions, NotebookContentChange, IClientSessionOptions, ICellModel } from 'sql/workbench/services/notebook/browser/models/modelInterfaces';
import { ClientSession } from 'sql/workbench/services/notebook/browser/models/clientSession';
import { CellTypes, NotebookChangeType } from 'sql/workbench/services/notebook/common/contracts';
import { Deferred } from 'sql/base/common/promise';
@ -32,6 +32,9 @@ import { isUndefinedOrNull } from 'vs/base/common/types';
import { assign } from 'vs/base/common/objects';
import { NotebookEditorContentManager } from 'sql/workbench/contrib/notebook/browser/models/notebookInput';
import { SessionManager } from 'sql/workbench/services/notebook/browser/sessionManager';
import { mssqlProviderName } from 'sql/platform/connection/common/constants';
import { ConnectionProfile } from 'sql/platform/connection/common/connectionProfile';
import { uriPrefixes } from 'sql/platform/connection/common/utils';
let expectedNotebookContent: nb.INotebookContents = {
cells: [{
@ -205,6 +208,163 @@ suite('notebook model', function (): void {
assert.deepEqual(model.cells[1].source, expectedNotebookContent.cells[1].source);
});
test('Should handle multiple notebook managers', async function (): Promise<void> {
// Given a notebook with 2 cells
let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager);
mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedNotebookContent));
defaultModelOptions.contentManager = mockContentManager.object;
let defaultNotebookManager = new NotebookManagerStub();
defaultNotebookManager.providerId = 'SQL';
let jupyterNotebookManager = new NotebookManagerStub();
jupyterNotebookManager.providerId = 'jupyter';
// Setup 2 notebook managers
defaultModelOptions.notebookManagers = [defaultNotebookManager, jupyterNotebookManager];
// Change default notebook provider id to jupyter
defaultModelOptions.providerId = 'jupyter';
// When I initalize the model
let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, undefined);
await model.loadContents();
// I expect the default provider to be jupyter
assert.equal(model.notebookManager.providerId, 'jupyter', 'Notebook manager provider id incorrect');
// Similarly, change default notebook provider id to SQL
defaultModelOptions.providerId = 'SQL';
// When I initalize the model
model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, undefined);
await model.loadContents();
// I expect the default provider to be SQL
assert.equal(model.notebookManager.providerId, 'SQL', 'Notebook manager provider id incorrect after 2nd model load');
// Check that the getters return the correct values
assert.equal(model.notebookManagers.length, 2, 'There should be 2 notebook managers');
assert(!isUndefinedOrNull(model.getNotebookManager('SQL')), 'SQL notebook manager is not defined');
assert(!isUndefinedOrNull(model.getNotebookManager('jupyter')), 'Jupyter notebook manager is not defined');
assert(isUndefinedOrNull(model.getNotebookManager('foo')), 'foo notebook manager is incorrectly defined');
// Check other properties to ensure that they're returning as expected
// No server manager was passed into the notebook manager stub, so expect hasServerManager to return false
assert.equal(model.hasServerManager, false, 'Notebook model should not have a server manager');
assert.equal(model.notebookUri, defaultModelOptions.notebookUri, 'Notebook model has incorrect URI');
});
test('Should set active cell correctly', async function (): Promise<void> {
// Given a notebook with 2 cells
let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager);
mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedNotebookContent));
defaultModelOptions.contentManager = mockContentManager.object;
// When I initalize the model
let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, undefined);
await model.loadContents();
let activeCellChangeCount = 0;
let activeCellFromEvent: ICellModel = undefined;
model.onActiveCellChanged(c => {
activeCellChangeCount++;
activeCellFromEvent = c;
});
let notebookContentChange: NotebookContentChange;
model.contentChanged(c => notebookContentChange = c);
// Then I expect all cells to be in the model
assert.equal(model.cells.length, 2, 'Cell count in notebook model is not correct');
// Set the first cell as active
model.updateActiveCell(model.cells[0]);
assert.deepEqual(model.activeCell, model.cells[0], 'Active cell does not match the first cell');
assert.deepEqual(model.activeCell, activeCellFromEvent, 'Active cell returned from the event does not match');
assert.equal(activeCellChangeCount, 1, 'Active cell change count is incorrect');
assert(isUndefinedOrNull(notebookContentChange), 'Content change should be undefined');
// Set the second cell as active
model.updateActiveCell(model.cells[1]);
assert.deepEqual(model.activeCell, model.cells[1], 'Active cell does not match expected value');
assert.deepEqual(model.activeCell, activeCellFromEvent, 'Active cell returned from the event does not match (2nd)');
assert.equal(activeCellChangeCount, 2, 'Active cell change count is incorrect; should be 2');
// Delete the active cell
model.deleteCell(model.cells[1]);
assert(isUndefinedOrNull(model.activeCell), 'Active cell should be undefined after active cell is deleted');
assert.deepEqual(model.activeCell, activeCellFromEvent, 'Active cell should match value from event');
assert.equal(activeCellChangeCount, 3, 'Active cell change count is incorrect; should be 3');
// Set the remaining cell as active
model.updateActiveCell(model.cells[0]);
assert.deepEqual(model.activeCell, activeCellFromEvent, 'Active cell should match value from event');
assert.equal(activeCellChangeCount, 4, 'Active cell change count is incorrect; should be 4');
// Add new cell
let newCell = model.addCell(CellTypes.Code, 0);
// Ensure new cell is active cell
assert.deepEqual(model.activeCell, newCell, 'Active cell does not match newly created cell');
assert.equal(activeCellChangeCount, 5, 'Active cell change count is incorrect; should be 5');
});
test('Should delete cells correctly', async function (): Promise<void> {
// Given a notebook with 2 cells
let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager);
mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedNotebookContent));
defaultModelOptions.contentManager = mockContentManager.object;
// When I initalize the model
let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, undefined);
await model.loadContents();
// Count number of times onError event is fired
let errorCount = 0;
let notebookContentChange: NotebookContentChange;
model.onError(() => errorCount++);
model.contentChanged(c => notebookContentChange = c);
// Then I expect all cells to be in the model
assert.equal(model.cells.length, 2, 'Cell count in model is incorrect');
assert.equal(model.findCellIndex(model.cells[0]), 0, 'findCellIndex returned wrong cell info for first cell');
assert.equal(model.findCellIndex(model.cells[1]), 1, 'findCellIndex returned wrong cell info for second cell');
// Delete the first cell
model.deleteCell(model.cells[0]);
assert.equal(model.cells.length, 1, 'Cell model length should be 1 after cell deletion');
assert.deepEqual(model.cells[0].source, expectedNotebookContent.cells[1].source, 'Expected cell source is incorrect');
assert.equal(model.findCellIndex(model.cells[0]), 0, 'findCellIndex returned wrong cell info for only remaining cell');
assert.equal(notebookContentChange.changeType, NotebookChangeType.CellsModified, 'notebookContentChange changeType is incorrect');
assert.equal(notebookContentChange.isDirty, true, 'notebookContentChange should set dirty flag');
assert.equal(model.activeCell, undefined, 'active cell is not undefined');
// Delete the remaining cell
notebookContentChange = undefined;
model.deleteCell(model.cells[0]);
assert.equal(model.cells.length, 0, 'There should be no cells tracked in the notebook model');
assert.equal(model.findCellIndex(model.cells[0]), -1, 'findCellIndex is incorrectly finding a deleted cell');
assert.equal(errorCount, 0, 'There should be no errors after deleting a cell that exists');
assert.equal(notebookContentChange.changeType, NotebookChangeType.CellsModified, 'notebookContentChange changeType should indicate CellsModified');
assert.equal(model.activeCell, undefined, 'Active cell should be undefined');
// Try deleting the cell again
notebookContentChange = undefined;
model.deleteCell(model.cells[0]);
assert.equal(errorCount, 1, 'The model should record an error after trying to delete a cell that does not exist');
assert(isUndefinedOrNull(notebookContentChange), 'There should be no content change after an error is recorded');
// Try deleting as notebook model is in error state
notebookContentChange = undefined;
model.deleteCell(model.cells[0]);
assert.equal(errorCount, 2, 'Error count should be 2 after trying to delete a cell that does not exist a second time');
assert(isUndefinedOrNull(notebookContentChange), 'There still should be no content change after an error is recorded');
});
test('Should load contents but then go to error state if client session startup fails', async function (): Promise<void> {
let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager);
mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedNotebookContentOneCell));
@ -234,38 +394,10 @@ suite('notebook model', function (): void {
});
test('Should not be in error state if client session initialization succeeds', async function (): Promise<void> {
let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager);
mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedNotebookContent));
defaultModelOptions.contentManager = mockContentManager.object;
let kernelChangedEmitter: Emitter<nb.IKernelChangedArgs> = new Emitter<nb.IKernelChangedArgs>();
let statusChangedEmitter: Emitter<nb.ISession> = new Emitter<nb.ISession>();
let model = await loadModelAndStartClientSession();
mockClientSession.setup(c => c.isInErrorState).returns(() => false);
mockClientSession.setup(c => c.isReady).returns(() => true);
mockClientSession.setup(c => c.kernelChanged).returns(() => kernelChangedEmitter.event);
mockClientSession.setup(c => c.statusChanged).returns(() => statusChangedEmitter.event);
queryConnectionService.setup(c => c.getActiveConnections(TypeMoq.It.isAny())).returns(() => null);
sessionReady.resolve();
let actualSession: IClientSession = undefined;
let options: INotebookModelOptions = assign({}, defaultModelOptions, <Partial<INotebookModelOptions>>{
factory: mockModelFactory.object
});
let model = new NotebookModel(options, undefined, logService, undefined, undefined);
model.onClientSessionReady((session) => actualSession = session);
await model.requestModelLoad();
await model.startSession(notebookManagers[0]);
// Then I expect load to succeed
assert(!isUndefinedOrNull(model.clientSession));
// but on server load completion I expect error state to be set
// Note: do not expect serverLoad event to throw even if failed
await model.sessionLoadFinished;
assert.equal(model.inErrorState, false);
assert.deepEqual(actualSession, mockClientSession.object);
assert.equal(model.notebookManagers.length, 1);
assert.deepEqual(model.clientSession, mockClientSession.object);
});
@ -301,4 +433,108 @@ suite('notebook model', function (): void {
assert(!isUndefinedOrNull(actualChanged));
assert.equal(actualChanged.changeType, NotebookChangeType.TrustChanged);
});
test('Should close active session when closed', async function () {
let model = await loadModelAndStartClientSession();
// After client session is started, ensure session is ready
assert(model.isSessionReady);
// After closing the notebook
await model.handleClosed();
// Ensure client session is cleaned up
assert(isUndefinedOrNull(model.clientSession), 'clientSession is not cleaned up properly');
// Ensure session is no longer ready
assert.equal(model.isSessionReady, false, 'session is incorrectly showing as ready');
});
test('Should disconnect when connection profile created by notebook', async function () {
let model = await loadModelAndStartClientSession();
// Ensure notebook prefix is present in the connection URI
queryConnectionService.setup(c => c.getConnectionUri(TypeMoq.It.isAny())).returns(() => `${uriPrefixes.notebook}some/path`);
await changeContextWithConnectionProfile(model);
// After client session is started, ensure context isn't null/undefined
assert(!isUndefinedOrNull(model.context), 'context should exist after call to change context');
// After closing the notebook
await model.handleClosed();
// Ensure disconnect is called once
queryConnectionService.verify((c) => c.disconnect(TypeMoq.It.isAny()), TypeMoq.Times.once());
});
test('Should not disconnect when connection profile not created by notebook', async function () {
let model = await loadModelAndStartClientSession();
// Ensure notebook prefix isn't present in connection URI
queryConnectionService.setup(c => c.getConnectionUri(TypeMoq.It.isAny())).returns(() => `${uriPrefixes.default}some/path`);
await changeContextWithConnectionProfile(model);
// After client session is started, ensure context isn't null/undefined
assert(!isUndefinedOrNull(model.context), 'context should exist after call to change context');
// After closing the notebook
await model.handleClosed();
// Ensure disconnect is never called
queryConnectionService.verify((c) => c.disconnect(TypeMoq.It.isAny()), TypeMoq.Times.never());
});
async function loadModelAndStartClientSession(): Promise<NotebookModel> {
let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager);
mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedNotebookContent));
defaultModelOptions.contentManager = mockContentManager.object;
let kernelChangedEmitter: Emitter<nb.IKernelChangedArgs> = new Emitter<nb.IKernelChangedArgs>();
let statusChangedEmitter: Emitter<nb.ISession> = new Emitter<nb.ISession>();
mockClientSession.setup(c => c.isInErrorState).returns(() => false);
mockClientSession.setup(c => c.isReady).returns(() => true);
mockClientSession.setup(c => c.kernelChanged).returns(() => kernelChangedEmitter.event);
mockClientSession.setup(c => c.statusChanged).returns(() => statusChangedEmitter.event);
queryConnectionService.setup(c => c.getActiveConnections(TypeMoq.It.isAny())).returns(() => null);
sessionReady.resolve();
let actualSession: IClientSession = undefined;
let options: INotebookModelOptions = assign({}, defaultModelOptions, <Partial<INotebookModelOptions>>{
factory: mockModelFactory.object
});
let model = new NotebookModel(options, undefined, logService, undefined, undefined);
model.onClientSessionReady((session) => actualSession = session);
await model.requestModelLoad();
await model.startSession(notebookManagers[0]);
// Then I expect load to succeed
assert(!isUndefinedOrNull(model.clientSession), 'clientSession should exist after session is started');
assert.deepEqual(actualSession, mockClientSession.object, 'session returned is not the expected object');
// but on server load completion I expect error state to be set
// Note: do not expect serverLoad event to throw even if failed
await model.sessionLoadFinished;
return model;
}
async function changeContextWithConnectionProfile(model: NotebookModel) {
let connection = new ConnectionProfile(capabilitiesService.object, {
connectionName: 'newName',
savePassword: false,
groupFullName: 'testGroup',
serverName: 'testServerName',
databaseName: 'testDatabaseName',
authenticationType: 'integrated',
password: 'test',
userName: 'testUsername',
groupId: undefined,
providerName: mssqlProviderName,
options: {},
saveProfile: true,
id: 'testID'
});
await model.changeContext(connection.connectionName, connection);
}
});

Просмотреть файл

@ -396,6 +396,9 @@ export class NotebookModel extends Disposable implements INotebookModel {
let index = firstIndex(this._cells, (cell) => cell.equals(cellModel));
if (index > -1) {
this._cells.splice(index, 1);
if (this._activeCell === cellModel) {
this.updateActiveCell(undefined);
}
this._contentChangedEmitter.fire({
changeType: NotebookChangeType.CellsModified,
cells: [cellModel],
@ -434,10 +437,6 @@ export class NotebookModel extends Disposable implements INotebookModel {
return this._activeCell;
}
public set activeCell(cell: ICellModel) {
this._activeCell = cell;
}
private notifyError(error: string): void {
this._onErrorEmitter.fire({ message: error, severity: Severity.Error });
}