Merged
Conversation
If every subsecond component is ≤999, there's no need to list them out separately - there won't be any ambiguity in the serialized string. This makes the snapshot files a bit smaller and will save some space in the large durationroundingrelative.snapshot.json.
This makes sure that the invariant holds even without constraining.
This tests many different combinations of largestUnit, smallestUnit,
roundingMode, roundingIncrement, and relativeTo, checking that the
invariant holds:
duration.round({ ...options, relativeTo })
must give the same duration as
relativeTo.until(relativeTo.add(duration), options)
I wanted to make this into a snapshot test as well, but there was really
no way to cover enough of the testing space while keeping the snapshot
file within GitHub's 100 MB file size limit.
This changes the snapshot tests CI job to not run unless the label "run snapshot tests" is applied to the PR. (The snapshot tests take 15-20 minutes on CI, which is a bit much if you're just changing something in the spec text.)
ptomato
added
the
run snapshot tests
justingrant
approved these changes
Mar 3, 2026
Collaborator
justingrant
left a comment
justingrant
left a comment
There was a problem hiding this comment.
Looks good. Maybe add some text to the readme to explain the magic label needed to run these?
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3288 +/- ##
=======================================
Coverage 97.40% 97.40%
=======================================
Files 22 22
Lines 10771 10771
Branches 1866 1866
=======================================
Hits 10492 10492
Misses 258 258
Partials 21 21 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Apparently I made a JSON file that couldn't be read.
V8 unflagged Temporal, so there's no longer any need for this argument.
Collaborator
Author
Thanks for the review. I looked at adding it to the readme but it seemed out of place there, since we don't have any other contributing instructions. I've added a PR template instead, so contributors will see it when they open their PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This tests many different combinations of largestUnit, smallestUnit, roundingMode, roundingIncrement, and relativeTo, checking that the invariant holds:
must give the same duration as
I wanted to make this into a snapshot test as well, but there was really no way to cover enough of the testing space while keeping the snapshot file within GitHub's 100 MB file size limit.
Also a couple of cleanups for the snapshot tests, and a change to the CI configuration so that they only run when you add the label "run snapshot tests". Currently they take 15-20 minutes to run on CI, which is a bit much if you're only making a textual change.