Skip to content

Get more nightly tests to pass #11881

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
May 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions build/conda-functional-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@ django
isort
pathlib2>=2.2.0 ; python_version<'3.6' # Python 2.7 compatibility (pytest)
pythreejs
ipysheet
ipyvolume
beakerx
py4j
bqplot
K3D
debugpy
22 changes: 22 additions & 0 deletions src/client/datascience/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ import type { nbformat } from '@jupyterlab/coreutils';
import { Memento } from 'vscode';
import { splitMultilineString } from '../../datascience-ui/common';
import { noop } from '../common/utils/misc';
import { traceError, traceInfo } from '../logging';
import { Settings } from './constants';
import { ICell } from './types';

// Can't figure out a better way to do this. Enumerate
// the allowed keys of different output formats.
Expand Down Expand Up @@ -98,3 +100,23 @@ export function pruneCell(cell: nbformat.ICell): nbformat.ICell {

return result;
}

export function traceCellResults(prefix: string, results: ICell[]) {
if (results.length > 0 && results[0].data.cell_type === 'code') {
const cell = results[0].data as nbformat.ICodeCell;
const error = cell.outputs && cell.outputs[0] ? cell.outputs[0].evalue : undefined;
if (error) {
traceError(`${prefix} Error : ${error}`);
} else if (cell.outputs && cell.outputs[0]) {
if (cell.outputs[0].output_type.includes('image')) {
traceInfo(`${prefix} Output: image`);
} else {
const data = cell.outputs[0].data;
const text = cell.outputs[0].text;
traceInfo(`${prefix} Output: ${text || JSON.stringify(data)}`);
}
}
} else {
traceInfo(`${prefix} no output.`);
}
}
27 changes: 6 additions & 21 deletions src/client/datascience/jupyter/jupyterDebugger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { IConfigurationService, IExperimentsManager, Version } from '../../commo
import * as localize from '../../common/utils/localize';
import { EXTENSION_ROOT_DIR } from '../../constants';
import { captureTelemetry, sendTelemetryEvent } from '../../telemetry';
import { traceCellResults } from '../common';
import { Identifiers, Telemetry } from '../constants';
import {
CellState,
Expand Down Expand Up @@ -164,30 +165,14 @@ export class JupyterDebugger implements IJupyterDebugger, ICellHashListener {
if (importResults.length === 0 || importResults[0].state === CellState.error) {
traceWarning(`${this.debuggerPackage} not found in path.`);
} else {
this.traceCellResults('import startup', importResults);
traceCellResults('import startup', importResults);
}

// Then enable tracing
await this.executeSilently(notebook, this.tracingEnableCode);
}
}

private traceCellResults(prefix: string, results: ICell[]) {
if (results.length > 0 && results[0].data.cell_type === 'code') {
const cell = results[0].data as nbformat.ICodeCell;
const error = cell.outputs && cell.outputs[0] ? cell.outputs[0].evalue : undefined;
if (error) {
traceError(`${prefix} Error : ${error}`);
} else if (cell.outputs && cell.outputs[0]) {
const data = cell.outputs[0].data;
const text = cell.outputs[0].text;
traceInfo(`${prefix} Output: ${text || JSON.stringify(data)}`);
}
} else {
traceInfo(`${prefix} no output.`);
}
}

private async connect(
notebook: INotebook,
runByLine: boolean,
Expand Down Expand Up @@ -322,7 +307,7 @@ export class JupyterDebugger implements IJupyterDebugger, ICellHashListener {
notebook,
`import sys\r\nsys.path.extend([${debuggerPathList}])\r\nsys.path`
);
this.traceCellResults('Appending paths', result);
traceCellResults('Appending paths', result);
}
}

Expand Down Expand Up @@ -369,7 +354,7 @@ export class JupyterDebugger implements IJupyterDebugger, ICellHashListener {
purpose: 'parsePtvsdVersionInfo' | 'parseDebugpyVersionInfo' | 'pythonVersionInfo'
): Version | undefined {
if (cells.length < 1 || cells[0].state !== CellState.finished) {
this.traceCellResults(purpose, cells);
traceCellResults(purpose, cells);
return undefined;
}

Expand Down Expand Up @@ -397,7 +382,7 @@ export class JupyterDebugger implements IJupyterDebugger, ICellHashListener {
}
}

this.traceCellResults(purpose, cells);
traceCellResults(purpose, cells);

