Add DateTimeFormat.prototype.formatToParts tests #734
Conversation
zbraniecki
force-pushed
the
DTf.formatToParts
branch
from
4360069
to
c3ee132
Compare
|
@jugglinmike @leobalter @tcare Per TC39 discussion today, the formatToParts proposal needs this merged to finalize Stage 4. At the moment, @leobalter is reviewing (he's sitting next to me) |
|
Ok, I think there are some minor coverage holes here. Due to willing to get this done quickly, I'm doing some coordinated work with @zbraniecki and we'll have @tcare to review it. |
|
I've wrote some additional tests. There's something else I'll give feedback within the main.js file. |
| dtf.format(value), | ||
| reduce(dtf.formatToParts(value)), | ||
| `Expected the same value for value ${value}, | ||
| locales: ${locales} and options: ${options}` |
There was a problem hiding this comment.
can we avoid using DateTimeFormat#format to compare the values? In any other case, I believe it would be good to have a minimum check for type and value for each part.
There was a problem hiding this comment.
We can, but it's a useful shortcut I think.
Would you prefer me to compare all tests to a result Array?
|
@leobalter - I merged your changes, thank you! |
zbraniecki
force-pushed
the
DTf.formatToParts
branch
from
ab57c8c
to
876a848
Compare
| 1. If _x_ is not a finite Number, throw a *RangeError* exception. | ||
| ---*/ | ||
|
|
||
| Intl.DateTimeFormat.prototype.formatToParts |
There was a problem hiding this comment.
I slipped here, this line can be removed.
There was a problem hiding this comment.
but it passes! :) j.k., removed.
zbraniecki
force-pushed
the
DTf.formatToParts
branch
from
876a848
to
605143b
Compare
| assert.sameValue( | ||
| typeof Intl.DateTimeFormat.prototype.formatToParts, | ||
| 'function', | ||
| '`typeof Intl.DatetTimeFormat.prototype.formatToParts` is `function`' |
There was a problem hiding this comment.
Nit: there's a typo here in DateTimeFormat (Datet).
zbraniecki
force-pushed
the
DTf.formatToParts
branch
from
605143b
to
838e3b4
Compare
|
Looks good. |
|
@leobalter 2nd signoff and I'll merge? |
|
Ok, +1. I believe this can be improved with more static results but tests are already valid and the improvement can land afterwards. |
|
Agreed, sufficient enough for stage 4. Merging. |
This is a set of tests for tc39/ecma402#64 which we'd like to land in the spec soon.
I'd like to get the test262 tests for this change ready to land as part of the getting the proposal to stage 4.
I'm not sure how much more testing is required since most of the feature is tested in
DateTimeFormat.prototype.formattests, and this is just a different representation of the output.I'd like to get the PR to be ready to land once we land the feature in the spec, so would like to get it reviewed now.
@leobalter, @jugglinmike - can you review this?