Skip to content

Commit 9366642

Browse files
committed
fix(terminal): update unit tests for read()-in-TerminalRegistry architecture
1 parent ae04745 commit 9366642

4 files changed

Lines changed: 31 additions & 18 deletions

File tree

src/__mocks__/vscode.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ export const window = {
8181
sendText: () => {},
8282
}),
8383
onDidCloseTerminal: () => mockDisposable,
84+
onDidChangeTerminalShellIntegration: () => mockDisposable,
8485
createTextEditorDecorationType: () => ({ dispose: () => {} }),
8586
}
8687

src/integrations/terminal/Terminal.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -158,15 +158,16 @@ export class Terminal extends BaseTerminal {
158158
}
159159

160160
return new Promise<void>((resolve, reject) => {
161+
const ref = { disposable: null as vscode.Disposable | null }
161162
const timer = setTimeout(() => {
162-
disposable.dispose()
163+
ref.disposable?.dispose()
163164
reject(new Error(`Shell integration did not activate within ${timeoutMs / 1000}s`))
164165
}, timeoutMs)
165166

166-
const disposable = vscode.window.onDidChangeTerminalShellIntegration((e) => {
167+
ref.disposable = vscode.window.onDidChangeTerminalShellIntegration((e) => {
167168
if (e.terminal === this.terminal) {
168169
clearTimeout(timer)
169-
disposable.dispose()
170+
ref.disposable?.dispose()
170171
resolve()
171172
}
172173
})

src/integrations/terminal/__tests__/TerminalProcess.spec.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,9 @@ describe("TerminalProcess", () => {
118118
// onDidEndTerminalShellExecution is a separate global VSCode event, not
119119
// something coupled to the stream iterator being pulled again -- emit it
120120
// independently of stream consumption, matching real-world timing.
121-
terminalProcess.emit("shell_execution_complete", { exitCode: 0 })
121+
// Use setTimeout(0) so it fires after microtask-based stream processing
122+
// (async generator iterations) has consumed all chunks including the D marker.
123+
setTimeout(() => terminalProcess.emit("shell_execution_complete", { exitCode: 0 }), 0)
122124

123125
await runPromise
124126

@@ -363,10 +365,12 @@ describe("TerminalProcess", () => {
363365
terminalProcess.once("no_shell_integration", noShellIntegrationSpy)
364366

365367
const runPromise = terminalProcess.run("test command")
368+
// stream_available is now emitted by TerminalRegistry (onDidStartTerminalShellExecution).
369+
// Simulate that here so run() can proceed to consume the stream.
370+
terminalProcess.emit("stream_available", mockStream)
366371
await runPromise
367372
await eventPromises
368373

369-
expect(mockExecution.read).toHaveBeenCalledTimes(1)
370374
expect(completedOutput).toBe("")
371375
expect(noShellIntegrationSpy).not.toHaveBeenCalled()
372376
})
@@ -396,10 +400,12 @@ describe("TerminalProcess", () => {
396400
terminalProcess.once("no_shell_integration", noShellIntegrationSpy)
397401

398402
const runPromise = terminalProcess.run("test command")
403+
// stream_available is now emitted by TerminalRegistry (onDidStartTerminalShellExecution).
404+
// Simulate that here so run() can proceed to consume the stream.
405+
terminalProcess.emit("stream_available", mockStream)
399406
await runPromise
400407
await eventPromises
401408

402-
expect(mockExecution.read).toHaveBeenCalledTimes(1)
403409
expect(completedOutput).toBe("some output without marker\n")
404410
expect(noShellIntegrationSpy).not.toHaveBeenCalled()
405411
})
@@ -437,7 +443,9 @@ describe("TerminalProcess", () => {
437443
// onDidEndTerminalShellExecution is a separate global VSCode event, not
438444
// something coupled to the stream iterator being pulled again -- emit it
439445
// independently of stream consumption, matching real-world timing.
440-
terminalProcess.emit("shell_execution_complete", { exitCode: 0 })
446+
// Use setTimeout(0) so it fires after microtask-based stream processing
447+
// has consumed all chunks including the D marker.
448+
setTimeout(() => terminalProcess.emit("shell_execution_complete", { exitCode: 0 }), 0)
441449

442450
await runPromise
443451

src/integrations/terminal/__tests__/TerminalProcessExec.bash.spec.ts

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -228,18 +228,21 @@ async function testTerminalCommand(
228228
})
229229
}
230230

231-
// Wait for some output to be processed
232-
await new Promise<void>((resolve) => {
233-
terminalProcess.once("line", () => resolve())
234-
})
235-
236-
// Then trigger the end event, referencing the SAME execution object as above.
231+
// Trigger the end event after microtask-based stream consumption completes.
232+
// The stream yields chunks as microtasks (async generator); setTimeout(0)
233+
// fires after all pending microtasks, so the D marker will be consumed and
234+
// the loop will have broken on sawEndMarker before this fires. Firing before
235+
// that (e.g. after the first 'line' event) would cause DONE_SENTINEL to win
236+
// the Promise.race and skip unconsumed chunks.
237237
if (eventHandlers.endTerminalShellExecution) {
238-
eventHandlers.endTerminalShellExecution({
239-
terminal: mockTerminal,
240-
execution: mockExecution,
241-
exitCode: exitCode,
242-
})
238+
const _exitCode = exitCode
239+
setTimeout(() => {
240+
eventHandlers.endTerminalShellExecution({
241+
terminal: mockTerminal,
242+
execution: mockExecution,
243+
exitCode: _exitCode,
244+
})
245+
}, 0)
243246
}
244247

245248
// Store exit details for return

0 commit comments

Comments
 (0)