Skip to content

Conversation

@SATVIKsynopsis
Copy link
Contributor

@SATVIKsynopsis SATVIKsynopsis commented Dec 12, 2025

added updates to #7215

hi i have updated the date addition test suite with a more comprehensive set of cases based on earlier discussion.

the main date_addition_snapshot.rs test now covers a much wider range of dates (iso 2000–2004) so leap years, leap days, and month boundaries are properly executed.

this produces fairly large snapshot files but I believe it gets us closer to the complete coverage we were aiming for.

i also added date_optimized_snapshot.rs, which doesn’t produce snapshots but gives a compact human readable summary of the chosen date and duration ranges.

what i thought was the idea to make it easier to inspect the test configuration without scrolling through thousands of snapshot lines.
if this helper isn’t needed or could be structured better i am totally open to adjusting or removing it. and all the tests passed locally.

i am not fully sure that these changes do exactly what you had in mind, so please feel free to suggest changes. i will update things as said.

happy to revise anything here to better match expectations.

@SATVIKsynopsis
Copy link
Contributor Author

SATVIKsynopsis commented Dec 15, 2025

CI is currently failing due to the size and runtime of the snapshot tests across calendars (macOS and test-gigo appear to fail first, with other jobs cancelled).

I validated the exhaustive date * duration * calendar matrix locally to confirm correctness (especially around leap days and leap months), but I understand committing exhaustive snapshots may not be desirable.

I am happy to reduce this to:

  1. a small, human-readable snapshot targeting leap boundaries, or

  2. a reduced per-calendar snapshot with a narrow date / duration range

Let me know which direction you would prefer, and I will adjust accordingly.

@SATVIKsynopsis
Copy link
Contributor Author

SATVIKsynopsis commented Dec 15, 2025

i have now kept the exhaustive date × duration × calendar coverage as an ignored test, so it doesn’t affect ci runtime. i validated the full matrix locally to ensure leap days and leap months behave correctly across calendars.

for the pr itself, i am happy to proceed in one of the following ways:

  1. commit a small human readable snapshot focused on leap boundaries

  2. add a reduced per calendar snapshot with a narrow date / duration range

  3. include the full snapshots if you feel that’s useful despite the size as you asked.

let me know which direction you would prefer.

@SATVIKsynopsis
Copy link
Contributor Author

rebased onto latest main after the snapshot refactor and resolved conflicts.

the pr now keeps only the optimized snapshot test and a snapshot output file with no large snapshot files committed. all CI checks pass too.

happy to add reduced or full snapshots if you would like, but I kept this minimal for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant