test(cron): expand parseAbsoluteTimeMs test coverage to 39 cases#91656
Conversation
|
Codex review: passed. Reviewed June 19, 2026, 2:34 AM ET / 06:34 UTC. Summary PR surface: Tests +233. Total +233 across 1 file. Reproducibility: not applicable. this is a test coverage PR, not a runtime bug report with user steps. Source inspection confirms the requested parser coverage is still added only by this open PR path. Review metrics: none identified. Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Next step before merge
Security Review detailsBest possible solution: Land the repaired test-only coverage after a maintainer clears the human-review pause and the exact head remains green. Do we have a high-confidence way to reproduce the issue? Not applicable: this is a test coverage PR, not a runtime bug report with user steps. Source inspection confirms the requested parser coverage is still added only by this open PR path. Is this the best way to solve the issue? Yes. A test-only expansion in AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 5d6ac2308657. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Tests +233. Total +233 across 1 file. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
@vincentkoc please consider merge my code, thanks! |
|
@vincentkoc Drar sir, please consider merge my code, appreciate, Thanks! |
|
/clownfish automerge |
|
Clownfish is on the reef for this PR. 🐠 I tagged A maintainer can call |
|
🦞✅ Source: What merged:
Automerge notes:
The automerge loop is complete. Automerge progress:
|
|
🦞✅ Source: Why human review is needed: What the maintainer can do as a next step: I added |
|
Thanks, ClawSweeper. Clownfish picked up the reef notes and is starting a guarded repair pass. 🐠 Source: I will update this PR branch when I can. If GitHub branch permissions block that swim lane, I will open a safe credited replacement instead and keep it narrow. |
Add comprehensive timezone parsing tests for ISO 8601 variants: - UTC indicators (Z/z suffix) - Timezone offsets with colon (+HH:MM, -HH:MM) and without (+HHMM, -HHMM) - Sub-second precision (milliseconds, microseconds, nanoseconds) - Edge case timezones (+14:00, -12:00, +05:30, +12:45) - Whitespace trimming and empty string handling - Invalid format rejection - Real-world cron scheduling examples Test organization: - Epoch milliseconds (4 tests) - ISO date only (2 tests) - ISO datetime without timezone (2 tests) - ISO with Z suffix (5 tests) - ISO with colon timezone offsets (7 tests) - ISO with non-colon timezone offsets (3 tests) - ISO with milliseconds and timezone (3 tests) - Whitespace handling (2 tests) - Invalid formats (3 tests) - Edge cases (4 tests) - Real-world examples (2 tests) All 39 tests pass, validating correct parsing behavior across the full range of expected input formats for cron schedule timestamps.
c824d41 to
64a0548
Compare
|
Clownfish 🐠 reef update Thanks for the contribution here. Clownfish gave the branch a tidy little reef repair and kept this PR as the main lane. Source PR: #91656 fish notes: model gpt-5.5, reasoning medium; reviewed against 69a49d9. |
…nclaw#91656) Summary: - The PR expands `src/cron/parse.test.ts` with grouped `parseAbsoluteTimeMs` coverage for epoch, ISO timezone/offset, precision, whitespace, invalid-format, and cron example cases. - PR surface: Tests +233. Total +233 across 1 file. - Reproducibility: not applicable. this is a test coverage PR, not a runtime bug report with user steps. Source inspection confirms the requested parser coverage is still added only by this open PR path. Automerge notes: - Ran the ClawSweeper repair loop before final review. - Included post-review commit in the final squash: test(cron): expand parseAbsoluteTimeMs test coverage to 39 cases Validation: - ClawSweeper review passed for head 69a49d9. - Required merge gates passed before the squash merge. Prepared head SHA: 69a49d9 Review: openclaw#91656 (comment) Co-authored-by: 刘江0668001123 <[email protected]> Co-authored-by: openclaw-clownfish[bot] <280122609+openclaw-clownfish[bot]@users.noreply.github.com>
…nclaw#91656) Summary: - The PR expands `src/cron/parse.test.ts` with grouped `parseAbsoluteTimeMs` coverage for epoch, ISO timezone/offset, precision, whitespace, invalid-format, and cron example cases. - PR surface: Tests +233. Total +233 across 1 file. - Reproducibility: not applicable. this is a test coverage PR, not a runtime bug report with user steps. Source inspection confirms the requested parser coverage is still added only by this open PR path. Automerge notes: - Ran the ClawSweeper repair loop before final review. - Included post-review commit in the final squash: test(cron): expand parseAbsoluteTimeMs test coverage to 39 cases Validation: - ClawSweeper review passed for head 69a49d9. - Required merge gates passed before the squash merge. Prepared head SHA: 69a49d9 Review: openclaw#91656 (comment) Co-authored-by: 刘江0668001123 <[email protected]> Co-authored-by: openclaw-clownfish[bot] <280122609+openclaw-clownfish[bot]@users.noreply.github.com>
Summary
parseAbsoluteTimeMsinsrc/cron/parse.tshad minimal test coverage (only 2 test cases), leaving edge cases untested for ISO 8601 timezone parsing variants that cron schedules commonly use.src/cron/parse.tsimplementation — this is a test-only addition validating existing behavior.Change Type
Scope
Linked Issue
User-visible / Behavior Changes
None. Pure test coverage expansion.
Security Impact
Evidence
pnpm test src/cron/parse.test.tscompletes successfully with full coverage of:Zandzsuffix)+HH:MM,-HH:MM) including edge cases (+14:00, -12:00, +05:30, +12:45)+HHMM,-HHMM)Human Verification
Compatibility / Migration
AI assistance
AI-assisted (Claude Code). Tested locally (39 tests + typecheck/lint/format). Author understands the change.
Risks and Mitigations
parseAbsoluteTimeMsbehavior against expectedDate.parse()results.Real behavior proof
Behavior or issue addressed: Comprehensive test coverage for ISO 8601 timezone parsing edge cases previously untested. Validate that
parseAbsoluteTimeMscorrectly handles all ISO 8601 format variations used in cron scheduling without breaking existing behavior.Real environment tested: Local OpenClaw development environment on macOS, Node.js v22.22.1, Cron scheduling subsystem.
Exact steps or command run after this patch:
src/cron/parse.test.ts— expanded from 2 to 39 test cases.node node_modules/vitest/vitest.mjs run src/cron/parse.test.ts.pnpm tsgo.oxlint.oxfmt.Evidence after fix:
Observed result after fix: All 39 tests pass.
parseAbsoluteTimeMscorrectly parses epoch milliseconds, ISO 8601 date-only strings (normalized to midnight UTC), datetime without timezone (implicit UTC), UTC suffixes (Z/z), timezone offsets with and without colons (+08:00,-05:00,+0530,-1245), sub-second precision, and properly rejects invalid formats and whitespace-only input. No regressions in existing behavior.What was not tested: No manual verification against actual
Date.parse()engine differences across Node.js versions. No testing on Linux or Windows. No performance benchmarking with large batch timestamp parsing. No fuzz testing with random malformed inputs.