mirror of
https://github.com/ckaczor/azuredatastudio.git
synced 2026-02-16 10:58:30 -05:00
Refactored Server Instance Tests (#11868)
* Refactored and improved testing through sinon * Addressed changes - helper function and direct call to function refactor
This commit is contained in:
Vasu Bhog
@@ -8,7 +8,7 @@ import * as vscode from 'vscode';
|
||||
import * as path from 'path';
|
||||
import * as fs from 'fs-extra';
|
||||
import * as os from 'os';
|
||||
import { spawn, ExecOptions, SpawnOptions, ChildProcess } from 'child_process';
|
||||
import { spawn, SpawnOptions, ChildProcess } from 'child_process';
|
||||
import * as nls from 'vscode-nls';
|
||||
const localize = nls.loadMessageBundle();
|
||||
|
||||
@@ -25,6 +25,26 @@ const defaultPort = 8888;
|
||||
type MessageListener = (data: string | Buffer) => void;
|
||||
type ErrorListener = (err: any) => void;
|
||||
|
||||
/**
|
||||
* Helper function ensures server instance process stops
|
||||
*/
|
||||
export function ensureProcessEnded(childProcess: ChildProcess): void {
|
||||
if (!childProcess) {
|
||||
return;
|
||||
}
|
||||
// Wait 5 seconds and then force kill. Jupyter stop is slow so this seems a reasonable time limit
|
||||
setTimeout(() => {
|
||||
// Test if the process is still alive. Throws an exception if not
|
||||
try {
|
||||
process.kill(childProcess.pid, 'SIGKILL');
|
||||
} catch (error) {
|
||||
if (!error || !error.code || (typeof error.code === 'string' && error.code !== 'ESRCH')) {
|
||||
console.log(error);
|
||||
}
|
||||
}
|
||||
}, 5000);
|
||||
}
|
||||
|
||||
export interface IInstanceOptions {
|
||||
/**
|
||||
* The path to the initial document we want to start this server for
|
||||
@@ -43,60 +63,6 @@ export interface IInstanceOptions {
|
||||
notebookDirectory?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Helper class to enable testing without calling into file system or
|
||||
* commandline shell APIs
|
||||
*/
|
||||
export class ServerInstanceUtils {
|
||||
public mkDir(dirPath: string, outputChannel?: vscode.OutputChannel): Promise<void> {
|
||||
return utils.mkDir(dirPath, outputChannel);
|
||||
}
|
||||
public removeDir(dirPath: string): Promise<void> {
|
||||
return fs.remove(dirPath);
|
||||
}
|
||||
public pathExists(dirPath: string): Promise<boolean> {
|
||||
return fs.pathExists(dirPath);
|
||||
}
|
||||
public copy(src: string, dest: string): Promise<void> {
|
||||
return fs.copy(src, dest);
|
||||
}
|
||||
public async exists(path: string): Promise<boolean> {
|
||||
try {
|
||||
await fs.access(path);
|
||||
return true;
|
||||
} catch (e) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
public generateUuid(): string {
|
||||
return UUID.generateUuid();
|
||||
}
|
||||
public executeBufferedCommand(cmd: string, options: ExecOptions, outputChannel?: vscode.OutputChannel): Thenable<string> {
|
||||
return utils.executeBufferedCommand(cmd, options, outputChannel);
|
||||
}
|
||||
|
||||
public spawn(command: string, args?: ReadonlyArray<string>, options?: SpawnOptions): ChildProcess {
|
||||
return spawn(command, args, options);
|
||||
}
|
||||
|
||||
public ensureProcessEnded(childProcess: ChildProcess): void {
|
||||
if (!childProcess) {
|
||||
return;
|
||||
}
|
||||
// Wait 5 seconds and then force kill. Jupyter stop is slow so this seems a reasonable time limit
|
||||
setTimeout(() => {
|
||||
// Test if the process is still alive. Throws an exception if not
|
||||
try {
|
||||
process.kill(childProcess.pid, 'SIGKILL');
|
||||
} catch (error) {
|
||||
if (!error || !error.code || (typeof error.code === 'string' && error.code !== 'ESRCH')) {
|
||||
console.log(error);
|
||||
}
|
||||
}
|
||||
}, 5000);
|
||||
}
|
||||
}
|
||||
|
||||
export class PerFolderServerInstance implements IServerInstance {
|
||||
|
||||
/**
|
||||
@@ -122,14 +88,12 @@ export class PerFolderServerInstance implements IServerInstance {
|
||||
private _uri: vscode.Uri;
|
||||
private _isStarted: boolean = false;
|
||||
private _isStopping: boolean = false;
|
||||
private utils: ServerInstanceUtils;
|
||||
private childProcess: ChildProcess;
|
||||
private errorHandler: ErrorHandler = new ErrorHandler();
|
||||
|
||||
private readonly notebookScriptPath: string;
|
||||
|
||||
constructor(private options: IInstanceOptions, fsUtils?: ServerInstanceUtils) {
|
||||
this.utils = fsUtils || new ServerInstanceUtils();
|
||||
constructor(private options: IInstanceOptions) {
|
||||
this.notebookScriptPath = path.join(this.options.install.extensionPath, 'resources', 'pythonScripts', 'startNotebook.py');
|
||||
}
|
||||
|
||||
@@ -157,28 +121,27 @@ export class PerFolderServerInstance implements IServerInstance {
|
||||
try {
|
||||
this._isStopping = true;
|
||||
if (this.baseDir) {
|
||||
let exists = await this.utils.pathExists(this.baseDir);
|
||||
let exists = await fs.pathExists(this.baseDir);
|
||||
if (exists) {
|
||||
await this.utils.removeDir(this.baseDir);
|
||||
await fs.remove(this.baseDir);
|
||||
}
|
||||
}
|
||||
if (this._isStarted) {
|
||||
let install = this.options.install;
|
||||
let stopCommand = `"${install.pythonExecutable}" "${this.notebookScriptPath}" stop ${this._port}`;
|
||||
await this.utils.executeBufferedCommand(stopCommand, install.execOptions, install.outputChannel);
|
||||
await utils.executeBufferedCommand(stopCommand, install.execOptions, install.outputChannel);
|
||||
}
|
||||
} catch (error) {
|
||||
// For now, we don't care as this is non-critical
|
||||
this.notify(this.options.install, localize('serverStopError', "Error stopping Notebook Server: {0}", utils.getErrorMessage(error)));
|
||||
} finally {
|
||||
this._isStarted = false;
|
||||
this.utils.ensureProcessEnded(this.childProcess);
|
||||
ensureProcessEnded(this.childProcess);
|
||||
this.handleConnectionClosed();
|
||||
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
private async configureJupyter(): Promise<void> {
|
||||
await this.createInstanceFolders();
|
||||
let resourcesFolder = path.join(this.options.install.extensionPath, 'resources', constants.jupyterConfigRootFolder);
|
||||
@@ -188,35 +151,35 @@ export class PerFolderServerInstance implements IServerInstance {
|
||||
}
|
||||
|
||||
private async createInstanceFolders(): Promise<void> {
|
||||
this.baseDir = path.join(this.getSystemJupyterHomeDir(), 'instances', `${this.utils.generateUuid()}`);
|
||||
this.baseDir = path.join(this.getSystemJupyterHomeDir(), 'instances', `${UUID.generateUuid()}`);
|
||||
this.instanceConfigRoot = path.join(this.baseDir, 'config');
|
||||
this.instanceDataRoot = path.join(this.baseDir, 'data');
|
||||
await this.utils.mkDir(this.baseDir, this.options.install.outputChannel);
|
||||
await this.utils.mkDir(this.instanceConfigRoot, this.options.install.outputChannel);
|
||||
await this.utils.mkDir(this.instanceDataRoot, this.options.install.outputChannel);
|
||||
await utils.mkDir(this.baseDir, this.options.install.outputChannel);
|
||||
await utils.mkDir(this.instanceConfigRoot, this.options.install.outputChannel);
|
||||
await utils.mkDir(this.instanceDataRoot, this.options.install.outputChannel);
|
||||
}
|
||||
|
||||
private async copyInstanceConfig(resourcesFolder: string): Promise<void> {
|
||||
let configSource = path.join(resourcesFolder, NotebookConfigFilename);
|
||||
let configDest = path.join(this.instanceConfigRoot, NotebookConfigFilename);
|
||||
await this.utils.copy(configSource, configDest);
|
||||
await fs.copy(configSource, configDest);
|
||||
}
|
||||
|
||||
private async CopyCustomJs(resourcesFolder: string): Promise<void> {
|
||||
let customPath = path.join(this.instanceConfigRoot, 'custom');
|
||||
await this.utils.mkDir(customPath, this.options.install.outputChannel);
|
||||
await utils.mkDir(customPath, this.options.install.outputChannel);
|
||||
let customSource = path.join(resourcesFolder, CustomJsFilename);
|
||||
let customDest = path.join(customPath, CustomJsFilename);
|
||||
await this.utils.copy(customSource, customDest);
|
||||
await fs.copy(customSource, customDest);
|
||||
}
|
||||
|
||||
private async copyKernelsToSystemJupyterDirs(): Promise<void> {
|
||||
let kernelsExtensionSource = path.join(this.options.install.extensionPath, 'kernels');
|
||||
this._systemJupyterDir = path.join(this.getSystemJupyterHomeDir(), 'kernels');
|
||||
if (!(await this.utils.exists(this._systemJupyterDir))) {
|
||||
await this.utils.mkDir(this._systemJupyterDir, this.options.install.outputChannel);
|
||||
if (!(await utils.exists(this._systemJupyterDir))) {
|
||||
await utils.mkDir(this._systemJupyterDir, this.options.install.outputChannel);
|
||||
}
|
||||
await this.utils.copy(kernelsExtensionSource, this._systemJupyterDir);
|
||||
await fs.copy(kernelsExtensionSource, this._systemJupyterDir);
|
||||
}
|
||||
|
||||
private getSystemJupyterHomeDir(): string {
|
||||
@@ -384,7 +347,7 @@ export class PerFolderServerInstance implements IServerInstance {
|
||||
env: env,
|
||||
detached: false
|
||||
};
|
||||
let childProcess = this.utils.spawn(startCommand, [], options);
|
||||
let childProcess = spawn(startCommand, [], options);
|
||||
return childProcess;
|
||||
}
|
||||
|
||||
|
||||
@@ -6,11 +6,14 @@
|
||||
import * as should from 'should';
|
||||
import * as TypeMoq from 'typemoq';
|
||||
import * as stream from 'stream';
|
||||
import { ChildProcess } from 'child_process';
|
||||
import * as cp from 'child_process';
|
||||
import * as si from '../../jupyter/serverInstance'
|
||||
import 'mocha';
|
||||
import * as sinon from 'sinon';
|
||||
import * as utils from '../../common/utils';
|
||||
import * as fs from 'fs-extra';
|
||||
|
||||
import { JupyterServerInstallation } from '../../jupyter/jupyterServerInstallation';
|
||||
import { PerFolderServerInstance, ServerInstanceUtils } from '../../jupyter/serverInstance';
|
||||
import { MockOutputChannel } from '../common/stubs';
|
||||
import * as testUtils from '../common/testUtils';
|
||||
import { LocalJupyterServerManager } from '../../jupyter/jupyterServerManager';
|
||||
@@ -24,8 +27,7 @@ describe('Jupyter server instance', function (): void {
|
||||
let expectedPath = 'mydir/notebook.ipynb';
|
||||
let mockInstall: TypeMoq.IMock<JupyterServerInstallation>;
|
||||
let mockOutputChannel: TypeMoq.IMock<MockOutputChannel>;
|
||||
let mockUtils: TypeMoq.IMock<ServerInstanceUtils>;
|
||||
let serverInstance: PerFolderServerInstance;
|
||||
let serverInstance: si.PerFolderServerInstance;
|
||||
|
||||
beforeEach(() => {
|
||||
mockInstall = TypeMoq.Mock.ofType(JupyterServerInstallation, undefined, undefined, '/root');
|
||||
@@ -33,14 +35,16 @@ describe('Jupyter server instance', function (): void {
|
||||
mockInstall.setup(i => i.outputChannel).returns(() => mockOutputChannel.object);
|
||||
mockInstall.setup(i => i.pythonExecutable).returns(() => 'python3');
|
||||
mockInstall.object.execOptions = { env: Object.assign({}, process.env) };
|
||||
mockUtils = TypeMoq.Mock.ofType(ServerInstanceUtils);
|
||||
mockUtils.setup(u => u.ensureProcessEnded(TypeMoq.It.isAny())).returns(() => undefined);
|
||||
serverInstance = new PerFolderServerInstance({
|
||||
serverInstance = new si.PerFolderServerInstance({
|
||||
documentPath: expectedPath,
|
||||
install: mockInstall.object
|
||||
}, mockUtils.object);
|
||||
});
|
||||
sinon.stub(si,'ensureProcessEnded').returns(undefined);
|
||||
});
|
||||
|
||||
this.afterEach(function () {
|
||||
sinon.restore();
|
||||
});
|
||||
|
||||
it('Should not be started initially', function (): void {
|
||||
// Given a new instance It should not be started
|
||||
@@ -50,17 +54,17 @@ describe('Jupyter server instance', function (): void {
|
||||
|
||||
it('Should create config and data directories on configure', async function (): Promise<void> {
|
||||
// Given a server instance
|
||||
mockUtils.setup(u => u.mkDir(TypeMoq.It.isAnyString(), TypeMoq.It.isAny())).returns(() => Promise.resolve());
|
||||
mockUtils.setup(u => u.copy(TypeMoq.It.isAnyString(), TypeMoq.It.isAnyString())).returns(() => Promise.resolve());
|
||||
mockUtils.setup(u => u.exists(TypeMoq.It.isAnyString())).returns(() => Promise.resolve(false));
|
||||
let mkdirStub = sinon.stub(utils,'mkDir').withArgs(sinon.match.any,sinon.match.any).returns(Promise.resolve());
|
||||
let copyStub = sinon.stub(fs,'copy').returns();
|
||||
let pathStub = sinon.stub(utils,'exists').withArgs(sinon.match.any).returns(Promise.resolve(false));
|
||||
|
||||
// When I run configure
|
||||
await serverInstance.configure();
|
||||
|
||||
// Then I expect a folder to have been created with config and data subdirs
|
||||
mockUtils.verify(u => u.mkDir(TypeMoq.It.isAnyString(), TypeMoq.It.isAny()), TypeMoq.Times.exactly(5));
|
||||
mockUtils.verify(u => u.copy(TypeMoq.It.isAnyString(), TypeMoq.It.isAnyString()), TypeMoq.Times.exactly(3));
|
||||
mockUtils.verify(u => u.exists(TypeMoq.It.isAnyString()), TypeMoq.Times.exactly(1));
|
||||
sinon.assert.callCount(mkdirStub,5);
|
||||
sinon.assert.callCount(copyStub,3);
|
||||
sinon.assert.callCount(pathStub,1);
|
||||
});
|
||||
|
||||
it('Should have URI info after start', async function (): Promise<void> {
|
||||
@@ -69,11 +73,12 @@ describe('Jupyter server instance', function (): void {
|
||||
sdtout: (listener: (msg: string) => void) => { },
|
||||
stderr: (listener: (msg: string) => void) => listener(successMessage)
|
||||
});
|
||||
mockUtils.setup(u => u.spawn(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
|
||||
.returns(() => <ChildProcess>process.object);
|
||||
let spawnStub = sinon.stub(cp,'spawn').returns(<cp.ChildProcess>process.object);
|
||||
should(spawnStub.calledOnce).be.false();
|
||||
|
||||
// When I call start
|
||||
await serverInstance.start();
|
||||
should(spawnStub.calledOnce).be.true();
|
||||
|
||||
// Then I expect all parts of the URI to be defined
|
||||
should(serverInstance.uri).not.be.undefined();
|
||||
@@ -100,8 +105,7 @@ describe('Jupyter server instance', function (): void {
|
||||
stderr: (listener: (msg: string) => void) => listener(successMessage),
|
||||
error: (listener: (msg: string | Error) => void) => setTimeout(() => listener(new Error(error)), 10)
|
||||
});
|
||||
mockUtils.setup(u => u.spawn(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
|
||||
.returns(() => <ChildProcess>process.object);
|
||||
sinon.stub(cp,'spawn').returns(<cp.ChildProcess>process.object);
|
||||
|
||||
// When I call start then I expect it to pass
|
||||
await serverInstance.start();
|
||||
@@ -112,8 +116,7 @@ describe('Jupyter server instance', function (): void {
|
||||
let process = setupSpawn({
|
||||
exit: (listener: (msg: string | number) => void) => listener(code)
|
||||
});
|
||||
mockUtils.setup(u => u.spawn(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
|
||||
.returns(() => <ChildProcess>process.object);
|
||||
sinon.stub(cp,'spawn').returns(<cp.ChildProcess>process.object);
|
||||
|
||||
// When I call start then I expect the error to be thrown
|
||||
await testUtils.assertThrowsAsync(() => serverInstance.start(), undefined);
|
||||
@@ -126,41 +129,44 @@ describe('Jupyter server instance', function (): void {
|
||||
sdtout: (listener: (msg: string) => void) => { },
|
||||
stderr: (listener: (msg: string) => void) => listener(successMessage)
|
||||
});
|
||||
mockUtils.setup(u => u.spawn(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
|
||||
.returns(() => <ChildProcess>process.object);
|
||||
sinon.stub(cp,'spawn').returns(<cp.ChildProcess>process.object);
|
||||
|
||||
let actualCommand: string = undefined;
|
||||
mockUtils.setup(u => u.executeBufferedCommand(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
|
||||
.returns((cmd) => {
|
||||
actualCommand = cmd;
|
||||
return Promise.resolve(undefined);
|
||||
});
|
||||
mockUtils.setup(u => u.pathExists(TypeMoq.It.isAny())).returns(() => Promise.resolve(false));
|
||||
mockUtils.setup(u => u.removeDir(TypeMoq.It.isAny())).returns(() => Promise.resolve());
|
||||
let commandStub = sinon.stub(utils, 'executeBufferedCommand').withArgs(sinon.match.any, sinon.match.any,sinon.match.any)
|
||||
.returns(Promise.resolve(undefined));
|
||||
|
||||
sinon.stub(fs,'pathExists').withArgs(sinon.match.any,sinon.match.any).returns();
|
||||
let removeStub = sinon.stub(fs,'remove').withArgs(sinon.match.any,sinon.match.any).returns();
|
||||
|
||||
// When I call start and then stop
|
||||
await serverInstance.start();
|
||||
await serverInstance.stop();
|
||||
|
||||
// Then I expect stop to be called on the child process
|
||||
actualCommand = commandStub.args[0][0] as string;
|
||||
should(actualCommand.includes(`stop ${serverInstance.port}`)).be.true('Command did not contain specified port.');
|
||||
mockUtils.verify(u => u.removeDir(TypeMoq.It.isAny()), TypeMoq.Times.never());
|
||||
sinon.assert.callCount(removeStub,0);
|
||||
});
|
||||
|
||||
it('Should remove directory on close', async function (): Promise<void> {
|
||||
// Given configure and startup are done
|
||||
mockUtils.setup(u => u.mkDir(TypeMoq.It.isAnyString(), TypeMoq.It.isAny())).returns(() => Promise.resolve());
|
||||
mockUtils.setup(u => u.copy(TypeMoq.It.isAnyString(), TypeMoq.It.isAnyString())).returns(() => Promise.resolve());
|
||||
sinon.stub(utils,'mkDir').withArgs(sinon.match.any,sinon.match.any).returns(Promise.resolve());
|
||||
sinon.stub(fs,'copy').returns();
|
||||
|
||||
let process = setupSpawn({
|
||||
sdtout: (listener: (msg: string) => void) => { },
|
||||
stderr: (listener: (msg: string) => void) => listener(successMessage)
|
||||
});
|
||||
mockUtils.setup(u => u.spawn(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
|
||||
.returns(() => <ChildProcess>process.object);
|
||||
mockUtils.setup(u => u.executeBufferedCommand(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
|
||||
.returns((cmd) => Promise.resolve(undefined));
|
||||
mockUtils.setup(u => u.pathExists(TypeMoq.It.isAny())).returns(() => Promise.resolve(true));
|
||||
mockUtils.setup(u => u.removeDir(TypeMoq.It.isAny())).returns(() => Promise.resolve());
|
||||
|
||||
sinon.stub(cp,'spawn').returns(<cp.ChildProcess>process.object);
|
||||
|
||||
sinon.stub(utils, 'executeBufferedCommand').withArgs(sinon.match.any, sinon.match.any,sinon.match.any)
|
||||
.returns(Promise.resolve(undefined));
|
||||
|
||||
let pathStub = sinon.stub(fs,'pathExists');
|
||||
pathStub.resolves(true);
|
||||
|
||||
let removeStub = sinon.stub(fs,'remove').returns();
|
||||
|
||||
await serverInstance.configure();
|
||||
await serverInstance.start();
|
||||
@@ -169,7 +175,7 @@ describe('Jupyter server instance', function (): void {
|
||||
await serverInstance.stop();
|
||||
|
||||
// Then I expect the directory to be cleaned up
|
||||
mockUtils.verify(u => u.removeDir(TypeMoq.It.isAny()), TypeMoq.Times.once());
|
||||
sinon.assert.callCount(removeStub,1);
|
||||
});
|
||||
|
||||
function setupSpawn(callbacks: IProcessCallbacks): TypeMoq.IMock<ChildProcessStub> {
|
||||
|
||||
Reference in New Issue
Block a user