Skip to content
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 109 additions & 0 deletions src/services/browser-capture.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { existsSync, writeFileSync } from "node:fs";

// Mock dependencies
vi.mock("node:fs", () => ({
writeFileSync: vi.fn(),
existsSync: vi.fn(),
}));

vi.mock("node:os", () => ({
tmpdir: () => "/tmp",
}));

const mockCDPSession = {
on: vi.fn(),
send: vi.fn(),
};

const mockPage = {
setViewport: vi.fn(),
goto: vi.fn(),
createCDPSession: vi.fn().mockResolvedValue(mockCDPSession),
};

const mockBrowser = {
newPage: vi.fn().mockResolvedValue(mockPage),
close: vi.fn(),
};

const mockLaunch = vi.fn().mockResolvedValue(mockBrowser);

vi.mock("puppeteer-core", () => ({
default: {
launch: mockLaunch,
},
}));

import {
startBrowserCapture,
stopBrowserCapture,
isBrowserCaptureRunning,
hasFrameFile,
FRAME_FILE,
} from "./browser-capture";

describe("browser-capture", () => {
beforeEach(() => {
vi.clearAllMocks();
});

afterEach(async () => {
await stopBrowserCapture();
});

it("should start browser capture", async () => {
await startBrowserCapture({ url: "http://example.com" });
expect(mockLaunch).toHaveBeenCalledWith(expect.objectContaining({
headless: true,
args: expect.arrayContaining(["--no-sandbox", "--disable-gpu"]),
}));
expect(mockBrowser.newPage).toHaveBeenCalled();
expect(mockPage.goto).toHaveBeenCalledWith("http://example.com", expect.objectContaining({ waitUntil: "domcontentloaded" }));
expect(mockPage.createCDPSession).toHaveBeenCalled();
expect(mockCDPSession.send).toHaveBeenCalledWith("Page.startScreencast", expect.objectContaining({
format: "jpeg",
quality: 70,
}));
expect(isBrowserCaptureRunning()).toBe(true);
});

it("should not start if already running", async () => {
await startBrowserCapture({ url: "http://example.com" });
mockLaunch.mockClear();
await startBrowserCapture({ url: "http://example.com" });
expect(mockLaunch).not.toHaveBeenCalled();
});

it("should handle screencast frames", async () => {
await startBrowserCapture({ url: "http://example.com" });

// Simulate frame event
// The implementation uses: cdp.on("Page.screencastFrame", ...)
const onCall = mockCDPSession.on.mock.calls.find((call: any[]) => call[0] === "Page.screencastFrame");
expect(onCall).toBeDefined();

const onCallback = onCall[1];
const frameData = { data: Buffer.from("test-image-data").toString("base64"), sessionId: 123 };
await onCallback(frameData);

expect(writeFileSync).toHaveBeenCalledWith(FRAME_FILE, expect.any(Buffer));
expect(mockCDPSession.send).toHaveBeenCalledWith("Page.screencastFrameAck", { sessionId: 123 });
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The implementation of the Page.screencastFrame event handler in browser-capture.ts uses a try...catch block that silently ignores errors. If an error occurs during writeFileSync, the Page.screencastFrameAck is not sent, which could cause the screencast to stall.

To improve test coverage and highlight this potential issue, please add a test case for this error scenario.

Here is a suggested test to add:

  it("should not send screencast frame ack on write error", async () => {
    await startBrowserCapture({ url: "http://example.com" });

    // Simulate an error during file write
    vi.mocked(writeFileSync).mockImplementation(() => {
      throw new Error("EIO: i/o error");
    });

    // Find and trigger the screencast frame event handler
    const onCall = mockCDPSession.on.mock.calls.find(
      (call: any[]) => call[0] === "Page.screencastFrame",
    );
    expect(onCall).toBeDefined();
    const onCallback = onCall![1];
    const frameData = {
      data: Buffer.from("test-image-data").toString("base64"),
      sessionId: 123,
    };

    // The error should be caught internally, so the call should not throw
    await expect(onCallback(frameData)).resolves.toBeUndefined();

    // Verify that the frame was NOT acknowledged due to the error
    expect(mockCDPSession.send).not.toHaveBeenCalledWith(
      "Page.screencastFrameAck",
      { sessionId: 123 },
    );
  });


it("should stop browser capture", async () => {
await startBrowserCapture({ url: "http://example.com" });
await stopBrowserCapture();
expect(mockBrowser.close).toHaveBeenCalled();
expect(isBrowserCaptureRunning()).toBe(false);
});

it("should check if frame file exists", () => {
vi.mocked(existsSync).mockReturnValue(true);
expect(hasFrameFile()).toBe(true);
expect(existsSync).toHaveBeenCalledWith(FRAME_FILE);

vi.mocked(existsSync).mockReturnValue(false);
expect(hasFrameFile()).toBe(false);
});
Comment on lines +101 to +108

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This test case combines two distinct scenarios: when the frame file exists and when it doesn't. For better clarity, readability, and test isolation, it's a best practice to separate these into two distinct it blocks. This makes it easier to identify which specific scenario fails if the test breaks.

  it("should return true if frame file exists", () => {
    vi.mocked(existsSync).mockReturnValue(true);
    expect(hasFrameFile()).toBe(true);
    expect(existsSync).toHaveBeenCalledWith(FRAME_FILE);
  });

  it("should return false if frame file does not exist", () => {
    vi.mocked(existsSync).mockReturnValue(false);
    expect(hasFrameFile()).toBe(false);
    expect(existsSync).toHaveBeenCalledWith(FRAME_FILE);
  });

});
Loading