Skip to content

Commit

Permalink
Functional tests for native interactive window (#6786)
Browse files Browse the repository at this point in the history
  • Loading branch information
joyceerhl authored Sep 1, 2021
1 parent 661ee81 commit 8042246
Show file tree
Hide file tree
Showing 11 changed files with 316 additions and 54 deletions.
2 changes: 1 addition & 1 deletion .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -370,4 +370,4 @@
}
}
]
}
}
6 changes: 3 additions & 3 deletions src/client/datascience/editor-integration/cellhashprovider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { IConfigurationService } from '../../common/types';
import { getCellResource } from '../cellFactory';
import { CellMatcher } from '../cellMatcher';
import { Identifiers } from '../constants';
import { getInteractiveCellMetadata } from '../interactive-window/nativeInteractiveWindow';
import { getInteractiveCellMetadata } from '../interactive-window/interactiveWindow';
import { ICellHash, ICellHashListener, ICellHashProvider, IFileHashes } from '../types';

// eslint-disable-next-line @typescript-eslint/no-require-imports, @typescript-eslint/no-var-requires
Expand Down Expand Up @@ -134,9 +134,9 @@ export class CellHashProvider implements ICellHashProvider {
// Find the text document that matches. We need more information than
// the add code gives us
const { line: cellLine, file } = cell.metadata.interactive;
const id = getInteractiveCellMetadata(cell).id;
const id = getInteractiveCellMetadata(cell)?.id;
const doc = this.documentManager.textDocuments.find((d) => this.fs.areLocalPathsSame(d.fileName, file));
if (doc) {
if (doc && id) {
// Compute the code that will really be sent to jupyter
const { stripped, trueStartLine } = this.extractStrippedLines(cell);

Expand Down
4 changes: 2 additions & 2 deletions src/client/datascience/editor-integration/codeLensFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { IConfigurationService, IDisposableRegistry, Resource } from '../../comm
import * as localize from '../../common/utils/localize';
import { generateCellRangesFromDocument } from '../cellFactory';
import { CodeLensCommands, Commands } from '../constants';
import { getInteractiveCellMetadata } from '../interactive-window/nativeInteractiveWindow';
import { getInteractiveCellMetadata } from '../interactive-window/interactiveWindow';
import { IKernelProvider } from '../jupyter/kernels/types';
import { InteractiveWindowView } from '../notebook/constants';
import { ICellHashProvider, ICellRange, ICodeLensFactory, IFileHashes } from '../types';
Expand Down Expand Up @@ -184,7 +184,7 @@ export class CodeLensFactory implements ICodeLensFactory {
};
this.notebookData.set(e.cell.notebook.uri.toString(), data);
}
if (data) {
if (data !== undefined && metadata !== undefined) {
data.cellExecutionCounts.set(metadata.id, e.cell.executionSummary.executionOrder);
data.documentExecutionCounts.set(
metadata.interactive.file.toLowerCase(),
Expand Down
7 changes: 5 additions & 2 deletions src/client/datascience/editor-integration/hoverProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { sleep } from '../../common/utils/async';
import { StopWatch } from '../../common/utils/stopWatch';
import { sendTelemetryEvent } from '../../telemetry';
import { Identifiers, Telemetry } from '../constants';
import { getInteractiveCellMetadata } from '../interactive-window/nativeInteractiveWindow';
import { getInteractiveCellMetadata } from '../interactive-window/interactiveWindow';
import { IKernelProvider } from '../jupyter/kernels/types';
import { InteractiveWindowView } from '../notebook/constants';
import {
Expand Down Expand Up @@ -57,7 +57,10 @@ export class HoverProvider implements INotebookExecutionLogger, IHoverProvider {
return;
}
const size = this.runFiles.size;
this.runFiles.add(getInteractiveCellMetadata(e.cell).interactive.file.toLocaleLowerCase());
const metadata = getInteractiveCellMetadata(e.cell);
if (metadata !== undefined) {
this.runFiles.add(metadata.interactive.file.toLocaleLowerCase());
}
if (size !== this.runFiles.size) {
await this.initializeHoverProvider();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,12 @@ type InteractiveCellMetadata = {
};
id: string;
};
export function getInteractiveCellMetadata(cell: NotebookCell): InteractiveCellMetadata {
return cell.metadata as InteractiveCellMetadata;
export function getInteractiveCellMetadata(cell: NotebookCell): InteractiveCellMetadata | undefined {
if (cell.metadata.interactive !== undefined) {
return cell.metadata as InteractiveCellMetadata;
}
}
export class NativeInteractiveWindow implements IInteractiveWindowLoadable {
export class InteractiveWindow implements IInteractiveWindowLoadable {
public get onDidChangeViewState(): Event<void> {
return this._onDidChangeViewState.event;
}
Expand Down Expand Up @@ -500,7 +502,7 @@ export class NativeInteractiveWindow implements IInteractiveWindowLoadable {
const notebookEditor = await this._editorReadyPromise;
const matchingCell = notebookEditor.document
.getCells()
.find((cell) => getInteractiveCellMetadata(cell).id === id);
.find((cell) => getInteractiveCellMetadata(cell)?.id === id);
if (matchingCell) {
this.revealCell(matchingCell, notebookEditor);
}
Expand All @@ -514,13 +516,12 @@ export class NativeInteractiveWindow implements IInteractiveWindowLoadable {
}, 200); // Rendering output is async so the output is not guaranteed to immediately exist
}

// TODO this does not need to be async since we no longer need to roundtrip to the UI
public async hasCell(id: string): Promise<boolean> {
const notebookEditor = await this._editorReadyPromise;
if (!notebookEditor) {
return false;
}
return notebookEditor.document.getCells().some((cell) => getInteractiveCellMetadata(cell).id === id);
return notebookEditor.document.getCells().some((cell) => getInteractiveCellMetadata(cell)?.id === id);
}

public get owningResource(): Resource {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
TextEditor,
Uri,
ViewColumn,
window,
workspace,
WorkspaceEdit
} from 'vscode';
Expand Down Expand Up @@ -134,14 +135,14 @@ export class NativeInteractiveWindowCommandListener implements IDataScienceComma
this.disposableRegistry.push(
commandManager.registerCommand(
Commands.ExpandAllCells,
(context?: { notebookEditor: { notebookUri: Uri } }) =>
async (context?: { notebookEditor: { notebookUri: Uri } }) =>
this.expandAllCells(context?.notebookEditor.notebookUri)
)
);
this.disposableRegistry.push(
commandManager.registerCommand(
Commands.CollapseAllCells,
(context?: { notebookEditor: { notebookUri: Uri } }) =>
async (context?: { notebookEditor: { notebookUri: Uri } }) =>
this.collapseAllCells(context?.notebookEditor.notebookUri)
)
);
Expand Down Expand Up @@ -372,21 +373,19 @@ export class NativeInteractiveWindowCommandListener implements IDataScienceComma
return this.exportDialog.showDialog(ExportFormat.ipynb, file);
}

private expandAllCells(uri?: Uri) {
const interactiveWindow = uri
? this.interactiveWindowProvider.windows.find((window) => window.notebookUri?.toString() === uri.toString())
: this.interactiveWindowProvider.activeWindow;
private async expandAllCells(uri?: Uri) {
const interactiveWindow = this.getTargetInteractiveWindow(uri);
traceInfo(`Expanding all cells in interactive window with uri ${interactiveWindow?.notebookUri}`);
if (interactiveWindow) {
interactiveWindow.expandAllCells();
await interactiveWindow.expandAllCells();
}
}

private collapseAllCells(uri?: Uri) {
const interactiveWindow = uri
? this.interactiveWindowProvider.windows.find((window) => window.notebookUri?.toString() === uri.toString())
: this.interactiveWindowProvider.activeWindow;
private async collapseAllCells(uri?: Uri) {
const interactiveWindow = this.getTargetInteractiveWindow(uri);
traceInfo(`Collapsing all cells in interactive window with uri ${interactiveWindow?.notebookUri}`);
if (interactiveWindow) {
interactiveWindow.collapseAllCells();
await interactiveWindow.collapseAllCells();
}
}

Expand All @@ -398,18 +397,14 @@ export class NativeInteractiveWindowCommandListener implements IDataScienceComma
}

private exportAs(uri?: Uri) {
const interactiveWindow = uri
? this.interactiveWindowProvider.windows.find((window) => window.notebookUri?.toString() === uri.toString())
: this.interactiveWindowProvider.activeWindow;
const interactiveWindow = this.getTargetInteractiveWindow(uri);
if (interactiveWindow) {
interactiveWindow.exportAs();
}
}

private export(uri?: Uri) {
const interactiveWindow = uri
? this.interactiveWindowProvider.windows.find((window) => window.notebookUri?.toString() === uri.toString())
: this.interactiveWindowProvider.activeWindow;
const interactiveWindow = this.getTargetInteractiveWindow(uri);
if (interactiveWindow) {
interactiveWindow.export();
}
Expand Down Expand Up @@ -488,9 +483,7 @@ export class NativeInteractiveWindowCommandListener implements IDataScienceComma
}

private async clearAllCellsInInteractiveWindow(context?: { notebookEditor: { notebookUri: Uri } }): Promise<void> {
// Use the context if invoked from interactive/toolbar
// Then fallback to the active interactive window
const uri = context?.notebookEditor.notebookUri ?? this.interactiveWindowProvider.activeWindow?.notebookUri;
const uri = this.getTargetInteractiveWindow(context?.notebookEditor.notebookUri)?.notebookUri;
if (!uri) {
return;
}
Expand Down Expand Up @@ -551,4 +544,22 @@ export class NativeInteractiveWindowCommandListener implements IDataScienceComma
await this.clipboard.writeText(source);
}
}

private getTargetInteractiveWindow(notebookUri: Uri | undefined) {
let targetInteractiveWindow;
if (notebookUri !== undefined) {
targetInteractiveWindow = this.interactiveWindowProvider.windows.find(
(w) => w.notebookUri?.toString() === notebookUri.toString()
);
} else if (notebookUri === undefined && this.interactiveWindowProvider.activeWindow !== undefined) {
targetInteractiveWindow = this.interactiveWindowProvider.activeWindow;
} else if (
notebookUri === undefined &&
this.interactiveWindowProvider.activeWindow === undefined &&
window.activeTextEditor?.document.uri !== undefined
) {
targetInteractiveWindow = this.interactiveWindowProvider.get(window.activeTextEditor.document.uri);
}
return targetInteractiveWindow;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ import { IExportDialog } from '../export/types';
import { IKernelProvider } from '../jupyter/kernels/types';
import { INotebookControllerManager } from '../notebook/types';
import { IInteractiveWindow, IInteractiveWindowProvider, IJupyterDebugger, INotebookExporter } from '../types';
import { NativeInteractiveWindow } from './nativeInteractiveWindow';
import { InteractiveWindow } from './interactiveWindow';

// Export for testing
export const AskedForPerFileSettingKey = 'ds_asked_per_file_interactive';

@injectable()
export class NativeInteractiveWindowProvider implements IInteractiveWindowProvider, IAsyncDisposable {
export class InteractiveWindowProvider implements IInteractiveWindowProvider, IAsyncDisposable {
public get onDidChangeActiveInteractiveWindow(): Event<IInteractiveWindow | undefined> {
return this._onDidChangeActiveInteractiveWindow.event;
}
Expand All @@ -57,7 +57,7 @@ export class NativeInteractiveWindowProvider implements IInteractiveWindowProvid
private readonly _onDidChangeActiveInteractiveWindow = new EventEmitter<IInteractiveWindow | undefined>();
private readonly _onDidCreateInteractiveWindow = new EventEmitter<IInteractiveWindow>();
private lastActiveInteractiveWindow: IInteractiveWindow | undefined;
private _windows: NativeInteractiveWindow[] = [];
private _windows: InteractiveWindow[] = [];

constructor(
@inject(IServiceContainer) private serviceContainer: IServiceContainer,
Expand Down Expand Up @@ -114,7 +114,7 @@ export class NativeInteractiveWindowProvider implements IInteractiveWindowProvid
private create(resource: Resource, mode: InteractiveWindowMode) {
// Set it as soon as we create it. The .ctor for the interactive window
// may cause a subclass to talk to the IInteractiveWindowProvider to get the active interactive window.
const result = new NativeInteractiveWindow(
const result = new InteractiveWindow(
this.serviceContainer.get<IApplicationShell>(IApplicationShell),
this.serviceContainer.get<IDocumentManager>(IDocumentManager),
this.serviceContainer.get<IFileSystem>(IFileSystem),
Expand Down
6 changes: 3 additions & 3 deletions src/client/datascience/serviceRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,8 @@ import { IApplicationEnvironment } from '../common/application/types';
import { NotebookIPyWidgetCoordinator } from './ipywidgets/notebookIPyWidgetCoordinator';
import { ExtensionRecommendationService } from './extensionRecommendation';
import { PythonVariablesRequester } from './jupyter/pythonVariableRequester';
import { NativeInteractiveWindowCommandListener } from './interactive-window/nativeInteractiveWindowCommandListener';
import { NativeInteractiveWindowProvider } from './interactive-window/nativeInteractiveWindowProvider';
import { NativeInteractiveWindowCommandListener } from './interactive-window/interactiveWindowCommandListener';
import { InteractiveWindowProvider } from './interactive-window/interactiveWindowProvider';
import { JupyterPaths } from './kernel-launcher/jupyterPaths';
import { LocalKnownPathKernelSpecFinder } from './kernel-launcher/localKnownPathKernelSpecFinder';
import { LocalPythonAndRelatedNonPythonKernelSpecFinder } from './kernel-launcher/localPythonAndRelatedNonPythonKernelSpecFinder';
Expand Down Expand Up @@ -238,7 +238,7 @@ export function registerTypes(serviceManager: IServiceManager, inNotebookApiExpe
serviceManager.addSingleton<IExtensionSingleActivationService>(IExtensionSingleActivationService, NotebookUsageTracker);
serviceManager.addSingleton<IExtensionSingleActivationService>(IExtensionSingleActivationService, MigrateJupyterInterpreterStateService);
serviceManager.addSingleton<IExtensionSingleActivationService>(IExtensionSingleActivationService, VariableViewActivationService);
serviceManager.addSingleton<IInteractiveWindowProvider>(IInteractiveWindowProvider, NativeInteractiveWindowProvider);
serviceManager.addSingleton<IInteractiveWindowProvider>(IInteractiveWindowProvider, InteractiveWindowProvider);
serviceManager.addSingleton<IDataScienceCommandListener>(IDataScienceCommandListener, NativeInteractiveWindowCommandListener);
serviceManager.addSingleton<IDataScienceCommandListener>(IDataScienceCommandListener, KernelCommandListener);
serviceManager.addSingleton<IJupyterDebugger>(IJupyterDebugger, JupyterDebugger, undefined, [ICellHashListener]);
Expand Down
4 changes: 2 additions & 2 deletions src/client/datascience/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -519,8 +519,8 @@ export interface IInteractiveWindow extends IInteractiveBase {
editor?: TextEditor,
runningStopWatch?: StopWatch
): Promise<boolean>;
expandAllCells(): void;
collapseAllCells(): void;
expandAllCells(): Promise<void>;
collapseAllCells(): Promise<void>;
exportCells(): void;
scrollToCell(id: string): void;
exportAs(cells?: ICell[]): void;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ suite('CellHashProvider Unit Tests', () => {
line,
originalSource: code
},
executionId: 1
id: 1
},
outputs: [],
mime: undefined
Expand Down
Loading

0 comments on commit 8042246

Please sign in to comment.