Repository navigation
test(frontend): use runtime-relative dates in api-client-review tests - #3533
Conversation
Replace fixed calendar timestamps with dates computed from Date.now(). Expected values reuse the same constants; ordering and gaps are kept (invites still expire 7 days after creation). Signed-off-by: Christopher <131207452+PetrefiedThunder@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: fa4b4bf7-e3fa-4590-bc8d-4126fa7ef0ab) |
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes.
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Description check |  |
The description explains the test changes and reports the test result, but it omits most required template sections, including Type of Change, Checklist, and Related Issues. The Summary and Test Plan … | Add the required Summary, Type of Change, Test Plan, Checklist, and Related Issues sections. Include the applicable checkbox selections and link or reference issue #3514 in Related Issues. |
| Docstring Coverage | |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly describes the main change: updating API-client review tests to use runtime-relative dates. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Description check
-
Explanation
The description explains the test changes and reports the test result, but it omits most required template sections, including Type of Change, Checklist, and Related Issues. The Summary and Test Plan also do not use the required structure.
Resolution
Add the required Summary, Type of Change, Test Plan, Checklist, and Related Issues sections. Include the applicable checkbox selections and link or reference issue
#3514in Related Issues.
✨ Finishing Touches: 💡 2
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
PR Blocker ChecklistHead SHA: Status: Ready
|
PetrefiedThunder
left a comment
PetrefiedThunder
left a comment
There was a problem hiding this comment.
REVIEW: FAIL gemini 7f01099
Findings:
- Dates: good. NOW_MS is read once at module load. The derived ISO strings are only passed through and checked with exact toEqual. The client does no expiry, elapsed time or relative formatting on them, so there is no risk from midnight, DST or timezone and fake timers are not needed. Order and gaps match the old fixtures, and invites still expire 7 days after creation.
- Invented data remains in the same file, so it is not data-free yet:
- emails owner@example.com (L313, L346) and pending@example.com (L331, L354)
- tenant name 'Acme' and id 'tenant-1' (L160, L163)
- 'Deploy key', 'Viewer', 'Preview text'
- fixed ids rev-1, key-1, user-1, role-1, invite-1, evt-1, LOT-n
Generate ids and emails at runtime (crypto.randomUUID) or use real recorded data. If a value cannot be made data-free, add a "Coverage gap" note.
- Assertions: still meaningful (URL and method contract, CSRF and tenant headers, review_id to id mapping, CSV count and hash validation).
- CI at this SHA: api-client-review.test.ts passed (22 tests). Unit Tests failed in 5 other files this PR does not touch: dashboard-load-errors, dashboard-qa-agreement, qa-health, suppliers-metrics and suppliers-removal. PR Gate: Frontend, Frontend CI Status Gate, Build & Bundle Analysis, Bundle Size Check, Workflow Contract, Secrets, SAST, Tool Pinning, Unused File Guard and Vercel are red. Not merging: merge freeze.
…review tests Signed-off-by: Christopher <131207452+PetrefiedThunder@users.noreply.github.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: 975a4da7-2ada-4b11-8fcb-c61a8ad30f95) |
PetrefiedThunder
left a comment
PetrefiedThunder
left a comment
There was a problem hiding this comment.
REVIEW: FAIL gemini eeae7d3
Delta 7f01099..eeae7d3 checked against my last FAIL:
- Fixed: every id, email, tenant name, key name, preview text and lot code is now generated at runtime with crypto.randomUUID. Emails use the reserved .invalid TLD. Each expected value reuses its input constant, so the assertions are unchanged in strength.
- Verified as real: 'Viewer' (services/ingestion/app/team_mgmt.py, viewer -> label "Viewer"). Error strings 'Tenant mismatch', 'Authentication required', 'CSV contained no data rows', 'No valid rows found', 'No CTE type' and 'Request failed' all exist in admin/ingestion/frontend source. 'Field required' is Pydantic v2's standard message. mobile_capture_pwa_offline and ci_artifact exist in source. The csrf-token header value, the 64-char hash pattern, and invalid inputs on purpose ('not-a-hash', 'not an array', 'not a display message') are acceptable.
- Still invented. These two are free text that no source emits, so replace them with real backend strings or uid():
- errors: ['persistence warning'] (L310): not found anywhere in services/ or frontend/src.
Row ${index + 2}: rejectedin csvResult (L67): the ": rejected" text is made up. Use a real csv_templates.py row error (e.g. "Row N: No CTE type", already used at L257).
- CI: api-client-review.test.ts passes (22 tests). The Unit Tests failures (5 files, 10 tests) are identical by name to main's job 120366116851, so they are not caused by this PR. Other red checks (PR Gate: Frontend, Frontend CI Status Gate, Bundle, Workflow Contract, Secrets, SAST, Tool Pinning, Unused File Guard, Vercel) are unchanged from the previous head. Not merging: merge freeze.
…api-client-review tests Signed-off-by: Christopher <131207452+PetrefiedThunder@users.noreply.github.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: 7bedac72-2789-411b-8031-59a06373dc4e) |
PetrefiedThunder
left a comment
PetrefiedThunder
left a comment
There was a problem hiding this comment.
REVIEW: PASS gemini 6dcfbf8
Delta eeae7d3..6dcfbf8 touches only api-client-review.test.ts, two lines:
- L67:
Row ${index + 2}: quantity is requiredexactly matches services/ingestion/app/csv_templates.py:862 (f"Row {row_num}: quantity is required"). - L310: errors: [uid()] replaces the invented 'persistence warning'.
No other changes and no new invented data. Together with my earlier checks, the file is now data-free: runtime uuid ids and .invalid emails, plus only real role labels and real error strings.
CI: Unit Tests job 120506847764 logs show api-client-review.test.ts passing (22 tests). The only failures are the same 10 tests in 5 files that also fail on main (job 120366116851). The other red checks are the same as at the previous heads. Not merging: merge freeze.
Branch updated with mainMain was merged into Because merge commits created with GITHUB_TOKEN do not trigger workflows, a push to this branch or a manual CI re-run is needed for checks to run on it. Last synced: 2026-10-24T02:11:41.204Z |
Signed-off-by: Christopher <131207452+PetrefiedThunder@users.noreply.github.com>
PetrefiedThunder
left a comment
PetrefiedThunder
left a comment
There was a problem hiding this comment.
REVIEW: PASS gemini a4af4a4
Delta 6dcfbf8..a4af4a4 is only the merge of main (#3552 658bc7f) plus an empty retrigger commit. The PR's own change to frontend/src/tests/lib/api-client-review.test.ts is byte identical to the head I passed at 6dcfbf8: runtime relative dates, no invented data. CI was still running at review time; merge still needs fully green CI.
✅ Bundle Size Report
Bundle is within the 10.0MB limit. |
PetrefiedThunder
left a comment
PetrefiedThunder
left a comment
There was a problem hiding this comment.
Delta review: since the last PASS, the only change is a clean sync merge of main with no manual conflict resolution.
REVIEW: PASS pi 5591dbf
Makes
frontend/src/__tests__/lib/api-client-review.test.tsdata-free. The remaining fixed timestamps (ingestion_timestamp, review created_at/updated_at, API key created_at, invite created_at/expires_at) are now constants computed fromDate.now()at runtime. Each expected value reuses the same constant, so the assertions are unchanged. The original order and gaps are kept (review < invite < key < ingestion), and invites still expire 7 days after creation.Follow-up to #3514. Deferred:
qa-health.test.tsx(touched by #3524).Tests:
vitest run src/__tests__/lib/api-client-review.test.ts --execArgv=--no-experimental-webstorage: 22 passed.VS Code Gemini reviews; do not merge (merge freeze).
Note
Low Risk
Test-only fixture refactors in one Vitest file; no application or API client logic changes.
Overview
Refactors
api-client-review.test.tsso fixtures no longer use hardcoded calendar dates, tenant names, emails, or IDs—part of making api-client contract tests data-free (follow-up to #3514).Timestamps are derived from
Date.now()at load time (INGESTED_AT,REVIEW_CREATED_AT, invite/key dates, etc.), keeping the same relative ordering and the 7-day invite expiry window while assertions still compare mock input to the same constants.Identifiers and copy use
crypto.randomUUID()(and@example.invalidemails) shared between mocked responses and expectations; approve/reject/key URLs and normalized objects wire through those constants. The system role label staysViewer(aligned with ingestion team_mgmt). Rejected CSV row errors incsvResultnow use a concrete validation-style message (quantity is required) instead of a genericrejectedstring.No production
api-clientbehavior changes—contract coverage is unchanged, only fixture hygiene.Reviewed by Cursor Bugbot for commit a4af4a4. Bugbot is set up for automated code reviews on this repo. Configure here.