Skip to content

Commit 939787b

Browse files
rchiodorchiodo
authored
A bunch of fixes for nightly tests (microsoft#13099)
* A bunch of fixes for nightly tests * Fix python interactive title * Rework container to support rewiring appshell * Broke other remote tests, forgot warning overload. * Some more fixups and hope for multiple * Missing other app shell method * Fix hygiene
1 parent 5d21a13 commit 939787b

File tree

11 files changed

+116
-157
lines changed

11 files changed

+116
-157
lines changed

‎.vscode/launch.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@
285285
"--ui=tdd",
286286
"--recursive",
287287
"--colors",
288-
//"--grep", "<suite name>",
288+
// "--grep", "<suite>",
289289
"--timeout=300000",
290290
"--exit"
291291
],

‎package.nls.json‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -561,9 +561,10 @@
561561
"DataScienceRendererExtension.downloadCompletedOutputMessage": "Notebook Renderers extension download complete.",
562562
"DataScience.uriProviderDescriptionFormat": "{0} (From {1} extension)",
563563
"DataScience.unknownPackage": "unknown",
564-
"DataScience.interactiveWindowTitleFormat": "Python Interactive - {0}",
565-
"DataScience.interactiveWindowModeBannerTitle": "Do you want to open a new Python Interactive window for this file? [More Information](command:workbench.action.openSettings?%5B%22python.dataScience.interactiveWindowMode%22%5D).",
566-
"DataScience.interactiveWindowModeBannerSwitchYes": "Yes",
567-
"DataScience.interactiveWindowModeBannerSwitchAlways": "Always",
568-
"DataScience.interactiveWindowModeBannerSwitchNo": "No"
564+
"DataScience.interactiveWindowTitle" : "Python Interactive",
565+
"DataScience.interactiveWindowTitleFormat" : "Python Interactive - {0}",
566+
"DataScience.interactiveWindowModeBannerTitle" : "Do you want to open a new Python Interactive window for this file? [More Information](command:workbench.action.openSettings?%5B%22python.dataScience.interactiveWindowMode%22%5D).",
567+
"DataScience.interactiveWindowModeBannerSwitchYes" : "Yes",
568+
"DataScience.interactiveWindowModeBannerSwitchAlways" : "Always",
569+
"DataScience.interactiveWindowModeBannerSwitchNo" : "No"
569570
}

‎src/client/common/utils/localize.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ export namespace DataScience {
324324
'{0} (From {1} extension)'
325325
);
326326
export const unknownPackage = localize('DataScience.unknownPackage', 'unknown');
327-
export const interactiveWindowTitle = localize('DataScience.interactiveWindowTitleFormat', 'Python Interactive');
327+
export const interactiveWindowTitle = localize('DataScience.interactiveWindowTitle', 'Python Interactive');
328328
export const interactiveWindowTitleFormat = localize(
329329
'DataScience.interactiveWindowTitleFormat',
330330
'Python Interactive - {0}'

‎src/client/datascience/jupyter/jupyterNotebook.ts‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -225,13 +225,19 @@ export class JupyterNotebookBase implements INotebook {
225225
this.sessionStatusChanged.dispose();
226226
this.onStatusChangedEvent = undefined;
227227
}
228-
229-
traceInfo(`Shutting down session ${this.identity.toString()}`);
230-
if (this.session) {
231-
await this.session.dispose().catch(traceError.bind('Failed to dispose session from JupyterNotebook'));
232-
}
233228
this.loggers.forEach((d) => d.dispose());
234229
this.disposed.fire();
230+
231+
try {
232+
traceInfo(`Shutting down session ${this.identity.toString()}`);
233+
if (this.session) {
234+
await this.session
235+
.dispose()
236+
.catch(traceError.bind('Failed to dispose session from JupyterNotebook'));
237+
}
238+
} catch (exc) {
239+
traceError(`Exception shutting down session `, exc);
240+
}
235241
}
236242
}
237243

