Add different formatting for test/intl402/DurationFormat/prototype/format/style-digital-en.js

[Constellation]

ECMA-402 does not require specific CLDR format, and each platform has some customization for their platform consistency.
macOS system ICU is shipping new CLDR, but it has many overrides on the top of it to make the formatted output suitable for the system.
This change modifies the test to accept that alternative output from AppleICU.

[Constellation]

@rwaldron @sffc @FrankYFTang

[ptomato]

Thanks for the submission. Clearly we need to correct this, as the spec does not mandate a particular locale format, and we should not have a test hardcoded to one engine's output.

However, I'd prefer not to perpetuate the problem by having a test hardcoded to two engines' outputs. See #3786.

You probably know the Intl.DurationFormat spec better than I do here, so what are the essential requirements of formatting {style: 'digital'} in English? Could we say that the output must have the number of years, months, weeks, days, hours, minutes, and seconds in order, and that there must be some form of unit (y.*, m.*, w.*, d.*) in between the first five quantities, and two colons in between the hours, minutes, and seconds? Maybe there's a regexp that would express that.

(A minor point, if we aren't using assert.sameValue() anymore, make sure to include the actual value, and expected value if applicable, in the error message, to help debugging.)

[sffc]

Please also note that the default length for digital units is "short" not "narrow" since tc39/proposal-intl-duration-format@4c24876

[FrankYFTang]

expected and expected1 of using " " for separator instead of ", " is incorrect. There is a v8 bug which does not map digital to short style but narrow style and that should be fixed.

[ptomato]

By my reading, either one is permitted by the spec. These separators come from joining the elements together with ListFormat with {type:'unit'}, which is ILD.

(Seems like most implementations currently use ", " in the en locale, while MDN documentation suggests that you will get " ".)

[Constellation]

Changed it to accept comma via RegExp.

[sffc]

An appropriate test would be one where year/month/day get formatted in default (short) format and then again in digital format, and the results should be the same per the spec.

[FrankYFTang]

How about we change the testing for the duration format to write testing code which use Intl.ListFormat and Intl.NumberFormat to generate expectation and compare with the result produced by the Intl.DurationFormat.

[Constellation]

An appropriate test would be one where year/month/day get formatted in default (short) format and then again in digital format, and the results should be the same per the spec.

Oops, right. Fixed.

[Constellation]

How about we change the testing for the duration format to write testing code which use Intl.ListFormat and Intl.NumberFormat to generate expectation and compare with the result produced by the Intl.DurationFormat.

Right now, I'll just fix the broken test in the repository.

[Constellation]

Thanks for the submission. Clearly we need to correct this, as the spec does not mandate a particular locale format, and we should not have a test hardcoded to one engine's output.

However, I'd prefer not to perpetuate the problem by having a test hardcoded to two engines' outputs. See #3786.

You probably know the Intl.DurationFormat spec better than I do here, so what are the essential requirements of formatting {style: 'digital'} in English? Could we say that the output must have the number of years, months, weeks, days, hours, minutes, and seconds in order, and that there must be some form of unit (y.*, m.*, w.*, d.*) in between the first five quantities, and two colons in between the hours, minutes, and seconds? Maybe there's a regexp that would express that.

(A minor point, if we aren't using assert.sameValue() anymore, make sure to include the actual value, and expected value if applicable, in the error message, to help debugging.)

Changed it to RegExp approach.

[ptomato]

Thanks, works for me, I'll merge it if no-one else suggests any improvements.

[anba]

IMO, applying Frank's suggestion seems the best solution, because it results in the output which is required by the spec:

const parts = [
  new Intl.NumberFormat("en", {style: "unit", unit: "year", unitDisplay: "short"}).format(duration.years),
  new Intl.NumberFormat("en", {style: "unit", unit: "month", unitDisplay: "short"}).format(duration.months),
  new Intl.NumberFormat("en", {style: "unit", unit: "week", unitDisplay: "short"}).format(duration.weeks),
  new Intl.NumberFormat("en", {style: "unit", unit: "day", unitDisplay: "short"}).format(duration.days),
  [
    new Intl.NumberFormat("en", {}).format(duration.hours),
    new Intl.NumberFormat("en", {minimumIntegerDigits: 2}).format(duration.minutes),
    new Intl.NumberFormat("en", {minimumIntegerDigits: 2, minimumFractionDigits: 0, maximumFractionDigits: 9, roundingMode: "trunc"}).format(
      duration.seconds + (duration.milliseconds / 10**3) + (duration.microseconds / 10**6) + (duration.nanoseconds / 10 ** 9)
    ),
  ].join(":")
];
const expected = new Intl.ListFormat("en", {type: "unit", style: "short"}).format(parts);

// And then change the assertion to:
assert.sameValue(df.format(duration), expected, `Assert DurationFormat format output using ${style} style option`);

[FrankYFTang]

Thanks for the submission. Clearly we need to correct this, as the spec does not mandate a particular locale format, and we should not have a test hardcoded to one engine's output.
However, I'd prefer not to perpetuate the problem by having a test hardcoded to two engines' outputs. See #3786.
You probably know the Intl.DurationFormat spec better than I do here, so what are the essential requirements of formatting {style: 'digital'} in English? Could we say that the output must have the number of years, months, weeks, days, hours, minutes, and seconds in order, and that there must be some form of unit (y.*, m.*, w.*, d.*) in between the first five quantities, and two colons in between the hours, minutes, and seconds? Maybe there's a regexp that would express that.
(A minor point, if we aren't using assert.sameValue() anymore, make sure to include the actual value, and expected value if applicable, in the error message, to help debugging.)

Changed it to RegExp approach.

Why we need a RegExp to make it ",? " here instead of requiring ", " ?

In what case the one without "," is the correct expectation?

[Constellation]

@FrankYFTang Currently, just keeping the original expectation since the original test was accepting that.
It seems that this test is added in 4fda9c4.
Did you add that based on some ICU configuration?

[FrankYFTang]

Did you add that based on some ICU configuration?

no, that is based on the spec text prior to the merge of
https://github.com/tc39/proposal-intl-duration-format/pull/127/files#diff-7d681727fcf47dc0b9a7512a470fb0da63276c625891a5cc232d725bd12912fd

That PR got merged but the test was not updated to sync to that PR. (nor the v8 implementation till https://chromium-review.googlesource.com/c/v8/v8/+/4781659 (which is under review))

[Constellation]

@FrankYFTang OK, I've just changed it to comma split one.
@ptomato Can you merge it?

[FrankYFTang]

LGTM

[ptomato]

I'd prefer that we implement @anba's suggestion in #3895 (comment), to avoid hard-coded expectations, but since (1) that's probably not what you signed up for when you opened the PR and (2) it seems like this particular hard-coded output is acceptable to everyone who commented on this thread and does not make the situation worse than it already was, I'll merge it now. See #3786 for follow up.