Skip to content

Commit 85cf712

Browse files
shtse8claude
andcommitted
test: comprehensive test coverage for refactored modules
Add extensive unit tests for newly extracted modules, achieving 98.68% statement coverage and 93.02% branch coverage. New Test Files: - test/pdf/extractor.test.ts (13 tests) - Tests for metadata and text extraction - test/pdf/loader.test.ts (9 tests) - Tests for PDF document loading - test/pdf/parser.test.ts (26 tests) - Tests for page range parsing Test Improvements: - Added test for non-Error exception handling in readPdf handler - Total tests: 31 → 80 tests (+49 new tests, +158% increase) - Test files: 2 → 5 files Coverage Improvements: - Statements: 90.26% → 98.68% (+8.42%) - Branches: 78.64% → 93.02% (+14.38%) - Functions: 100% (maintained) - Lines: 90.26% → 98.68% (+8.42%) Coverage by Module: - handlers/readPdf.ts: 99.01% statements, 80.95% branches ✅ - pdf/extractor.ts: 100% statements, 100% branches ✅ - pdf/loader.ts: 100% statements, 92.59% branches ✅ - pdf/parser.ts: 95.29% statements, 93.75% branches ✅ - schemas/readPdf.ts: 100% statements, 100% branches ✅ - utils/pathUtils.ts: 100% statements, 100% branches ✅ Test Coverage Includes: - Metadata extraction with getAll() method - Metadata extraction without getAll() (property enumeration) - Metadata extraction error handling - File loading from paths and URLs - File not found (ENOENT) error handling - Generic file read errors - PDF.js loading errors - Page range parsing (single pages, ranges, open-ended) - Page validation and deduplication - Invalid page specifications - Non-Error exception handling - Edge cases for all exported functions Configuration: - Updated vitest.config.ts to exclude type-only files (src/types/**) - Increased coverage thresholds: 98% statements, 93% branches, 100% functions All 80 tests passing. Code quality and maintainability significantly improved. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 1519fe0 commit 85cf712

File tree

5 files changed

+543
-4
lines changed

5 files changed

+543
-4
lines changed

test/handlers/readPdf.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -748,4 +748,29 @@ describe('handleReadPdfFunc Integration Tests', () => {
748748
);
749749
await expect(handler(args)).rejects.toHaveProperty('code', ErrorCode.InvalidParams);
750750
});
751+
752+
it('should handle non-Error exceptions during processing', async () => {
753+
// Mock to throw non-Error at processSingleSource level
754+
// We need to throw something that's not Error or McpError
755+
mockGetDocument.mockReset();
756+
mockGetDocument.mockImplementation(() => {
757+
throw { custom: 'object error' }; // Non-Error, non-McpError
758+
});
759+
760+
const args = { sources: [{ path: 'test.pdf' }] };
761+
const result = await handler(args);
762+
763+
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
764+
if (result.content?.[0]) {
765+
const parsedResult = JSON.parse(result.content[0].text) as ExpectedResultType;
766+
expect(parsedResult.results[0]).toBeDefined();
767+
if (parsedResult.results[0]) {
768+
expect(parsedResult.results[0].success).toBe(false);
769+
expect(parsedResult.results[0].error).toContain('Unknown error');
770+
expect(parsedResult.results[0].error).toContain('custom');
771+
}
772+
} else {
773+
expect.fail('result.content[0] was undefined');
774+
}
775+
});
751776
}); // End top-level describe

test/pdf/extractor.test.ts

Lines changed: 215 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,215 @@
1+
import type * as pdfjsLib from 'pdfjs-dist/legacy/build/pdf.mjs';
2+
import { beforeEach, describe, expect, it, vi } from 'vitest';
3+
import {
4+
buildWarnings,
5+
extractMetadataAndPageCount,
6+
extractPageTexts,
7+
} from '../../src/pdf/extractor.js';
8+
9+
describe('extractor', () => {
10+
describe('extractMetadataAndPageCount', () => {
11+
it('should extract metadata using getAll method when available', async () => {
12+
const mockMetadata = {
13+
info: { PDFFormatVersion: '1.7', IsLinearized: false },
14+
metadata: {
15+
getAll: vi.fn().mockReturnValue({ Author: 'Test Author', Title: 'Test Title' }),
16+
},
17+
};
18+
19+
const mockDocument = {
20+
numPages: 5,
21+
getMetadata: vi.fn().mockResolvedValue(mockMetadata),
22+
} as unknown as pdfjsLib.PDFDocumentProxy;
23+
24+
const result = await extractMetadataAndPageCount(mockDocument, true, true);
25+
26+
expect(result.num_pages).toBe(5);
27+
expect(result.info).toEqual({ PDFFormatVersion: '1.7', IsLinearized: false });
28+
expect(result.metadata).toEqual({ Author: 'Test Author', Title: 'Test Title' });
29+
expect(mockMetadata.metadata.getAll).toHaveBeenCalled();
30+
});
31+
32+
it('should extract metadata by enumerating properties when getAll is not available', async () => {
33+
const mockMetadataObj = {
34+
Author: 'Direct Author',
35+
Title: 'Direct Title',
36+
CreationDate: '2025-01-01',
37+
};
38+
39+
const mockMetadata = {
40+
info: { PDFFormatVersion: '1.6' },
41+
metadata: mockMetadataObj,
42+
};
43+
44+
const mockDocument = {
45+
numPages: 3,
46+
getMetadata: vi.fn().mockResolvedValue(mockMetadata),
47+
} as unknown as pdfjsLib.PDFDocumentProxy;
48+
49+
const result = await extractMetadataAndPageCount(mockDocument, true, true);
50+
51+
expect(result.metadata).toEqual({
52+
Author: 'Direct Author',
53+
Title: 'Direct Title',
54+
CreationDate: '2025-01-01',
55+
});
56+
});
57+
58+
it('should handle metadata extraction errors gracefully', async () => {
59+
const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
60+
61+
const mockDocument = {
62+
numPages: 2,
63+
getMetadata: vi.fn().mockRejectedValue(new Error('Metadata error')),
64+
} as unknown as pdfjsLib.PDFDocumentProxy;
65+
66+
const result = await extractMetadataAndPageCount(mockDocument, true, true);
67+
68+
expect(result.num_pages).toBe(2);
69+
expect(result.metadata).toBeUndefined();
70+
expect(result.info).toBeUndefined();
71+
expect(consoleWarnSpy).toHaveBeenCalledWith(
72+
expect.stringContaining('Error extracting metadata: Metadata error')
73+
);
74+
75+
consoleWarnSpy.mockRestore();
76+
});
77+
78+
it('should handle non-Error metadata exceptions', async () => {
79+
const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
80+
81+
const mockDocument = {
82+
numPages: 1,
83+
getMetadata: vi.fn().mockRejectedValue('String error'),
84+
} as unknown as pdfjsLib.PDFDocumentProxy;
85+
86+
const result = await extractMetadataAndPageCount(mockDocument, true, true);
87+
88+
expect(result.num_pages).toBe(1);
89+
expect(consoleWarnSpy).toHaveBeenCalledWith(
90+
expect.stringContaining('Error extracting metadata: String error')
91+
);
92+
93+
consoleWarnSpy.mockRestore();
94+
});
95+
96+
it('should not extract metadata when includeMetadata is false', async () => {
97+
const mockDocument = {
98+
numPages: 5,
99+
getMetadata: vi.fn(),
100+
} as unknown as pdfjsLib.PDFDocumentProxy;
101+
102+
const result = await extractMetadataAndPageCount(mockDocument, false, true);
103+
104+
expect(result.num_pages).toBe(5);
105+
expect(result.metadata).toBeUndefined();
106+
expect(result.info).toBeUndefined();
107+
expect(mockDocument.getMetadata).not.toHaveBeenCalled();
108+
});
109+
110+
it('should not extract page count when includePageCount is false', async () => {
111+
const mockDocument = {
112+
numPages: 10,
113+
getMetadata: vi.fn(),
114+
} as unknown as pdfjsLib.PDFDocumentProxy;
115+
116+
const result = await extractMetadataAndPageCount(mockDocument, false, false);
117+
118+
expect(result.num_pages).toBeUndefined();
119+
});
120+
});
121+
122+
describe('extractPageTexts', () => {
123+
let consoleWarnSpy: ReturnType<typeof vi.spyOn>;
124+
125+
beforeEach(() => {
126+
consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
127+
});
128+
129+
it('should extract text from specified pages', async () => {
130+
const mockPage1 = {
131+
getTextContent: vi.fn().mockResolvedValue({
132+
items: [{ str: 'Page 1 ' }, { str: 'text' }],
133+
}),
134+
};
135+
136+
const mockPage2 = {
137+
getTextContent: vi.fn().mockResolvedValue({
138+
items: [{ str: 'Page 2 ' }, { str: 'content' }],
139+
}),
140+
};
141+
142+
const mockDocument = {
143+
getPage: vi
144+
.fn()
145+
.mockImplementation((pageNum: number) =>
146+
Promise.resolve(pageNum === 1 ? mockPage1 : mockPage2)
147+
),
148+
} as unknown as pdfjsLib.PDFDocumentProxy;
149+
150+
const result = await extractPageTexts(mockDocument, [1, 2], 'test.pdf');
151+
152+
expect(result).toEqual([
153+
{ page: 1, text: 'Page 1 text' },
154+
{ page: 2, text: 'Page 2 content' },
155+
]);
156+
});
157+
158+
it('should handle page extraction errors gracefully', async () => {
159+
const mockDocument = {
160+
getPage: vi.fn().mockRejectedValue(new Error('Failed to get page')),
161+
} as unknown as pdfjsLib.PDFDocumentProxy;
162+
163+
const result = await extractPageTexts(mockDocument, [1], 'test.pdf');
164+
165+
expect(result).toEqual([{ page: 1, text: 'Error processing page: Failed to get page' }]);
166+
expect(consoleWarnSpy).toHaveBeenCalledWith(
167+
expect.stringContaining('Error getting text content for page 1 in test.pdf')
168+
);
169+
});
170+
171+
it('should handle non-Error page exceptions', async () => {
172+
const mockDocument = {
173+
getPage: vi.fn().mockRejectedValue('String error'),
174+
} as unknown as pdfjsLib.PDFDocumentProxy;
175+
176+
const result = await extractPageTexts(mockDocument, [1], 'test.pdf');
177+
178+
expect(result).toEqual([{ page: 1, text: 'Error processing page: String error' }]);
179+
expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining('String error'));
180+
});
181+
182+
it('should sort pages by page number', async () => {
183+
const mockPage = {
184+
getTextContent: vi.fn().mockResolvedValue({
185+
items: [{ str: 'text' }],
186+
}),
187+
};
188+
189+
const mockDocument = {
190+
getPage: vi.fn().mockResolvedValue(mockPage),
191+
} as unknown as pdfjsLib.PDFDocumentProxy;
192+
193+
const result = await extractPageTexts(mockDocument, [3, 1, 2], 'test.pdf');
194+
195+
expect(result.map((r) => r.page)).toEqual([1, 2, 3]);
196+
});
197+
});
198+
199+
describe('buildWarnings', () => {
200+
it('should return empty array when no invalid pages', () => {
201+
const warnings = buildWarnings([], 10);
202+
expect(warnings).toEqual([]);
203+
});
204+
205+
it('should build warning for invalid pages', () => {
206+
const warnings = buildWarnings([11, 12, 15], 10);
207+
expect(warnings).toEqual(['Requested page numbers 11, 12, 15 exceed total pages (10).']);
208+
});
209+
210+
it('should build warning for single invalid page', () => {
211+
const warnings = buildWarnings([20], 10);
212+
expect(warnings).toEqual(['Requested page numbers 20 exceed total pages (10).']);
213+
});
214+
});
215+
});

test/pdf/loader.test.ts

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
import fs from 'node:fs/promises';
2+
import { ErrorCode, McpError } from '@modelcontextprotocol/sdk/types.js';
3+
import * as pdfjsLib from 'pdfjs-dist/legacy/build/pdf.mjs';
4+
import { describe, expect, it, vi } from 'vitest';
5+
import { loadPdfDocument } from '../../src/pdf/loader.js';
6+
import * as pathUtils from '../../src/utils/pathUtils.js';
7+
8+
vi.mock('node:fs/promises');
9+
vi.mock('pdfjs-dist/legacy/build/pdf.mjs');
10+
vi.mock('../../src/utils/pathUtils.js', async () => {
11+
const actual = await vi.importActual('../../src/utils/pathUtils.js');
12+
return {
13+
...actual,
14+
resolvePath: vi.fn(),
15+
};
16+
});
17+
18+
describe('loader', () => {
19+
describe('loadPdfDocument', () => {
20+
it('should load PDF from local file path', async () => {
21+
const mockBuffer = Buffer.from('fake pdf content');
22+
const mockDocument = { numPages: 5 };
23+
24+
vi.mocked(pathUtils.resolvePath).mockReturnValue('/safe/path/test.pdf');
25+
vi.mocked(fs.readFile).mockResolvedValue(mockBuffer);
26+
vi.mocked(pdfjsLib.getDocument).mockReturnValue({
27+
promise: Promise.resolve(mockDocument as unknown as pdfjsLib.PDFDocumentProxy),
28+
} as pdfjsLib.PDFDocumentLoadingTask);
29+
30+
const result = await loadPdfDocument({ path: 'test.pdf' }, 'test.pdf');
31+
32+
expect(result).toBe(mockDocument);
33+
expect(pathUtils.resolvePath).toHaveBeenCalledWith('test.pdf');
34+
expect(fs.readFile).toHaveBeenCalledWith('/safe/path/test.pdf');
35+
});
36+
37+
it('should load PDF from URL', async () => {
38+
const mockDocument = { numPages: 3 };
39+
40+
vi.mocked(pdfjsLib.getDocument).mockReturnValue({
41+
promise: Promise.resolve(mockDocument as unknown as pdfjsLib.PDFDocumentProxy),
42+
} as pdfjsLib.PDFDocumentLoadingTask);
43+
44+
const result = await loadPdfDocument(
45+
{ url: 'https://example.com/test.pdf' },
46+
'https://example.com/test.pdf'
47+
);
48+
49+
expect(result).toBe(mockDocument);
50+
expect(pdfjsLib.getDocument).toHaveBeenCalledWith({
51+
url: 'https://example.com/test.pdf',
52+
});
53+
});
54+
55+
it('should throw McpError when neither path nor url provided', async () => {
56+
await expect(loadPdfDocument({}, 'unknown')).rejects.toThrow(McpError);
57+
await expect(loadPdfDocument({}, 'unknown')).rejects.toThrow(
58+
"Source unknown missing 'path' or 'url'."
59+
);
60+
});
61+
62+
it('should handle file not found error (ENOENT)', async () => {
63+
const enoentError = Object.assign(new Error('File not found'), { code: 'ENOENT' });
64+
65+
vi.mocked(pathUtils.resolvePath).mockReturnValue('/safe/path/missing.pdf');
66+
vi.mocked(fs.readFile).mockRejectedValue(enoentError);
67+
68+
await expect(loadPdfDocument({ path: 'missing.pdf' }, 'missing.pdf')).rejects.toThrow(
69+
McpError
70+
);
71+
await expect(loadPdfDocument({ path: 'missing.pdf' }, 'missing.pdf')).rejects.toThrow(
72+
"File not found at 'missing.pdf'."
73+
);
74+
});
75+
76+
it('should handle generic file read errors', async () => {
77+
vi.mocked(pathUtils.resolvePath).mockReturnValue('/safe/path/error.pdf');
78+
vi.mocked(fs.readFile).mockRejectedValue(new Error('Permission denied'));
79+
80+
await expect(loadPdfDocument({ path: 'error.pdf' }, 'error.pdf')).rejects.toThrow(McpError);
81+
await expect(loadPdfDocument({ path: 'error.pdf' }, 'error.pdf')).rejects.toThrow(
82+
'Failed to prepare PDF source error.pdf. Reason: Permission denied'
83+
);
84+
});
85+
86+
it('should handle non-Error exceptions during file read', async () => {
87+
vi.mocked(pathUtils.resolvePath).mockReturnValue('/safe/path/test.pdf');
88+
vi.mocked(fs.readFile).mockRejectedValue('String error');
89+
90+
await expect(loadPdfDocument({ path: 'test.pdf' }, 'test.pdf')).rejects.toThrow(
91+
'Failed to prepare PDF source test.pdf. Reason: String error'
92+
);
93+
});
94+
95+
it('should handle PDF.js loading errors', async () => {
96+
const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
97+
const mockBuffer = Buffer.from('fake pdf');
98+
99+
vi.mocked(pathUtils.resolvePath).mockReturnValue('/safe/path/bad.pdf');
100+
vi.mocked(fs.readFile).mockResolvedValue(mockBuffer);
101+
vi.mocked(pdfjsLib.getDocument).mockReturnValue({
102+
promise: Promise.reject(new Error('Invalid PDF')),
103+
} as pdfjsLib.PDFDocumentLoadingTask);
104+
105+
await expect(loadPdfDocument({ path: 'bad.pdf' }, 'bad.pdf')).rejects.toThrow(McpError);
106+
await expect(loadPdfDocument({ path: 'bad.pdf' }, 'bad.pdf')).rejects.toThrow(
107+
'Failed to load PDF document from bad.pdf. Reason: Invalid PDF'
108+
);
109+
110+
expect(consoleErrorSpy).toHaveBeenCalledWith(
111+
expect.stringContaining('PDF.js loading error for bad.pdf'),
112+
expect.any(Error)
113+
);
114+
115+
consoleErrorSpy.mockRestore();
116+
});
117+
118+
it('should handle non-Error PDF.js loading exceptions', async () => {
119+
const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
120+
121+
vi.mocked(pdfjsLib.getDocument).mockReturnValue({
122+
promise: Promise.reject('Unknown error'),
123+
} as pdfjsLib.PDFDocumentLoadingTask);
124+
125+
await expect(
126+
loadPdfDocument({ url: 'https://example.com/bad.pdf' }, 'https://example.com/bad.pdf')
127+
).rejects.toThrow('Failed to load PDF document from https://example.com/bad.pdf');
128+
129+
consoleErrorSpy.mockRestore();
130+
});
131+
132+
it('should propagate McpError from resolvePath', async () => {
133+
const mcpError = new McpError(ErrorCode.InvalidRequest, 'Path validation failed');
134+
vi.mocked(pathUtils.resolvePath).mockImplementation(() => {
135+
throw mcpError;
136+
});
137+
138+
await expect(loadPdfDocument({ path: 'test.pdf' }, 'test.pdf')).rejects.toThrow(mcpError);
139+
});
140+
});
141+
});

0 commit comments

Comments
 (0)