Skip to content

Commit 0b2504a

Browse files
committed
refactor(deepnote): Improve mock child process and server output management
- Enhanced `createMockChildProcess` to provide a more comprehensive mock implementation for testing. - Updated `DeepnoteServerStarter` to ensure proper disposal of existing disposables when monitoring server output, improving resource management. - Adjusted error handling in server startup to streamline diagnostics and output tracking.
1 parent c160ae3 commit 0b2504a

4 files changed

Lines changed: 143 additions & 29 deletions

File tree

src/kernels/deepnote/deepnoteServerStarter.node.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
280280

281281
throw new DeepnoteServerStartupError(
282282
interpreter.uri.fsPath,
283-
serverInfo?.jupyterPort ?? 0,
283+
0,
284284
'unknown',
285285
capturedOutput?.stdout || '',
286286
capturedOutput?.stderr || '',
@@ -402,6 +402,12 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
402402
*/
403403
private monitorServerOutput(fileKey: string, serverInfo: DeepnoteServerInfo): void {
404404
const proc = serverInfo.process;
405+
const existing = this.disposablesByFile.get(fileKey);
406+
if (existing) {
407+
for (const d of existing) {
408+
d.dispose();
409+
}
410+
}
405411
const disposables: IDisposable[] = [];
406412
this.disposablesByFile.set(fileKey, disposables);
407413

src/kernels/deepnote/deepnoteTestHelpers.node.ts

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,66 @@ import type { ChildProcess } from 'node:child_process';
55
* Satisfies the ChildProcess interface with minimal stub values.
66
*/
77
export function createMockChildProcess(overrides?: Partial<ChildProcess>): ChildProcess {
8-
return {
8+
const mockProcess: ChildProcess = {
99
pid: undefined,
10+
stdio: [null, null, null, null, null],
11+
stdin: null,
1012
stdout: null,
1113
stderr: null,
1214
exitCode: null,
15+
killed: false,
16+
connected: false,
17+
signalCode: null,
18+
spawnargs: [],
19+
spawnfile: '',
20+
kill: () => true,
21+
send: () => true,
22+
disconnect: () => true,
23+
unref: () => true,
24+
ref: () => true,
25+
addListener: function () {
26+
return this;
27+
},
28+
emit: () => true,
29+
on: function () {
30+
return this;
31+
},
32+
once: function () {
33+
return this;
34+
},
35+
removeListener: function () {
36+
return this;
37+
},
38+
removeAllListeners: function () {
39+
return this;
40+
},
41+
prependListener: function () {
42+
return this;
43+
},
44+
prependOnceListener: function () {
45+
return this;
46+
},
47+
[Symbol.dispose]: () => {
48+
return undefined;
49+
},
50+
off: function () {
51+
return this;
52+
},
53+
setMaxListeners: function () {
54+
return this;
55+
},
56+
getMaxListeners: () => 10,
57+
listeners: function () {
58+
return [];
59+
},
60+
rawListeners: function () {
61+
return [];
62+
},
63+
eventNames: function () {
64+
return [];
65+
},
66+
listenerCount: () => 0,
1367
...overrides
14-
} as ChildProcess;
68+
};
69+
return mockProcess;
1570
}

src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1264,7 +1264,6 @@ project:
12641264

12651265
suite('multi-notebook file sync', () => {
12661266
let workspaceSetCaptures: { uriKey: string; cellSourceJoined: string }[] = [];
1267-
let workspaceEditSetStub: sinon.SinonStub | undefined;
12681267

12691268
setup(() => {
12701269
reset(mockedNotebookManager);
@@ -1273,24 +1272,27 @@ project:
12731272
when(mockedNotebookManager.clearNotebookSelection(anything())).thenReturn();
12741273
resetCalls(mockedNotebookManager);
12751274
workspaceSetCaptures = [];
1276-
workspaceEditSetStub = sinon.stub(WorkspaceEdit.prototype, 'set').callsFake((uri: Uri, edits: unknown) => {
1277-
if (!Array.isArray(edits) || edits.length === 0) {
1278-
return;
1279-
}
1275+
when(mockedVSCodeNamespaces.workspace.applyEdit(anything())).thenCall((wsEdit: WorkspaceEdit) => {
1276+
applyEditCount++;
1277+
1278+
const ext = wsEdit as WorkspaceEdit & { notebookEntries(): [Uri, NotebookEdit[]][] };
12801279

1281-
const firstEdit = edits[0] as NotebookEdit;
1282-
if (firstEdit?.newCells && firstEdit.newCells.length > 0) {
1283-
workspaceSetCaptures.push({
1284-
uriKey: uri.toString(),
1285-
cellSourceJoined: firstEdit.newCells.map((c) => c.value).join('\n')
1286-
});
1280+
for (const [uri, edits] of ext.notebookEntries()) {
1281+
if (!edits.length) {
1282+
continue;
1283+
}
1284+
1285+
const firstEdit = edits[0];
1286+
if (firstEdit?.newCells && firstEdit.newCells.length > 0) {
1287+
workspaceSetCaptures.push({
1288+
uriKey: uri.toString(),
1289+
cellSourceJoined: firstEdit.newCells.map((c) => c.value).join('\n')
1290+
});
1291+
}
12871292
}
1288-
});
1289-
});
12901293

1291-
teardown(() => {
1292-
workspaceEditSetStub?.restore();
1293-
workspaceEditSetStub = undefined;
1294+
return Promise.resolve(true);
1295+
});
12941296
});
12951297

12961298
test('should reload each notebook with its own content when multiple notebooks are open', async () => {

src/test/mocks/vsc/extHostedTypes.ts

Lines changed: 61 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -707,6 +707,7 @@ export namespace vscMockExtHostedTypes {
707707
private _seqPool: number = 0;
708708

709709
private _resourceEdits: { seq: number; from: vscUri.URI; to: vscUri.URI }[] = [];
710+
private _notebookEdits = new Map<string, { uri: vscUri.URI; edits: NotebookEdit[] }>();
710711
private _textEdits = new Map<string, { seq: number; uri: vscUri.URI; edits: TextEdit[] }>();
711712

712713
// createResource(uri: vscode.Uri): void {
@@ -759,7 +760,20 @@ export namespace vscMockExtHostedTypes {
759760
}
760761

761762
has(uri: vscUri.URI): boolean {
762-
return this._textEdits.has(uri.toString());
763+
return this._textEdits.has(uri.toString()) || this._notebookEdits.has(uri.toString());
764+
}
765+
766+
/**
767+
* Notebook edits passed to {@link set} (for tests that inspect edits via workspace.applyEdit).
768+
*/
769+
notebookEntries(): [vscUri.URI, NotebookEdit[]][] {
770+
const res: [vscUri.URI, NotebookEdit[]][] = [];
771+
772+
this._notebookEdits.forEach((value) => {
773+
res.push([value.uri, value.edits.slice()]);
774+
});
775+
776+
return res.slice();
763777
}
764778

765779
set(uri: vscode.Uri, edits: NotebookEdit[]): void;
@@ -779,17 +793,54 @@ export namespace vscMockExtHostedTypes {
779793
| [TextEdit | SnippetTextEdit, vscode.WorkspaceEditEntryMetadata]
780794
)[]
781795
): void {
782-
let data = this._textEdits.get(uri.toString());
796+
const uriKey = uri.toString();
797+
798+
if (!edits) {
799+
const data = this._textEdits.get(uriKey);
800+
if (data) {
801+
// @ts-ignore
802+
data.edits = undefined;
803+
}
804+
this._notebookEdits.delete(uriKey);
805+
806+
return;
807+
}
808+
809+
if (edits.length === 0) {
810+
let data = this._textEdits.get(uriKey);
811+
if (!data) {
812+
data = { seq: this._seqPool++, uri, edits: [] };
813+
this._textEdits.set(uriKey, data);
814+
}
815+
data.edits = [];
816+
817+
return;
818+
}
819+
820+
const first = edits[0];
821+
const firstEdit = Array.isArray(first) ? first[0] : first;
822+
823+
if (firstEdit instanceof NotebookEdit) {
824+
const normalized: NotebookEdit[] = edits.map((item) => {
825+
if (Array.isArray(item)) {
826+
return item[0] as NotebookEdit;
827+
}
828+
829+
return item as NotebookEdit;
830+
});
831+
832+
this._notebookEdits.set(uriKey, { uri, edits: normalized });
833+
834+
return;
835+
}
836+
837+
let data = this._textEdits.get(uriKey);
783838
if (!data) {
784839
data = { seq: this._seqPool++, uri, edits: [] };
785-
this._textEdits.set(uri.toString(), data);
786-
}
787-
if (!edits) {
788-
// @ts-ignore
789-
data.edits = undefined;
790-
} else {
791-
//data.edits = edits.slice(0);
840+
this._textEdits.set(uriKey, data);
792841
}
842+
843+
data.edits = edits.slice(0) as TextEdit[];
793844
}
794845

795846
get(uri: vscUri.URI): TextEdit[] {
@@ -825,7 +876,7 @@ export namespace vscMockExtHostedTypes {
825876
}
826877

827878
get size(): number {
828-
return this._textEdits.size + this._resourceEdits.length;
879+
return this._textEdits.size + this._notebookEdits.size + this._resourceEdits.length;
829880
}
830881

831882
toJSON(): any {

0 commit comments

Comments
 (0)