From dff43687e2e839a166445a4a419f8a2c6a11c364 Mon Sep 17 00:00:00 2001 From: Charles Gagnon Date: Wed, 10 Jun 2020 13:44:38 -0700 Subject: [PATCH] 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 | 4 +- extensions/notebook/src/common/constants.ts | 2 + .../notebook/src/common/notebookUtils.ts | 145 ++++++++++++++++++ extensions/notebook/src/common/utils.ts | 4 +- extensions/notebook/src/extension.ts | 140 +---------------- .../src/test/common/notebookUtils.test.ts | 46 ++++++ 6 files changed, 200 insertions(+), 141 deletions(-) create mode 100644 extensions/notebook/src/test/common/notebookUtils.test.ts diff --git a/extensions/notebook/coverConfig.json b/extensions/notebook/coverConfig.json index 7c935463c5..3901ca2fb5 100644 --- a/extensions/notebook/coverConfig.json +++ b/extensions/notebook/coverConfig.json @@ -3,8 +3,10 @@ "relativeSourcePath": "..", "relativeCoverageDir": "../../coverage", "ignorePatterns": [ + "**/integrationTest/**", "**/node_modules/**", - "**/test/**" + "**/test/**", + "extension.js" ], "reports": [ "cobertura", diff --git a/extensions/notebook/src/common/constants.ts b/extensions/notebook/src/common/constants.ts index 0fc468c788..b3f0e6491f 100644 --- a/extensions/notebook/src/common/constants.ts +++ b/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']; diff --git a/extensions/notebook/src/common/notebookUtils.ts b/extensions/notebook/src/common/notebookUtils.ts index 027a6099c7..e70084fd8a 100644 --- a/extensions/notebook/src/common/notebookUtils.ts +++ b/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 { }); }); } + +export async function newNotebook(connectionProfile?: azdata.IConnectionProfile): Promise { + 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 { + try { + let filter: { [key: string]: Array } = {}; + // 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 { + 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 { + 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 { + 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 { + 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 { + // 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); + }); + } + } +} diff --git a/extensions/notebook/src/common/utils.ts b/extensions/notebook/src/common/utils.ts index 98450a349e..c253cc4959 100644 --- a/extensions/notebook/src/common/utils.ts +++ b/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; } diff --git a/extensions/notebook/src/extension.ts b/extensions/notebook/src/extension.ts index 5f16ab543d..eca5d4109b 100644 --- a/extensions/notebook/src/extension.ts +++ b/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 { - 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 { - try { - let filter: { [key: string]: Array } = {}; - // 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 { - 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 { - 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 { - 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 { - 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 { - // 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) { diff --git a/extensions/notebook/src/test/common/notebookUtils.test.ts b/extensions/notebook/src/test/common/notebookUtils.test.ts new file mode 100644 index 0000000000..6462c8ca78 --- /dev/null +++ b/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 { + describe('newNotebook', async function (): Promise { + it('Should open a new notebook successfully', async function (): Promise { + 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 { + 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 { + 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'); + }); + }); +});