Repository navigation
test: add 183 integration tests to close coverage gaps - #5
Conversation
Coverage improved from 80%→85% statements, 72%→77% branches, 89%→93% functions. Key areas covered: - wp-rest-api.ts: 56%→93% (PMC lists, envelope formats, teaser rejection) - http-fetch.ts: 80%→93% (WP AJAX, Prism API, Next.js fallback chains) - text-density-extractor.ts: 48%→69% (success path, metadata, error handling) - crawler.ts: →97% (sliding window, sitemap+robots, URL frontier) - http-client.ts: SSRF boundary tests (IPv4-mapped IPv6, 172.16/12 edges) - Cookie/proxy propagation through all 5 fallback paths - Security: adversarial HTML, prototype pollution, malformed inputs - Shared test-helpers.ts to reduce fixture duplication Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary of ChangesHello @teng-lin, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a comprehensive suite of new integration tests designed to enhance the robustness and reliability of the codebase. The primary goal is to address existing code coverage deficiencies, particularly in critical areas related to HTTP fetching, content extraction, and API interactions. By adding a substantial number of tests, the changes aim to ensure that various edge cases, fallback mechanisms, and security considerations are thoroughly validated, leading to a more stable and predictable system behavior. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This is an excellent pull request that significantly improves test coverage with a large number of high-quality integration tests. The tests are well-structured, cover a wide range of scenarios including complex fallback logic, security robustness, and various edge cases. The use of shared test helpers is also a great practice. I've found only a couple of very minor issues in the test code, which I've detailed in the comments.
| let article_id = "a1b2c3d4-e5f6-7890-abcd-ef1234567890"; | ||
| jQuery.ajax({ action: 'fetch_article_content', data: {id: articleId} }); |
There was a problem hiding this comment.
There's a potential reference error in this JavaScript snippet. The variable is declared as article_id, but then used as articleId inside the jQuery.ajax call. To make the script valid, you should use a consistent variable name. Renaming article_id to articleId would resolve this.
| let article_id = "a1b2c3d4-e5f6-7890-abcd-ef1234567890"; | |
| jQuery.ajax({ action: 'fetch_article_content', data: {id: articleId} }); | |
| let articleId = "a1b2c3d4-e5f6-7890-abcd-ef1234567890"; | |
| jQuery.ajax({ action: 'fetch_article_content', data: {id: articleId} }); |
There was a problem hiding this comment.
Addressed in commit 09f22ac: Fixed variable name from article_id to articleId for consistency.
| // Fragment-only link (#section-1) resolves to the same page → deduped as already visited | ||
| // /page#anchor and /page normalize to the same URL → fetched only once | ||
| const pageFetches = fetchedUrls.filter((u) => u === 'https://example.com/page'); | ||
| expect(pageFetches.length).toBeLessThanOrEqual(1); |
There was a problem hiding this comment.
This assertion is a bit weak. Given the test setup, the /page URL should be fetched exactly once. Using toBeLessThanOrEqual(1) would allow the test to pass even if the URL is not fetched at all (length 0), which would hide a potential bug in the URL normalization or frontier logic. A stricter assertion toBe(1) would be more appropriate here.
| expect(pageFetches.length).toBeLessThanOrEqual(1); | |
| expect(pageFetches.length).toBe(1); |
There was a problem hiding this comment.
Addressed in commit 09f22ac: Strengthened assertion from toBeLessThanOrEqual(1) to toBe(1) — the URL should always be fetched exactly once.
- Fix variable name inconsistency (article_id → articleId) in WP AJAX fixture - Strengthen assertion from toBeLessThanOrEqual(1) to toBe(1) for URL dedup test Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
wp-rest-api.ts(56%→93%),http-fetch.ts(80%→93%),text-density-extractor.ts(48%→69%)New test files
wp-rest-api.test.tshttp-fetch-fallbacks.integration.test.tssecurity-robustness.test.tstext-density-extractor.test.tsextraction-edge-cases.test.tscookie-proxy-integration.test.tscrawler-integration.test.tshttp-client-integration.test.tstest-helpers.tsTest plan
npm run lint— clean (only pre-existing warning)npm run format:check— all files passnpm run test— 866 tests passingnpm run build— compiles successfully🤖 Generated with Claude Code