Skip to content

Commit

Permalink
Fixes to failing functional tests (#67)
Browse files Browse the repository at this point in the history
* Fixes to failing functional tests

* Fixes
  • Loading branch information
DonJayamanne authored Sep 17, 2020
1 parent 44897e2 commit c280df1
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 36 deletions.
5 changes: 4 additions & 1 deletion .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,10 @@
// Remove 'X' to turn on all logging in the debug output
"XVSC_PYTHON_FORCE_LOGGING": "1",
// Remove `X` prefix and update path to test with real python interpreter (for DS functional tests).
"XCI_PYTHON_PATH": "<Python Path>",
"XCI_PYTHON_PATH": "/Users/donjayamanne/Desktop/Development/crap/docBug/venv2/bin/python",
// Remove 'X' and initialize with second Python interpreter to be used for fucntional tests.
// Some tests require multiple python interpreters (do not rely on discovery for functional tests, be explicit).
"XCI_PYTHON_PATH2": "/Users/donjayamanne/Desktop/Development/crap/docBug/venv3/bin/python",
// Remove 'X' prefix to dump output for debugger. Directory has to exist prior to launch
"XDEBUGPY_LOG_DIR": "${workspaceRoot}/tmp/Debug_Output"
},
Expand Down
6 changes: 2 additions & 4 deletions src/test/datascience/dataScienceIocContainer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -876,9 +876,6 @@ export class DataScienceIocContainer extends UnitTestIocContainer {
when(this.applicationShell.onDidChangeWindowState).thenReturn(eventCallback);
when(this.applicationShell.withProgress(anything(), anything())).thenCall((_o, c) => c());

const interpreterManager = this.serviceContainer.get<InterpreterService>(IInterpreterService);
interpreterManager.initialize();

if (this.mockJupyter) {
this.addInterpreter(this.workingPython2, SupportedCommands.all);
this.addInterpreter(this.workingPython, SupportedCommands.all);
Expand Down Expand Up @@ -1069,7 +1066,7 @@ export class DataScienceIocContainer extends UnitTestIocContainer {
panel.attach(options);
return panel;
}
private forceSettingsChanged(resource: Resource, _newPath: string, partial: Partial<IJupyterSettings>) {
private forceSettingsChanged(resource: Resource, newPath: string, partial: Partial<IJupyterSettings>) {
// tslint:disable-next-line: no-suspicious-comment
// TODO: Python path will not be updated by this code so tests are unlikely to pass
const settings = this.getSettings(resource) as MockJupyterSettings;
Expand All @@ -1091,6 +1088,7 @@ export class DataScienceIocContainer extends UnitTestIocContainer {
return true;
}
});
this.get<InterpreterService>(IInterpreterService).updateInterpreter(resource, newPath);
}

private generateJupyterSettings() {
Expand Down
7 changes: 3 additions & 4 deletions src/test/datascience/intellisense.functional.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { INotebookEditorProvider } from '../../client/datascience/types';
import { IInterpreterService } from '../../client/interpreter/contracts';
import { MonacoEditor } from '../../datascience-ui/react-common/monacoEditor';
import { noop } from '../core';
import { InterpreterService } from '../interpreters/interpreterService';
import { DataScienceIocContainer } from './dataScienceIocContainer';
import { takeSnapshot, writeDiffSnapshot } from './helpers';
import * as InteractiveHelpers from './interactiveWindowTestHelpers';
Expand Down Expand Up @@ -249,11 +250,9 @@ import { ITestNativeEditorProvider } from './testNativeEditorProvider';
const oldActive = await interpreterService.getActiveInterpreter(undefined);
const interpreters = await interpreterService.getInterpreters(undefined);
if (interpreters.length > 1 && oldActive) {
//const firstOther = interpreters.filter((i) => i.path !== oldActive.path);
const firstOther = interpreters.filter((i) => i.path !== oldActive.path);

// tslint:disable-next-line: no-suspicious-comment
// TODO: Need to be able to change the interpreter too in the python API
//ioc.forceSettingsChanged(undefined, firstOther[0].path);
ioc.get<InterpreterService>(IInterpreterService).updateInterpreter(undefined, firstOther[0].path);
const active = await interpreterService.getActiveInterpreter(undefined);
assert.notDeepEqual(active, oldActive, 'Should have changed interpreter');
}
Expand Down
12 changes: 6 additions & 6 deletions src/test/datascience/interactiveWindow.functional.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { InteractivePanel } from '../../datascience-ui/history-react/interactive
import { IKeyboardEvent } from '../../datascience-ui/react-common/event';
import { ImageButton } from '../../datascience-ui/react-common/imageButton';
import { MonacoEditor } from '../../datascience-ui/react-common/monacoEditor';
import { InterpreterService } from '../interpreters/interpreterService';
import { DataScienceIocContainer } from './dataScienceIocContainer';
import { createDocument } from './editor-integration/helpers';
import { defaultDataScienceSettings, takeSnapshot, writeDiffSnapshot } from './helpers';
Expand Down Expand Up @@ -711,11 +712,10 @@ for i in range(0, 100):
ioc.addResourceToFolder(secondUri, path.join(EXTENSION_ROOT_DIR, 'src', 'test', 'datascience2'));

// tslint:disable-next-line: no-suspicious-comment
// TODO: Need to be able to change the current interpreter using the python API
// ioc.forceSettingsChanged(
// secondUri,
// interpreters.filter((i) => i.path !== activeInterpreter?.path)[0].path
// );
ioc.get<InterpreterService>(IInterpreterService).updateInterpreter(
secondUri,
interpreters.filter((i) => i.path !== activeInterpreter?.path)[0].path
);

// Then open a second time and make sure it uses this new path
const secondPath = await interpreterService.getActiveInterpreter(secondUri);
Expand All @@ -725,7 +725,7 @@ for i in range(0, 100):
assert.notEqual(
newWindow.notebook!.getMatchingInterpreter()?.path,
activeInterpreter?.path,
'Active intrepreter used to launch second notebook when it should not have'
'Active interpreter used to launch second notebook when it should not have'
);
verifyHtmlOnCell(ioc.getWrapper('interactive'), 'InteractiveCell', '<span>1</span>', CellPosition.Last);
} else {
Expand Down
5 changes: 2 additions & 3 deletions src/test/datascience/notebook.functional.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import { PythonEnvironment } from '../../client/pythonEnvironments/info';
import { concatMultilineString } from '../../datascience-ui/common';
import { generateTestState, ICellViewModel } from '../../datascience-ui/interactive-common/mainState';
import { sleep } from '../core';
import { InterpreterService } from '../interpreters/interpreterService';
import { DataScienceIocContainer } from './dataScienceIocContainer';
import { takeSnapshot, writeDiffSnapshot } from './helpers';
import { SupportedCommands } from './mockJupyterManager';
Expand Down Expand Up @@ -815,9 +816,7 @@ suite('DataScience notebook tests', () => {
}

// Force a settings changed so that all of the cached data is cleared
// tslint:disable-next-line: no-suspicious-comment
// TODO: Need a way to force cache data to clear
// ioc.forceSettingsChanged(undefined, '/usr/bin/test3/python');
ioc.get<InterpreterService>(IInterpreterService).updateInterpreter(undefined, 'bogus');

assert.ok(
await testCancelableMethod(
Expand Down
1 change: 1 addition & 0 deletions src/test/interpreters/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export async function getInterpreterInfo(pythonPath: string): Promise<PythonEnvi
const rawVersion = `${json.versionInfo.slice(0, 3).join('.')}-${json.versionInfo[3]}`;
return {
path: pythonPath,
displayName: `Python${rawVersion}`,
version: parsePythonVersion(rawVersion),
sysVersion: json.sysVersion,
sysPrefix: json.sysPrefix
Expand Down
61 changes: 43 additions & 18 deletions src/test/interpreters/interpreterService.ts
Original file line number Diff line number Diff line change
@@ -1,40 +1,65 @@
import { injectable } from 'inversify';
/* eslint-disable implicit-arrow-linebreak */
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import { EventEmitter, Uri } from 'vscode';
import { injectable } from 'inversify';
import { Event, EventEmitter, Uri } from 'vscode';
import { getInterpreterInfo } from '.';
import { Resource } from '../../client/common/types';
import { IInterpreterService } from '../../client/interpreter/contracts';
import { PythonEnvironment } from '../../client/pythonEnvironments/info';

let interpretersCache: Promise<PythonEnvironment[]> | undefined;
@injectable()
export class InterpreterService implements IInterpreterService {
public readonly onDidChangeInterpreter = new EventEmitter<void>().event;
public readonly didChangeInterpreter = new EventEmitter<void>();

public get onDidChangeInterpreter(): Event<void> {
return this.didChangeInterpreter.event;
}

private readonly customInterpretersPerUri = new Map<string, string>();

public async getInterpreters(_resource?: Uri): Promise<PythonEnvironment[]> {
const [active, globalInterpreter] = await Promise.all([
getInterpreterInfo(process.env.CI_PYTHON_PATH as string),
getInterpreterInfo('python')
]);
const interpreters: PythonEnvironment[] = [];
if (active) {
interpreters.push(active);
}
if (globalInterpreter) {
interpreters.push(globalInterpreter);
if (interpretersCache) {
return interpretersCache;
}
return interpreters;
interpretersCache = getAllInterpreters();
return interpretersCache;
}

public async getActiveInterpreter(_resource?: Uri): Promise<PythonEnvironment | undefined> {
return getInterpreterInfo(process.env.CI_PYTHON_PATH || 'python');
public async getActiveInterpreter(resource?: Uri): Promise<PythonEnvironment | undefined> {
const pythonPath = this.customInterpretersPerUri.has(resource?.fsPath || '')
? this.customInterpretersPerUri.get(resource?.fsPath || '')!
: process.env.CI_PYTHON_PATH || 'python';
return getInterpreterInfo(pythonPath);
}

public async getInterpreterDetails(pythonPath: string, _resource?: Uri): Promise<undefined | PythonEnvironment> {
return getInterpreterInfo(pythonPath);
}

public initialize(): void {
// Noop.
public updateInterpreter(resource: Resource, pythonPath: string): void {
if (pythonPath.trim().length > 0) {
this.customInterpretersPerUri.set(resource?.fsPath || '', pythonPath);
}
this.didChangeInterpreter.fire();
}
}

async function getAllInterpreters(): Promise<PythonEnvironment[]> {
const allInterpreters = await Promise.all([
getInterpreterInfo(process.env.CI_PYTHON_PATH as string),
getInterpreterInfo(process.env.CI_PYTHON_PATH2 as string),
getInterpreterInfo('python')
]);
const interpreters: PythonEnvironment[] = [];
const items = new Set<string>();
allInterpreters.forEach((item) => {
if (item && !items.has(item.path)) {
items.add(item.path);
interpreters.push(item);
}
});
return interpreters;
}

0 comments on commit c280df1

Please sign in to comment.