Skip to content

Commit 3b9d72e

Browse files
committed
Fix test helper bugs: always pass historyLineLimit, use !== undefined checks, fix eviction wait condition
- Bug 1: makeManager always uses makeTerminalManagerWithOptions with the historyLineLimit parameter, instead of a fast path via TerminalManagerLive that silently defaulted to 5000. - Bug 2: Use !== undefined checks instead of truthy checks for numeric options (subprocessPollIntervalMs, processKillGraceMs, maxRetainedInactiveSessions) so explicit zero values are not dropped. - Bug 3: Eviction test waits for 2 'exited' events in the event stream instead of ptyAdapter.processes.length === 2 which was always true.
1 parent 5a6e698 commit 3b9d72e

File tree

1 file changed

+29
-66
lines changed

1 file changed

+29
-66
lines changed

apps/server/src/terminal/Layers/Manager.test.ts

Lines changed: 29 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,15 @@ import {
1212
import { Effect, Encoding, Exit, Layer, ManagedRuntime, Ref, Scope, Stream } from "effect";
1313
import { afterEach, describe, expect, it } from "vitest";
1414

15-
import { ServerConfig } from "../../config";
1615
import { TerminalManager } from "../Services/Manager";
1716
import {
18-
PtyAdapter,
1917
type PtyAdapterShape,
2018
type PtyExitEvent,
2119
type PtyProcess,
2220
type PtySpawnInput,
2321
PtySpawnError,
2422
} from "../Services/PTY";
25-
import { makeTerminalManagerWithOptions, TerminalManagerLive } from "./Manager";
23+
import { makeTerminalManagerWithOptions } from "./Manager";
2624

2725
class FakePtyProcess implements PtyProcess {
2826
readonly writes: string[] = [];
@@ -194,13 +192,29 @@ async function makeManager(
194192
const logsDir = path.join(baseDir, "userdata", "logs", "terminals");
195193
const ptyAdapter = options.ptyAdapter ?? new FakePtyAdapter();
196194

197-
const terminalLayer = TerminalManagerLive.pipe(
198-
Layer.provideMerge(Layer.succeed(PtyAdapter, ptyAdapter)),
199-
Layer.provideMerge(ServerConfig.layerTest(process.cwd(), baseDir)),
200-
Layer.provideMerge(NodeServices.layer),
201-
);
202-
203-
const runtime = ManagedRuntime.make(terminalLayer);
195+
const layer = Layer.effect(
196+
TerminalManager,
197+
makeTerminalManagerWithOptions({
198+
logsDir,
199+
historyLineLimit,
200+
ptyAdapter,
201+
...(options.shellResolver !== undefined ? { shellResolver: options.shellResolver } : {}),
202+
...(options.subprocessChecker !== undefined
203+
? { subprocessChecker: options.subprocessChecker }
204+
: {}),
205+
...(options.subprocessPollIntervalMs !== undefined
206+
? { subprocessPollIntervalMs: options.subprocessPollIntervalMs }
207+
: {}),
208+
...(options.processKillGraceMs !== undefined
209+
? { processKillGraceMs: options.processKillGraceMs }
210+
: {}),
211+
...(options.maxRetainedInactiveSessions !== undefined
212+
? { maxRetainedInactiveSessions: options.maxRetainedInactiveSessions }
213+
: {}),
214+
}),
215+
).pipe(Layer.provideMerge(NodeServices.layer));
216+
217+
const runtime = ManagedRuntime.make(layer);
204218
const manager = await runtime.runPromise(Effect.service(TerminalManager));
205219
const eventsRef = await Effect.runPromise(Ref.make<TerminalEvent[]>([]));
206220
const eventScope = await Effect.runPromise(Scope.make("sequential"));
@@ -210,60 +224,6 @@ async function makeManager(
210224
).pipe(Effect.forkIn(eventScope)),
211225
);
212226

213-
if (
214-
historyLineLimit !== 5 ||
215-
options.shellResolver ||
216-
options.subprocessChecker ||
217-
options.subprocessPollIntervalMs ||
218-
options.processKillGraceMs ||
219-
options.maxRetainedInactiveSessions
220-
) {
221-
await runtime.dispose();
222-
223-
const customLayer = Layer.effect(
224-
TerminalManager,
225-
makeTerminalManagerWithOptions({
226-
logsDir,
227-
historyLineLimit,
228-
ptyAdapter,
229-
...(options.shellResolver ? { shellResolver: options.shellResolver } : {}),
230-
...(options.subprocessChecker ? { subprocessChecker: options.subprocessChecker } : {}),
231-
...(options.subprocessPollIntervalMs
232-
? { subprocessPollIntervalMs: options.subprocessPollIntervalMs }
233-
: {}),
234-
...(options.processKillGraceMs ? { processKillGraceMs: options.processKillGraceMs } : {}),
235-
...(options.maxRetainedInactiveSessions
236-
? { maxRetainedInactiveSessions: options.maxRetainedInactiveSessions }
237-
: {}),
238-
}),
239-
).pipe(Layer.provideMerge(NodeServices.layer));
240-
241-
const customRuntime = ManagedRuntime.make(customLayer);
242-
const customManager = await customRuntime.runPromise(Effect.service(TerminalManager));
243-
const customEventsRef = await Effect.runPromise(Ref.make<TerminalEvent[]>([]));
244-
const customEventScope = await Effect.runPromise(Scope.make("sequential"));
245-
await customRuntime.runPromise(
246-
Stream.runForEach(customManager.streamEvents, (event) =>
247-
Ref.update(customEventsRef, (events) => [...events, event]),
248-
).pipe(Effect.forkIn(customEventScope)),
249-
);
250-
251-
return {
252-
baseDir,
253-
logsDir,
254-
ptyAdapter,
255-
runtime: customRuntime,
256-
manager: customManager,
257-
eventsRef: customEventsRef,
258-
run: <A, E>(effect: Effect.Effect<A, E>) => customRuntime.runPromise(effect),
259-
getEvents: () => Effect.runPromise(Ref.get(customEventsRef)),
260-
dispose: async () => {
261-
await Effect.runPromise(Scope.close(customEventScope, Exit.void));
262-
await customRuntime.dispose();
263-
},
264-
};
265-
}
266-
267227
return {
268228
baseDir,
269229
logsDir,
@@ -636,7 +596,7 @@ describe("TerminalManager", () => {
636596
});
637597

638598
it("evicts oldest inactive terminal sessions when retention limit is exceeded", async () => {
639-
const { manager, ptyAdapter, run, logsDir } = await createManager(5, {
599+
const { manager, ptyAdapter, run, logsDir, getEvents } = await createManager(5, {
640600
maxRetainedInactiveSessions: 1,
641601
});
642602

@@ -656,7 +616,10 @@ describe("TerminalManager", () => {
656616
await new Promise((resolve) => setTimeout(resolve, 5));
657617
second.emitExit({ exitCode: 0, signal: 0 });
658618

659-
await waitFor(() => ptyAdapter.processes.length === 2);
619+
await waitFor(async () => {
620+
const events = await getEvents();
621+
return events.filter((e) => e.type === "exited").length === 2;
622+
});
660623

661624
const reopenedSecond = await run(manager.open(openInput({ threadId: "thread-2" })));
662625
const reopenedFirst = await run(manager.open(openInput({ threadId: "thread-1" })));

0 commit comments

Comments
 (0)