‎src/test/datascience/dataScienceIocContainer.ts‎

Lines changed: 48 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { interfaces } from 'inversify';
99
import * as os from 'os';
1010
import * as path from 'path';
1111
import { SemVer } from 'semver';
12-
import { anything, instance, mock, reset, when } from 'ts-mockito';
12+
import { anyString, anything, instance, mock, reset, when } from 'ts-mockito';
1313
import * as TypeMoq from 'typemoq';
1414
import {
1515
CancellationTokenSource,
@@ -20,6 +20,7 @@ import {
2020
FileSystemWatcher,
2121
Memento,
2222
Uri,
23+
WindowState,
2324
WorkspaceFolder,
2425
WorkspaceFoldersChangeEvent
2526
} from 'vscode';
@@ -68,6 +69,7 @@ import {
6869
IDiagnosticsService
6970
} from '../../client/application/diagnostics/types';
7071
import { ApplicationEnvironment } from '../../client/common/application/applicationEnvironment';
72+
import { ApplicationShell } from '../../client/common/application/applicationShell';
7173
import { ClipboardService } from '../../client/common/application/clipboard';
7274
import { VSCodeNotebook } from '../../client/common/application/notebook';
7375
import { TerminalManager } from '../../client/common/application/terminalManager';
@@ -158,6 +160,7 @@ import {
158160
ICryptoUtils,
159161
ICurrentProcess,
160162
IDataScienceSettings,
163+
IDisposable,
161164
IExperimentService,
162165
IExperimentsManager,
163166
IExtensionContext,
@@ -424,7 +427,7 @@ export class DataScienceIocContainer extends UnitTestIocContainer {
424427
}
425428
private static jupyterInterpreters: PythonInterpreter[] = [];
426429
private static foundPythonPath: string | undefined;
427-
public applicationShell!: TypeMoq.IMock<IApplicationShell>;
430+
public applicationShell!: ApplicationShell;
428431
// tslint:disable-next-line:no-any
429432
public datascience!: TypeMoq.IMock<IDataScience>;
430433
public shouldMockJupyter: boolean;
@@ -938,7 +941,7 @@ export class DataScienceIocContainer extends UnitTestIocContainer {
938941
this.serviceManager.addSingletonInstance<ICommandManager>(ICommandManager, this.commandManager);
939942

940943
// Mock the app shell
941-
const appShell = (this.applicationShell = TypeMoq.Mock.ofType<IApplicationShell>());
944+
this.applicationShell = mock(ApplicationShell);
942945
const configurationService = TypeMoq.Mock.ofType<IConfigurationService>();
943946

944947
configurationService.setup((c) => c.getSettings(TypeMoq.It.isAny())).returns(this.getSettings.bind(this));
@@ -948,7 +951,7 @@ export class DataScienceIocContainer extends UnitTestIocContainer {
948951
EnvironmentVariablesProvider
949952
);
950953

951-
this.serviceManager.addSingletonInstance<IApplicationShell>(IApplicationShell, appShell.object);
954+
this.serviceManager.addSingletonInstance<IApplicationShell>(IApplicationShell, instance(this.applicationShell));
952955
this.serviceManager.addSingleton<IClipboard>(IClipboard, ClipboardService);
953956
this.serviceManager.addSingletonInstance<IDocumentManager>(IDocumentManager, this.documentManager);
954957
this.serviceManager.addSingletonInstance<IConfigurationService>(
@@ -1131,35 +1134,52 @@ export class DataScienceIocContainer extends UnitTestIocContainer {
11311134
}
11321135
};
11331136

1134-
appShell.setup((a) => a.showErrorMessage(TypeMoq.It.isAnyString())).returns(() => Promise.resolve(''));
1135-
appShell
1136-
.setup((a) => a.showErrorMessage(TypeMoq.It.isAnyString(), anything()))
1137-
.returns(() => Promise.resolve(''));
1138-
appShell
1139-
.setup((a) => a.showErrorMessage(TypeMoq.It.isAnyString(), anything(), anything()))
1140-
.returns(() => Promise.resolve(''));
1141-
appShell
1142-
.setup((a) => a.showInformationMessage(TypeMoq.It.isAny(), TypeMoq.It.isAny()))
1143-
.returns(() => Promise.resolve(''));
1144-
appShell
1145-
.setup((a) => a.showInformationMessage(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
1146-
.returns((_a1: string, a2: string, _a3: string) => Promise.resolve(a2));
1147-
appShell
1148-
.setup((a) =>
1149-
a.showInformationMessage(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())
1150-
)
1151-
.returns((_a1: string, a2: any, _a3: string, a4: string) => {
1137+
when(this.applicationShell.showErrorMessage(anyString())).thenReturn(Promise.resolve(''));
1138+
when(this.applicationShell.showErrorMessage(anyString(), anything())).thenReturn(Promise.resolve(''));
1139+
when(this.applicationShell.showErrorMessage(anyString(), anything(), anything())).thenReturn(
1140+
Promise.resolve('')
1141+
);
1142+
when(this.applicationShell.showInformationMessage(anyString())).thenReturn(Promise.resolve(''));
1143+
when(this.applicationShell.showInformationMessage(anyString(), anything())).thenReturn(Promise.resolve(''));
1144+
when(
1145+
this.applicationShell.showInformationMessage(anyString(), anything(), anything())
1146+
).thenCall((_a1, a2, _a3) => Promise.resolve(a2));
1147+
when(this.applicationShell.showInformationMessage(anyString(), anything(), anything(), anything())).thenCall(
1148+
(_a1, a2, _a3, a4) => {
11521149
if (typeof a2 === 'string') {
11531150
return Promise.resolve(a2);
11541151
} else {
11551152
return Promise.resolve(a4);
11561153
}
1157-
});
1158-
appShell
1159-
.setup((a) => a.showSaveDialog(TypeMoq.It.isAny()))
1160-
.returns(() => Promise.resolve(Uri.file('test.ipynb')));
1161-
appShell.setup((a) => a.setStatusBarMessage(TypeMoq.It.isAny())).returns(() => dummyDisposable);
1162-
appShell.setup((a) => a.showInputBox(TypeMoq.It.isAny())).returns(() => Promise.resolve('Python'));
1154+
}
1155+
);
1156+
when(this.applicationShell.showWarningMessage(anyString())).thenReturn(Promise.resolve(''));
1157+
when(this.applicationShell.showWarningMessage(anyString(), anything())).thenReturn(Promise.resolve(''));
1158+
when(this.applicationShell.showWarningMessage(anyString(), anything(), anything())).thenCall((_a1, a2, _a3) =>
1159+
Promise.resolve(a2)
1160+
);
1161+
when(this.applicationShell.showWarningMessage(anyString(), anything(), anything(), anything())).thenCall(
1162+
(_a1, a2, _a3, a4) => {
1163+
if (typeof a2 === 'string') {
1164+
return Promise.resolve(a2);
1165+
} else {
1166+
return Promise.resolve(a4);
1167+
}
1168+
}
1169+
);
1170+
when(this.applicationShell.showSaveDialog(anything())).thenReturn(Promise.resolve(Uri.file('test.ipynb')));
1171+
when(this.applicationShell.setStatusBarMessage(anything())).thenReturn(dummyDisposable);
1172+
when(this.applicationShell.showInputBox(anything())).thenReturn(Promise.resolve('Python'));
1173+
const eventCallback = (
1174+
_listener: (e: WindowState) => any,
1175+
_thisArgs?: any,
1176+
_disposables?: IDisposable[] | Disposable
1177+
) => {
1178+
return {
1179+
dispose: noop
1180+
};
1181+
};
1182+
when(this.applicationShell.onDidChangeWindowState).thenReturn(eventCallback);
11631183

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

‎src/test/datascience/interactiveWindow.functional.test.tsx‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -709,6 +709,8 @@ for i in range(0, 100):
709709
);
710710

711711
// Then open a second time and make sure it uses this new path
712+
const secondPath = await interpreterService.getActiveInterpreter(secondUri);
713+
assert.notEqual(secondPath?.path, activeInterpreter?.path, 'Second path was not set');
712714
const newWindow = (await interactiveWindowProvider.getOrCreate(undefined)) as InteractiveWindow;
713715
await addCode(ioc, 'a=1\na', false, secondUri);
714716
assert.notEqual(
@@ -1237,12 +1239,12 @@ for i in range(0, 100):
12371239
const lenses = fooWatcher?.getCodeLenses();
12381240
assert.equal(lenses?.length, 6, 'No code lenses found');
12391241
await runCodeLens(fooWatcher!.uri!, lenses ? lenses[0] : undefined, ioc);
1240-
verifyHtmlOnCell(pair1.mount.wrapper, 'InteractiveCell', '<span>foo</span>', CellPosition.Last);
1242+
verifyHtmlOnCell(pair1.mount.wrapper, 'InteractiveCell', 'foo', CellPosition.Last);
12411243

12421244
// Create another window, run a cell again
12431245
const pair2 = await getOrCreateInteractiveWindow(ioc);
12441246
await runCodeLens(fooWatcher!.uri!, lenses ? lenses[0] : undefined, ioc);
1245-
verifyHtmlOnCell(pair2.mount.wrapper, 'InteractiveCell', '<span>foo</span>', CellPosition.Last);
1247+
verifyHtmlOnCell(pair2.mount.wrapper, 'InteractiveCell', 'foo', CellPosition.Last);
12461248

12471249
// Make the first window active
12481250
pair2.mount.changeViewState(false, false);
@@ -1253,7 +1255,7 @@ for i in range(0, 100):
12531255
const lenses2 = barWatcher?.getCodeLenses();
12541256
assert.equal(lenses2?.length, 6, 'No code lenses found');
12551257
await runCodeLens(barWatcher!.uri!, lenses2 ? lenses2[0] : undefined, ioc);
1256-
verifyHtmlOnCell(pair1.mount.wrapper, 'InteractiveCell', '<span>bar</span>', CellPosition.Last);
1258+
verifyHtmlOnCell(pair1.mount.wrapper, 'InteractiveCell', 'bar', CellPosition.Last);
12571259
});
12581260
test('Per file', async () => {
12591261
addMockData(ioc, fooCode, 'foo');
@@ -1268,7 +1270,7 @@ for i in range(0, 100):
12681270
await runCodeLens(fooWatcher!.uri!, lenses ? lenses[0] : undefined, ioc);
12691271
assert.equal(interactiveWindowProvider.windows.length, 1, 'Interactive window not created');
12701272
const mounted1 = interactiveWindowProvider.getMountedWebView(interactiveWindowProvider.windows[0]);
1271-
verifyHtmlOnCell(mounted1.wrapper, 'InteractiveCell', '<span>foo</span>', CellPosition.Last);
1273+
verifyHtmlOnCell(mounted1.wrapper, 'InteractiveCell', 'foo', CellPosition.Last);
12721274

12731275
// Create another window, run a cell again
12741276
const barWatcher = createCodeWatcher(`# %%\n${barCode}`, 'bar.py', ioc);
@@ -1278,16 +1280,11 @@ for i in range(0, 100):
12781280
const mounted2 = interactiveWindowProvider.getMountedWebView(
12791281
interactiveWindowProvider.windows.find((w) => w.title.includes('bar'))
12801282
);
1281-
verifyHtmlOnCell(mounted2.wrapper, 'InteractiveCell', '<span>bar</span>', CellPosition.Last);
1283+
verifyHtmlOnCell(mounted2.wrapper, 'InteractiveCell', 'bar', CellPosition.Last);
12821284
});
12831285
test('Per file asks and changes titles', async () => {
12841286
addMockData(ioc, fooCode, 'foo');
12851287
addMockData(ioc, barCode, 'bar');
1286-
ioc.applicationShell
1287-
.setup((i) => i.showInformationMessage(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
1288-
.returns((_a1: string, a2: string, _a3: string) => {
1289-
return Promise.resolve(a2);
1290-
});
12911288
ioc.forceDataScienceSettingsChanged({ interactiveWindowMode: 'multiple' });
12921289
const interactiveWindowProvider = ioc.get<ITestInteractiveWindowProvider>(IInteractiveWindowProvider);
12931290
const globalMemento = ioc.get<Memento>(IMemento, GLOBAL_MEMENTO);
@@ -1300,7 +1297,7 @@ for i in range(0, 100):
13001297
await runCodeLens(fooWatcher!.uri!, lenses ? lenses[0] : undefined, ioc);
13011298
assert.equal(interactiveWindowProvider.windows.length, 1, 'Interactive window not created');
13021299
const mounted1 = interactiveWindowProvider.getMountedWebView(interactiveWindowProvider.windows[0]);
1303-
verifyHtmlOnCell(mounted1.wrapper, 'InteractiveCell', '<span>foo</span>', CellPosition.Last);
1300+
verifyHtmlOnCell(mounted1.wrapper, 'InteractiveCell', 'foo', CellPosition.Last);
13041301

13051302
// Create another window, run a cell again
13061303
const barWatcher = createCodeWatcher(`# %%\n${barCode}`, 'bar.py', ioc);
@@ -1310,7 +1307,7 @@ for i in range(0, 100):
13101307
const mounted2 = interactiveWindowProvider.getMountedWebView(
13111308
interactiveWindowProvider.windows.find((w) => w.title.includes('bar'))
13121309
);
1313-
verifyHtmlOnCell(mounted2.wrapper, 'InteractiveCell', '<span>bar</span>', CellPosition.Last);
1310+
verifyHtmlOnCell(mounted2.wrapper, 'InteractiveCell', 'bar', CellPosition.Last);
13141311

13151312
// First window should now have foo in the title too
13161313
assert.ok(interactiveWindowProvider.windows[0].title.includes('foo'), 'Title of first window did not change');

‎src/test/datascience/nativeEditor.functional.test.tsx‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import * as chaiAsPromised from 'chai-as-promised';
77
import * as dedent from 'dedent';
88
import { ReactWrapper } from 'enzyme';
99
import * as fs from 'fs-extra';
10+
import { IDisposable } from 'monaco-editor';
1011
import * as path from 'path';
1112
import * as sinon from 'sinon';
1213
import { anything, objectContaining, when } from 'ts-mockito';
@@ -903,7 +904,9 @@ df.head()`;
903904
await promise;
904905
}
905906
}
906-
await ioc.dispose();
907+
if (ioc) {
908+
await ioc.dispose();
909+
}
907910
try {
908911
notebookFile.cleanupCallback();
909912
} catch {
@@ -2074,11 +2077,19 @@ df.head()`;
20742077
}
20752078
await initIoc();
20762079

2080+
const eventCallback = (
2081+
listener: (e: WindowState) => any,
2082+
_thisArgs?: any,
2083+
_disposables?: IDisposable[] | Disposable
2084+
) => {
2085+
windowStateChangeHandlers.push(listener);
2086+
return {
2087+
dispose: noop
2088+
};
2089+
};
20772090
windowStateChangeHandlers = [];
20782091
// Keep track of all handlers for the onDidChangeWindowState event.
2079-
ioc.applicationShell
2080-
.setup((app) => app.onDidChangeWindowState(TypeMoq.It.isAny()))
2081-
.callback((cb) => windowStateChangeHandlers.push(cb));
2092+
when(ioc.applicationShell.onDidChangeWindowState).thenReturn(eventCallback);
20822093

20832094
// tslint:disable-next-line: no-invalid-this
20842095
await setupFunction.call(this);

0 commit comments

Comments
 (0)