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
149 changes: 149 additions & 0 deletions src/services/browser-capture.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import {
startBrowserCapture,
stopBrowserCapture,
isBrowserCaptureRunning,
hasFrameFile,
FRAME_FILE
} from './browser-capture';
import puppeteer from 'puppeteer-core';
import * as fs from 'node:fs';

vi.mock('puppeteer-core');
vi.mock('node:fs', () => {
return {
writeFileSync: vi.fn(),
existsSync: vi.fn(),
};
});

describe('Browser Capture Service', () => {
let mockPage: any;
let mockCdp: any;
let mockBrowser: any;
Comment on lines +21 to +23

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Using any for mock objects bypasses TypeScript's type checking. It is recommended to use specific types from puppeteer-core (e.g., Page, CDPSession, Browser) combined with Vitest's mocking types to ensure the mocks align with the actual API and provide better type safety.


beforeEach(() => {
mockCdp = {
on: vi.fn(),
send: vi.fn(),
};

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

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

vi.mocked(puppeteer.launch).mockResolvedValue(mockBrowser as any);
});

afterEach(async () => {
vi.clearAllMocks();
await stopBrowserCapture();
Comment on lines +47 to +48

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

It is recommended to call stopBrowserCapture() before vi.clearAllMocks(). This ensures that the service's internal state is fully cleaned up and any resulting mock interactions (like closing the browser) are completed before the mock history is cleared for the next test.

Suggested change
vi.clearAllMocks();
await stopBrowserCapture();
await stopBrowserCapture();
vi.clearAllMocks();

});
Comment on lines +46 to +49

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Test Isolation and Cleanup:
The afterEach block calls stopBrowserCapture() without checking if a capture was actually started. If a test fails before starting the capture, this could lead to inconsistent cleanup or errors. Consider adding a check to ensure that cleanup only occurs if a capture was started, or handle errors gracefully in the cleanup phase.

Recommended solution:

afterEach(async () => {
  vi.clearAllMocks();
  if (isBrowserCaptureRunning()) {
    await stopBrowserCapture();
  }
});


it('should start browser capture with correct parameters', async () => {
const config = {
url: 'http://localhost:3000/stream',
width: 1920,
height: 1080,
quality: 80,
overlayLayout: '{"test":true}',
theme: 'haxor',
avatarIndex: 2,
destinationId: 'dest-1'
};

await startBrowserCapture(config);

expect(puppeteer.launch).toHaveBeenCalledWith(expect.objectContaining({
headless: true,
args: expect.arrayContaining([
'--window-size=1920,1080',
'--no-sandbox',
'--use-gl=swiftshader',
]),
}));

expect(mockPage.setViewport).toHaveBeenCalledWith({
width: 1920,
height: 1080,
deviceScaleFactor: 1
});

expect(mockPage.evaluateOnNewDocument).toHaveBeenCalledWith(
expect.any(Function),
'{"test":true}',
'haxor',
2,
'dest-1'
);

// Should append popout
expect(mockPage.goto).toHaveBeenCalledWith('http://localhost:3000/stream?popout=', expect.objectContaining({
waitUntil: 'networkidle0'
}));

expect(mockCdp.send).toHaveBeenCalledWith('Page.startScreencast', {
format: 'jpeg',
quality: 80,
maxWidth: 1920,
maxHeight: 1080,
everyNthFrame: 2,
});

expect(isBrowserCaptureRunning()).toBe(true);
});

it('should not start a second instance if already running', async () => {
await startBrowserCapture({ url: 'http://localhost' });
expect(puppeteer.launch).toHaveBeenCalledTimes(1);

await startBrowserCapture({ url: 'http://localhost' });
expect(puppeteer.launch).toHaveBeenCalledTimes(1); // Still 1
});

it('should handle URL popout logic correctly', async () => {
await startBrowserCapture({ url: 'http://localhost:3000/stream#view?' });
expect(mockPage.goto).toHaveBeenCalledWith('http://localhost:3000/stream#view?&popout', expect.any(Object));
});

it('should handle screencast frames correctly', async () => {
await startBrowserCapture({ url: 'http://localhost' });

// Simulate frame
const onCall = mockCdp.on.mock.calls.find((call: any) => call[0] === 'Page.screencastFrame');
expect(onCall).toBeDefined();

const handler = onCall[1];

await handler({
data: Buffer.from('test frame').toString('base64'),
sessionId: 123
});

expect(vi.mocked(fs.writeFileSync)).toHaveBeenCalledWith(FRAME_FILE, expect.any(Buffer));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Instead of using expect.any(Buffer), verify that the buffer written to the file contains the expected decoded data. This ensures the base64 decoding logic in the service is working correctly.

Suggested change
expect(vi.mocked(fs.writeFileSync)).toHaveBeenCalledWith(FRAME_FILE, expect.any(Buffer));
expect(vi.mocked(fs.writeFileSync)).toHaveBeenCalledWith(FRAME_FILE, Buffer.from('test frame'));

expect(mockCdp.send).toHaveBeenCalledWith('Page.screencastFrameAck', { sessionId: 123 });
});
Comment on lines +117 to +133

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Frame File Content Verification:
The test for handling screencast frames verifies that fs.writeFileSync is called, but does not check the actual content written to the file. This could miss issues where the frame data is incorrectly encoded or corrupted.

Recommended solution:
Add an assertion to verify the content of the buffer passed to writeFileSync, for example:

expect(vi.mocked(fs.writeFileSync)).toHaveBeenCalledWith(FRAME_FILE, Buffer.from('test frame'));


it('should stop capture correctly', async () => {
await startBrowserCapture({ url: 'http://localhost' });
expect(isBrowserCaptureRunning()).toBe(true);

await stopBrowserCapture();
expect(mockBrowser.close).toHaveBeenCalled();
expect(isBrowserCaptureRunning()).toBe(false);
});

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