return undefined;
}
Expand Down Expand Up @@ -452,7 +437,7 @@ export class JupyterDebugger implements IJupyterDebugger, ICellHashListener {
return;
}
}
this.traceCellResults(`Installing ${this.debuggerPackage}`, debuggerInstallResults);
traceCellResults(`Installing ${this.debuggerPackage}`, debuggerInstallResults);
sendTelemetryEvent(Telemetry.PtvsdInstallFailed);
traceError(`Failed to install ${this.debuggerPackage}`);
// Failed to install debugger, throw to exit debugging
Expand Down
2 changes: 1 addition & 1 deletion src/test/common/platform/filesystem.functional.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -592,7 +592,7 @@ suite('FileSystem', () => {

suite('createWriteStream', () => {
test('wraps the low-level impl', async () => {
const filename = await fix.resolve('x/y/z/spam.py');
const filename = await fix.resolve('x/y/z/spam2.py');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this change for?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, this test is randomly failing. Been trying to figure out why. Near as I can tell is that the file exists when it shouldn't (read test is interfering with the write test).

I can't repro locally, but I figured I'd try this instead of just disabling the test.

const expected = fs.createWriteStream(filename);
expected.destroy();

Expand Down
10 changes: 9 additions & 1 deletion src/test/datascience/dataScienceIocContainer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,7 @@ import { registerInterpreterTypes } from '../../client/interpreter/serviceRegist
import { VirtualEnvironmentManager } from '../../client/interpreter/virtualEnvs';
import { IVirtualEnvironmentManager } from '../../client/interpreter/virtualEnvs/types';
import { LanguageServerSurveyBanner } from '../../client/languageServices/languageServerSurveyBanner';
import { traceInfo } from '../../client/logging';
import { CodeExecutionHelper } from '../../client/terminals/codeExecution/helper';
import { ICodeExecutionHelper } from '../../client/terminals/types';
import { IVsCodeApi } from '../../datascience-ui/react-common/postOffice';
Expand All @@ -401,6 +402,7 @@ import { MockLiveShareApi } from './mockLiveShare';
import { MockPythonSettings } from './mockPythonSettings';
import { MockWorkspaceConfiguration } from './mockWorkspaceConfig';
import { MockWorkspaceFolder } from './mockWorkspaceFolder';
import { TestExecutionLogger } from './testexecutionLogger';
import { TestInteractiveWindowProvider } from './testInteractiveWindowProvider';
import { TestNativeEditorProvider } from './testNativeEditorProvider';
import { TestPersistentStateFactory } from './testPersistentStateFactory';
Expand Down Expand Up @@ -489,6 +491,9 @@ export class DataScienceIocContainer extends UnitTestIocContainer {
}

public async dispose(): Promise<void> {
// Make sure to disable all command handling during dispose. Don't want
// anything to startup again.
this.commandManager.dispose();
try {
// Make sure to delete any temp files written by native editor storage
const globPr = promisify(glob);
Expand Down Expand Up @@ -793,6 +798,7 @@ export class DataScienceIocContainer extends UnitTestIocContainer {
this.serviceManager.add<INotebookExecutionLogger>(INotebookExecutionLogger, HoverProvider);
this.serviceManager.add<IGatherProvider>(IGatherProvider, GatherProvider);
this.serviceManager.add<IGatherLogger>(IGatherLogger, GatherLogger, undefined, [INotebookExecutionLogger]);
this.serviceManager.add<INotebookExecutionLogger>(INotebookExecutionLogger, TestExecutionLogger);
this.serviceManager.addSingleton<ICodeLensFactory>(ICodeLensFactory, CodeLensFactory, undefined, [
IInteractiveWindowListener
]);
Expand Down Expand Up @@ -1244,8 +1250,10 @@ export class DataScienceIocContainer extends UnitTestIocContainer {
this.forceSettingsChanged(undefined, list[0].path);

// Log this all the time. Useful in determining why a test may not pass.
const message = `Setting interpreter to ${list[0].displayName || list[0].path} -> ${list[0].path}`;
traceInfo(message);
// tslint:disable-next-line: no-console
console.log(`Setting interpreter to ${list[0].displayName || list[0].path} -> ${list[0].path}`);
console.log(message);

// Also set this as the interpreter to use for jupyter
await this.serviceManager
Expand Down
4 changes: 3 additions & 1 deletion src/test/datascience/mockCommandManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@ import { ICommandNameArgumentTypeMapping } from '../../client/common/application
import { ICommandManager } from '../../client/common/application/types';

// tslint:disable:no-any no-http-string no-multiline-string max-func-body-length

export class MockCommandManager implements ICommandManager {
private commands: Map<string, (...args: any[]) => any> = new Map<string, (...args: any[]) => any>();

public dispose() {
this.commands.clear();
}
public registerCommand<
E extends keyof ICommandNameArgumentTypeMapping,
U extends ICommandNameArgumentTypeMapping[E]
Expand Down
22 changes: 22 additions & 0 deletions src/test/datascience/testexecutionLogger.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
import { injectable } from 'inversify';
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note, this file will only be added to the loggers during testing. It simply logs the cell execution.

import { traceCellResults } from '../../client/datascience/common';
import { ICell, INotebookExecutionLogger } from '../../client/datascience/types';
import { traceInfo } from '../../client/logging';
import { concatMultilineStringInput } from '../../datascience-ui/common';

@injectable()
export class TestExecutionLogger implements INotebookExecutionLogger {
public preExecute(cell: ICell, _silent: boolean): Promise<void> {
traceInfo(`Cell Execution for ${cell.id} : \n${concatMultilineStringInput(cell.data.source)}\n`);
return Promise.resolve();
}
public postExecute(cell: ICell, _silent: boolean): Promise<void> {
traceCellResults(`Cell Execution complete for ${cell.id}\n`, [cell]);
return Promise.resolve();
}
public onKernelRestarted(): void {
// Can ignore this.
}
}