Clean up coverage and fix issue with notebook names (#10826)

* Clean up coverage and fix issue with notebook names

* update name

extensions/notebook/coverConfig.json

@@ -3,8 +3,10 @@
 	"relativeSourcePath": "..",
 	"relativeCoverageDir": "../../coverage",
 	"ignorePatterns": [
+		"**/integrationTest/**",
 		"**/node_modules/**",
-		"**/test/**"
+		"**/test/**",
+		"extension.js"
 	],
 	"reports": [
 		"cobertura",

extensions/notebook/src/common/constants.ts

@@ -59,3 +59,5 @@ export enum PythonPkgType {
 export const pythonWindowsInstallUrl = 'https://go.microsoft.com/fwlink/?linkid=2110625';
 export const pythonMacInstallUrl = 'https://go.microsoft.com/fwlink/?linkid=2128152';
 export const pythonLinuxInstallUrl = 'https://go.microsoft.com/fwlink/?linkid=2110524';
+
+export const notebookLanguages = ['notebook', 'ipynb'];

extensions/notebook/src/common/notebookUtils.ts

@@ -3,7 +3,18 @@
  *  Licensed under the Source EULA. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
+import * as azdata from 'azdata';
 import * as crypto from 'crypto';
+import * as os from 'os';
+import * as vscode from 'vscode';
+import * as nls from 'vscode-nls';
+import { getErrorMessage, isEditorTitleFree } from '../common/utils';
+
+const localize = nls.loadMessageBundle();
+
+const JUPYTER_NOTEBOOK_PROVIDER = 'jupyter';
+const msgSampleCodeDataFrame = localize('msgSampleCodeDataFrame', "This sample code loads the file into a data frame and shows the first 10 results.");
+const noNotebookVisible = localize('noNotebookVisible', "No notebook editor is active");
 
 /**
  * Creates a random token per https://nodejs.org/api/crypto.html#crypto_crypto_randombytes_size_callback.
@@ -20,3 +31,137 @@ export function getRandomToken(size: number = 24): Promise<string> {
 		});
 	});
 }
+
+export async function newNotebook(connectionProfile?: azdata.IConnectionProfile): Promise<azdata.nb.NotebookEditor> {
+	const title = findNextUntitledEditorName();
+	const untitledUri = vscode.Uri.parse(`untitled:${title}`);
+	const options: azdata.nb.NotebookShowOptions = connectionProfile ? {
+		viewColumn: null,
+		preserveFocus: true,
+		preview: null,
+		providerId: null,
+		connectionProfile: connectionProfile,
+		defaultKernel: null
+	} : null;
+	return azdata.nb.showNotebookDocument(untitledUri, options);
+}
+
+function findNextUntitledEditorName(): string {
+	let nextVal = 0;
+	// Note: this will go forever if it's coded wrong, or you have infinite Untitled notebooks!
+	while (true) {
+		let title = `Notebook-${nextVal}`;
+		if (isEditorTitleFree(title)) {
+			return title;
+		}
+		nextVal++;
+	}
+}
+
+export async function openNotebook(): Promise<void> {
+	try {
+		let filter: { [key: string]: Array<string> } = {};
+		// TODO support querying valid notebook file types
+		filter[localize('notebookFiles', "Notebooks")] = ['ipynb'];
+		let file = await vscode.window.showOpenDialog({
+			filters: filter
+		});
+		if (file) {
+			let doc = await vscode.workspace.openTextDocument(file[0]);
+			vscode.window.showTextDocument(doc);
+		}
+	} catch (err) {
+		vscode.window.showErrorMessage(getErrorMessage(err));
+	}
+}
+
+export async function runActiveCell(): Promise<void> {
+	try {
+		let notebook = azdata.nb.activeNotebookEditor;
+		if (notebook) {
+			await notebook.runCell();
+		} else {
+			throw new Error(noNotebookVisible);
+		}
+	} catch (err) {
+		vscode.window.showErrorMessage(getErrorMessage(err));
+	}
+}
+
+export async function clearActiveCellOutput(): Promise<void> {
+	try {
+		let notebook = azdata.nb.activeNotebookEditor;
+		if (notebook) {
+			await notebook.clearOutput();
+		} else {
+			throw new Error(noNotebookVisible);
+		}
+	} catch (err) {
+		vscode.window.showErrorMessage(getErrorMessage(err));
+	}
+}
+
+export async function runAllCells(startCell?: azdata.nb.NotebookCell, endCell?: azdata.nb.NotebookCell): Promise<void> {
+	try {
+		let notebook = azdata.nb.activeNotebookEditor;
+		if (notebook) {
+			await notebook.runAllCells(startCell, endCell);
+		} else {
+			throw new Error(noNotebookVisible);
+		}
+	} catch (err) {
+		vscode.window.showErrorMessage(getErrorMessage(err));
+	}
+}
+
+export async function addCell(cellType: azdata.nb.CellType): Promise<void> {
+	try {
+		let notebook = azdata.nb.activeNotebookEditor;
+		if (notebook) {
+			await notebook.edit((editBuilder: azdata.nb.NotebookEditorEdit) => {
+				// TODO should prompt and handle cell placement
+				editBuilder.insertCell({
+					cell_type: cellType,
+					source: ''
+				});
+			});
+		} else {
+			throw new Error(noNotebookVisible);
+		}
+	} catch (err) {
+		vscode.window.showErrorMessage(getErrorMessage(err));
+	}
+}
+
+export async function analyzeNotebook(oeContext?: azdata.ObjectExplorerContext): Promise<void> {
+	// Ensure we get a unique ID for the notebook. For now we're using a different prefix to the built-in untitled files
+	// to handle this. We should look into improving this in the future
+	let title = findNextUntitledEditorName();
+	let untitledUri = vscode.Uri.parse(`untitled:${title}`);
+
+	let editor = await azdata.nb.showNotebookDocument(untitledUri, {
+		connectionProfile: oeContext ? oeContext.connectionProfile : undefined,
+		providerId: JUPYTER_NOTEBOOK_PROVIDER,
+		preview: false,
+		defaultKernel: {
+			name: 'pysparkkernel',
+			display_name: 'PySpark',
+			language: 'python'
+		}
+	});
+	if (oeContext && oeContext.nodeInfo && oeContext.nodeInfo.nodePath) {
+		// Get the file path after '/HDFS'
+		let hdfsPath: string = oeContext.nodeInfo.nodePath.substring(oeContext.nodeInfo.nodePath.indexOf('/HDFS') + '/HDFS'.length);
+		if (hdfsPath.length > 0) {
+			let analyzeCommand = '#' + msgSampleCodeDataFrame + os.EOL + 'df = (spark.read.option("inferSchema", "true")'
+				+ os.EOL + '.option("header", "true")' + os.EOL + '.csv("{0}"))' + os.EOL + 'df.show(10)';
+
+			editor.edit(editBuilder => {
+				editBuilder.insertCell({
+					cell_type: 'code',
+					source: analyzeCommand.replace('{0}', hdfsPath)
+				}, 0);
+			});
+		}
+	}
+}

extensions/notebook/src/common/utils.ts

@@ -8,6 +8,7 @@ import * as fs from 'fs-extra';
 import * as nls from 'vscode-nls';
 import * as vscode from 'vscode';
 import * as azdata from 'azdata';
+import { notebookLanguages } from './constants';
 
 const localize = nls.loadMessageBundle();
 
@@ -189,7 +190,8 @@ export function sortPackageVersions(versions: string[], ascending: boolean = tru
 }
 
 export function isEditorTitleFree(title: string): boolean {
-	let hasTextDoc = vscode.workspace.textDocuments.findIndex(doc => doc.isUntitled && doc.fileName === title) > -1;
+
+	let hasTextDoc = vscode.workspace.textDocuments.findIndex(doc => doc.isUntitled && doc.fileName === title && !notebookLanguages.find(lang => lang === doc.languageId)) > -1;
 	let hasNotebookDoc = azdata.nb.notebookDocuments.findIndex(doc => doc.isUntitled && doc.fileName === title) > -1;
 	return !hasTextDoc && !hasNotebookDoc;
 }

extensions/notebook/src/extension.ts

@@ -5,7 +5,6 @@
 
 import * as vscode from 'vscode';
 import * as azdata from 'azdata';
-import * as os from 'os';
 import * as nls from 'vscode-nls';
 import * as path from 'path';
 
@@ -14,15 +13,12 @@ import { AppContext } from './common/appContext';
 import { ApiWrapper } from './common/apiWrapper';
 import { IExtensionApi, IPackageManageProvider } from './types';
 import { CellType } from './contracts/content';
-import { getErrorMessage, isEditorTitleFree } from './common/utils';
 import { NotebookUriHandler } from './protocol/notebookUriHandler';
 import { BookTreeViewProvider } from './book/bookTreeView';
+import { newNotebook, openNotebook, runActiveCell, runAllCells, clearActiveCellOutput, addCell, analyzeNotebook } from './common/notebookUtils';
 
 const localize = nls.loadMessageBundle();
 
-const JUPYTER_NOTEBOOK_PROVIDER = 'jupyter';
-const msgSampleCodeDataFrame = localize('msgSampleCodeDataFrame', "This sample code loads the file into a data frame and shows the first 10 results.");
-const noNotebookVisible = localize('noNotebookVisible', "No notebook editor is active");
 const BOOKS_VIEWID = 'bookTreeView';
 const PROVIDED_BOOKS_VIEWID = 'providedBooksView';
 let controller: JupyterController;
@@ -153,140 +149,6 @@ export async function activate(extensionContext: vscode.ExtensionContext): Promi
 	};
 }
 
-async function newNotebook(connectionProfile: azdata.IConnectionProfile): Promise<azdata.nb.NotebookEditor> {
-	const title = findNextUntitledEditorName();
-	const untitledUri = vscode.Uri.parse(`untitled:${title}`);
-	const options: azdata.nb.NotebookShowOptions = connectionProfile ? {
-		viewColumn: null,
-		preserveFocus: true,
-		preview: null,
-		providerId: null,
-		connectionProfile: connectionProfile,
-		defaultKernel: null
-	} : null;
-	return azdata.nb.showNotebookDocument(untitledUri, options);
-}
-
-function findNextUntitledEditorName(): string {
-	let nextVal = 0;
-	// Note: this will go forever if it's coded wrong, or you have infinite Untitled notebooks!
-	while (true) {
-		let title = `Notebook-${nextVal}`;
-		if (isEditorTitleFree(title)) {
-			return title;
-		}
-		nextVal++;
-	}
-}
-
-async function openNotebook(): Promise<void> {
-	try {
-		let filter: { [key: string]: Array<string> } = {};
-		// TODO support querying valid notebook file types
-		filter[localize('notebookFiles', "Notebooks")] = ['ipynb'];
-		let file = await vscode.window.showOpenDialog({
-			filters: filter
-		});
-		if (file) {
-			let doc = await vscode.workspace.openTextDocument(file[0]);
-			vscode.window.showTextDocument(doc);
-		}
-	} catch (err) {
-		vscode.window.showErrorMessage(getErrorMessage(err));
-	}
-}
-
-async function runActiveCell(): Promise<void> {
-	try {
-		let notebook = azdata.nb.activeNotebookEditor;
-		if (notebook) {
-			await notebook.runCell();
-		} else {
-			throw new Error(noNotebookVisible);
-		}
-	} catch (err) {
-		vscode.window.showErrorMessage(getErrorMessage(err));
-	}
-}
-
-async function clearActiveCellOutput(): Promise<void> {
-	try {
-		let notebook = azdata.nb.activeNotebookEditor;
-		if (notebook) {
-			await notebook.clearOutput();
-		} else {
-			throw new Error(noNotebookVisible);
-		}
-	} catch (err) {
-		vscode.window.showErrorMessage(getErrorMessage(err));
-	}
-}
-
-async function runAllCells(startCell?: azdata.nb.NotebookCell, endCell?: azdata.nb.NotebookCell): Promise<void> {
-	try {
-		let notebook = azdata.nb.activeNotebookEditor;
-		if (notebook) {
-			await notebook.runAllCells(startCell, endCell);
-		} else {
-			throw new Error(noNotebookVisible);
-		}
-	} catch (err) {
-		vscode.window.showErrorMessage(getErrorMessage(err));
-	}
-}
-
-async function addCell(cellType: azdata.nb.CellType): Promise<void> {
-	try {
-		let notebook = azdata.nb.activeNotebookEditor;
-		if (notebook) {
-			await notebook.edit((editBuilder: azdata.nb.NotebookEditorEdit) => {
-				// TODO should prompt and handle cell placement
-				editBuilder.insertCell({
-					cell_type: cellType,
-					source: ''
-				});
-			});
-		} else {
-			throw new Error(noNotebookVisible);
-		}
-	} catch (err) {
-		vscode.window.showErrorMessage(getErrorMessage(err));
-	}
-}
-
-async function analyzeNotebook(oeContext?: azdata.ObjectExplorerContext): Promise<void> {
-	// Ensure we get a unique ID for the notebook. For now we're using a different prefix to the built-in untitled files
-	// to handle this. We should look into improving this in the future
-	let title = findNextUntitledEditorName();
-	let untitledUri = vscode.Uri.parse(`untitled:${title}`);
-
-	let editor = await azdata.nb.showNotebookDocument(untitledUri, {
-		connectionProfile: oeContext ? oeContext.connectionProfile : undefined,
-		providerId: JUPYTER_NOTEBOOK_PROVIDER,
-		preview: false,
-		defaultKernel: {
-			name: 'pysparkkernel',
-			display_name: 'PySpark',
-			language: 'python'
-		}
-	});
-	if (oeContext && oeContext.nodeInfo && oeContext.nodeInfo.nodePath) {
-		// Get the file path after '/HDFS'
-		let hdfsPath: string = oeContext.nodeInfo.nodePath.substring(oeContext.nodeInfo.nodePath.indexOf('/HDFS') + '/HDFS'.length);
-		if (hdfsPath.length > 0) {
-			let analyzeCommand = '#' + msgSampleCodeDataFrame + os.EOL + 'df = (spark.read.option("inferSchema", "true")'
-				+ os.EOL + '.option("header", "true")' + os.EOL + '.csv("{0}"))' + os.EOL + 'df.show(10)';
-
-			editor.edit(editBuilder => {
-				editBuilder.insertCell({
-					cell_type: 'code',
-					source: analyzeCommand.replace('{0}', hdfsPath)
-				}, 0);
-			});
-		}
-	}
-}
-
 // this method is called when your extension is deactivated
 export function deactivate() {
 	if (controller) {

extensions/notebook/src/test/common/notebookUtils.test.ts

@@ -0,0 +1,46 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the Source EULA. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import * as azdata from 'azdata';
+import { newNotebook } from '../../common/notebookUtils';
+import * as should from 'should';
+import * as vscode from 'vscode';
+
+describe('notebookUtils Tests', async function (): Promise<void> {
+	describe('newNotebook', async function (): Promise<void> {
+		it('Should open a new notebook successfully', async function (): Promise<void> {
+			should(azdata.nb.notebookDocuments.length).equal(0, 'There should be not any open Notebook documents');
+			await newNotebook(undefined);
+			should(azdata.nb.notebookDocuments.length).equal(1, 'There should be exactly 1 open Notebook document');
+			await vscode.commands.executeCommand('workbench.action.closeActiveEditor');
+			should(azdata.nb.notebookDocuments.length).equal(0, 'There should be not any open Notebook documents');
+		});
+
+		it('Opening an untitled editor after closing should re-use previous untitled name', async function (): Promise<void> {
+			should(azdata.nb.notebookDocuments.length).equal(0, 'There should be not any open Notebook documents');
+			await newNotebook(undefined);
+			should(azdata.nb.notebookDocuments.length).equal(1, 'There should be exactly 1 open Notebook document');
+			should(azdata.nb.notebookDocuments[0].fileName).equal('Notebook-0', 'The first Untitled Notebook should have an index of 0');
+			await vscode.commands.executeCommand('workbench.action.closeActiveEditor');
+			should(azdata.nb.notebookDocuments.length).equal(0, 'There should be not any open Notebook documents');
+			await newNotebook(undefined);
+			should(azdata.nb.notebookDocuments.length).equal(1, 'There should be exactly 1 open Notebook document after second opening');
+			should(azdata.nb.notebookDocuments[0].fileName).equal('Notebook-0', 'The first Untitled Notebook should have an index of 0 after closing first Untitled Notebook');
+			await vscode.commands.executeCommand('workbench.action.closeActiveEditor');
+		});
+
+		it('Untitled Name index should increase', async function (): Promise<void> {
+			should(azdata.nb.notebookDocuments.length).equal(0, 'There should be not any open Notebook documents');
+			await newNotebook(undefined);
+			should(azdata.nb.notebookDocuments.length).equal(1, 'There should be exactly 1 open Notebook document');
+			const secondNotebook = await newNotebook(undefined);
+			should(azdata.nb.notebookDocuments.length).equal(2, 'There should be exactly 2 open Notebook documents');
+			should(secondNotebook.document.fileName).equal('Notebook-1', 'The second Untitled Notebook should have an index of 1');
+			await vscode.commands.executeCommand('workbench.action.closeActiveEditor');
+			await vscode.commands.executeCommand('workbench.action.closeActiveEditor');
+			should(azdata.nb.notebookDocuments.length).equal(0, 'There should be not any open Notebook documents');
+		});
+	});
+});