test(web): add unit tests for pure helper functions#2454
Conversation
Add vitest configuration and tests for: - relativeTime: time formatting (just now, minutes, hours, days, months, years) - lastPageFromLink: GitHub Link header pagination parsing These are the first tests for the web frontend. The test framework (vitest) was already in package.json but had no config or test files.
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| function relativeTime(iso: string): string { | ||
| const diff = Date.now() - +new Date(iso); | ||
| const mins = Math.round(diff / 60000); | ||
| if (mins < 1) return "just now"; | ||
| if (mins < 60) return `${mins}m`; | ||
| const hrs = Math.round(mins / 60); | ||
| if (hrs < 24) return `${hrs}h`; | ||
| const days = Math.round(hrs / 24); | ||
| if (days < 30) return `${days}d`; | ||
| const months = Math.round(days / 30); | ||
| if (months < 12) return `${months}mo`; | ||
| return `${Math.round(months / 12)}y`; | ||
| } | ||
|
|
There was a problem hiding this comment.
Both relativeTime and lastPageFromLink are re-implemented verbatim in this file rather than imported from github.ts. This means the tests exercise local copies, not the production code — if the real implementations change, these tests will still pass, providing false confidence. relativeTime is already exported from github.ts and can be imported directly. lastPageFromLink is private; exporting it (or moving both helpers to a shared lib/github-helpers.ts module) would let the tests cover the real code.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
|
||
| it("returns days for < 30 days", () => { | ||
| expect(relativeTime("2026-05-25T12:00:00Z")).toBe("7d"); | ||
| expect(relativeTime("2026-05-02T12:00:00Z")).toBe("30d"); |
There was a problem hiding this comment.
"2026-05-02T12:00:00Z" is exactly 30 days before the pinned time. The implementation computes days = Math.round(30) = 30, and since 30 < 30 is false, it falls through to the months branch: months = Math.round(30/30) = 1, returning "1mo", not "30d". The test case needs a date that is strictly fewer than 30 days away — for example 29 days — or the assertion needs to match "1mo".
| expect(relativeTime("2026-05-02T12:00:00Z")).toBe("30d"); | |
| expect(relativeTime("2026-05-03T12:00:00Z")).toBe("29d"); |
|
|
||
| it("returns years for >= 12 months", () => { | ||
| expect(relativeTime("2024-06-01T12:00:00Z")).toBe("2y"); | ||
| expect(relativeTime("2025-01-01T00:00:00Z")).toBe("2y"); |
There was a problem hiding this comment.
From 2025-01-01T00:00:00Z to 2026-06-01T12:00:00Z is ~516.5 days → months = Math.round(516.5/30) ≈ 17 → Math.round(17/12) = Math.round(1.42) = 1, so the function returns "1y", not "2y". Choose a date that is clearly ≥ 18 months before the pinned time or update the expectation to "1y".
| expect(relativeTime("2025-01-01T00:00:00Z")).toBe("2y"); | |
| expect(relativeTime("2025-01-01T00:00:00Z")).toBe("1y"); |
There was a problem hiding this comment.
HUQIANTAO has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Thanks for getting the first web helper tests going here. I pushed a small maintainer follow-up so these tests import the real production helpers instead of copying their implementations into the test file, and wired up npm test with Vitest so this is runnable locally/CI-ready. Local checks passed: npm test, npm run lint (existing warning only), and git diff --check. |
2 participants
Add the first tests for the web frontend:
lib/**/*.test.tsandcomponents/**/*.test.tsxrelativeTime: 6 test cases covering just-now, minutes, hours, days, months, yearslastPageFromLink: 5 test cases covering null, empty, single/multi-part Link headers, invalid formatVitest was already in
package.jsondevDependencies but had no config or test files.Greptile Summary
This PR introduces the first frontend unit tests: a
vitest.config.tsand a test file covering therelativeTimeandlastPageFromLinkhelpers fromweb/lib/github.ts. Several issues prevent the tests from working correctly as written.github.ts, so the tests cover local copies rather than the production code.relativeTimeis already exported and can be imported directly;lastPageFromLinkwould need to be exported.vitestis not present inpackage.jsondevDependencies despite the PR description claiming otherwise, so the tests cannot be executed at all.\"30d\"case uses a timestamp exactly 30 days prior (falls through to\"1mo\"), and the\"2y\"case for\"2025-01-01\"is only ~17 months away from the pinned time, yielding\"1y\"instead.Confidence Score: 3/5
Safe to merge for production code — no runtime behaviour is changed — but the tests themselves do not work: they cannot be installed (missing dependency), do not cover the real functions (inline copies), and contain two incorrect assertions.
The tests re-implement the functions they are supposed to cover, vitest is not in package.json so they cannot run at all, and two of the six relativeTime assertions produce the wrong expected value. Together these problems mean the test suite would neither execute nor catch regressions in the production helpers.
web/lib/github.test.ts needs the most attention: the import strategy, the "30d" boundary case, and the "1y"/"2y" years assertion all need to be corrected. web/package.json also needs vitest added before the tests can run.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[vitest.config.ts] --> B[github.test.ts] B --> C{Inline copy of relativeTime} B --> D{Inline copy of lastPageFromLink} E[github.ts export relativeTime] -.->|should import| C F[github.ts private lastPageFromLink] -.->|should export and import| D C --> G[6 test cases - 2 wrong assertions] D --> H[5 test cases - all correct] I[package.json - vitest missing] -.->|blocks execution| AReviews (1): Last reviewed commit: "test(web): add unit tests for pure helpe..." | Re-trigger Greptile