fix(#338): Add tests to verify runner jobs interval cleanup
- Add test verifying clearInterval is called in finally block - Add test verifying interval is cleared even when stream throws error - Prevents memory leaks from leaked intervals The clearInterval was already present in the codebase at line 409 of runner-jobs.service.ts. These tests provide explicit verification of the cleanup behavior. Refs #338 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -608,14 +608,11 @@ describe("RunnerJobsService", () => {
|
|||||||
const jobId = "job-123";
|
const jobId = "job-123";
|
||||||
const workspaceId = "workspace-123";
|
const workspaceId = "workspace-123";
|
||||||
|
|
||||||
let closeHandler: (() => void) | null = null;
|
|
||||||
|
|
||||||
const mockRes = {
|
const mockRes = {
|
||||||
write: vi.fn(),
|
write: vi.fn(),
|
||||||
end: vi.fn(),
|
end: vi.fn(),
|
||||||
on: vi.fn((event: string, handler: () => void) => {
|
on: vi.fn((event: string, handler: () => void) => {
|
||||||
if (event === "close") {
|
if (event === "close") {
|
||||||
closeHandler = handler;
|
|
||||||
// Immediately trigger close to break the loop
|
// Immediately trigger close to break the loop
|
||||||
setTimeout(() => handler(), 10);
|
setTimeout(() => handler(), 10);
|
||||||
}
|
}
|
||||||
@@ -638,6 +635,89 @@ describe("RunnerJobsService", () => {
|
|||||||
expect(mockRes.end).toHaveBeenCalled();
|
expect(mockRes.end).toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("should call clearInterval in finally block to prevent memory leaks", async () => {
|
||||||
|
const jobId = "job-123";
|
||||||
|
const workspaceId = "workspace-123";
|
||||||
|
|
||||||
|
// Spy on global setInterval and clearInterval
|
||||||
|
const mockIntervalId = 12345;
|
||||||
|
const setIntervalSpy = vi
|
||||||
|
.spyOn(global, "setInterval")
|
||||||
|
.mockReturnValue(mockIntervalId as never);
|
||||||
|
const clearIntervalSpy = vi.spyOn(global, "clearInterval").mockImplementation(() => {});
|
||||||
|
|
||||||
|
const mockRes = {
|
||||||
|
write: vi.fn(),
|
||||||
|
end: vi.fn(),
|
||||||
|
on: vi.fn(),
|
||||||
|
writableEnded: false,
|
||||||
|
};
|
||||||
|
|
||||||
|
// Mock job to complete immediately
|
||||||
|
mockPrismaService.runnerJob.findUnique
|
||||||
|
.mockResolvedValueOnce({
|
||||||
|
id: jobId,
|
||||||
|
status: RunnerJobStatus.RUNNING,
|
||||||
|
})
|
||||||
|
.mockResolvedValueOnce({
|
||||||
|
id: jobId,
|
||||||
|
status: RunnerJobStatus.COMPLETED,
|
||||||
|
});
|
||||||
|
|
||||||
|
mockPrismaService.jobEvent.findMany.mockResolvedValue([]);
|
||||||
|
|
||||||
|
await service.streamEvents(jobId, workspaceId, mockRes as never);
|
||||||
|
|
||||||
|
// Verify setInterval was called for keep-alive ping
|
||||||
|
expect(setIntervalSpy).toHaveBeenCalled();
|
||||||
|
|
||||||
|
// Verify clearInterval was called with the interval ID to prevent memory leak
|
||||||
|
expect(clearIntervalSpy).toHaveBeenCalledWith(mockIntervalId);
|
||||||
|
|
||||||
|
// Cleanup spies
|
||||||
|
setIntervalSpy.mockRestore();
|
||||||
|
clearIntervalSpy.mockRestore();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should clear interval even when stream throws an error", async () => {
|
||||||
|
const jobId = "job-123";
|
||||||
|
const workspaceId = "workspace-123";
|
||||||
|
|
||||||
|
// Spy on global setInterval and clearInterval
|
||||||
|
const mockIntervalId = 54321;
|
||||||
|
const setIntervalSpy = vi
|
||||||
|
.spyOn(global, "setInterval")
|
||||||
|
.mockReturnValue(mockIntervalId as never);
|
||||||
|
const clearIntervalSpy = vi.spyOn(global, "clearInterval").mockImplementation(() => {});
|
||||||
|
|
||||||
|
const mockRes = {
|
||||||
|
write: vi.fn(),
|
||||||
|
end: vi.fn(),
|
||||||
|
on: vi.fn(),
|
||||||
|
writableEnded: false,
|
||||||
|
};
|
||||||
|
|
||||||
|
mockPrismaService.runnerJob.findUnique.mockResolvedValueOnce({
|
||||||
|
id: jobId,
|
||||||
|
status: RunnerJobStatus.RUNNING,
|
||||||
|
});
|
||||||
|
|
||||||
|
// Simulate a fatal error during event polling
|
||||||
|
mockPrismaService.jobEvent.findMany.mockRejectedValue(new Error("Fatal database failure"));
|
||||||
|
|
||||||
|
// The method should throw but still clean up
|
||||||
|
await expect(service.streamEvents(jobId, workspaceId, mockRes as never)).rejects.toThrow(
|
||||||
|
"Fatal database failure"
|
||||||
|
);
|
||||||
|
|
||||||
|
// Verify clearInterval was called even on error (via finally block)
|
||||||
|
expect(clearIntervalSpy).toHaveBeenCalledWith(mockIntervalId);
|
||||||
|
|
||||||
|
// Cleanup spies
|
||||||
|
setIntervalSpy.mockRestore();
|
||||||
|
clearIntervalSpy.mockRestore();
|
||||||
|
});
|
||||||
|
|
||||||
// ERROR RECOVERY TESTS - Issue #187
|
// ERROR RECOVERY TESTS - Issue #187
|
||||||
|
|
||||||
it("should support resuming stream from lastEventId", async () => {
|
it("should support resuming stream from lastEventId", async () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user