Skip to content
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

Update expected results for day period to match CLDR 47 #4428

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

anba
Copy link
Contributor

@anba anba commented Mar 17, 2025

@anba anba requested a review from a team as a code owner March 17, 2025 07:28
Copy link
Contributor

@gibson042 gibson042 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm approving this as-is, but would really love to see the tests made less brittle, e.g.

var inputs = [];
for (var h = 0; h < 24; h++) {
  inputs.push(new Date(2017, 11, 12,  h, 0, 0, 0));
}
assert.sameValue(inputs.length, 24);

// Each expected dayPeriod value must be a) contiguous, and
// b) represented in sequence.
var expectedDayPeriods = [
  'in the morning',
  'noon',
  'in the afternoon',
  'in the evening',
  'at night'
];

var long = new Intl.DateTimeFormat('en', {
  dayPeriod: 'long'
});
var longNumeric = new Intl.DateTimeFormat('en', {
  dayPeriod: 'long',
  hour: 'numeric'
});
var counts = expectedDayPeriods.map(function() { return 0; });
var transitionTargets = [];
var firstDayPeriod, prevDayPeriod;
for (var h = 0; h < 24; h++) {
  var dayPeriod = long.format(inputs[h]);
  var i = expectedDayPeriods.indexOf(dayPeriod);
  assert(i >= 0, 'dayPeriod must be expected: ' + dayPeriod);
  if (h === 0) {
    firstDayPeriod = dayPeriod;
    transitionTargets.push(dayPeriod);
  } else if (dayPeriod !== prevDayPeriod) {
    transitionTargets.push(dayPeriod);
    var oldCount = counts[i];
    // The first value might span the beginning and end of a day's hours.
    if (dayPeriod !== firstDayPeriod) {
      assert.sameValue(oldCount, 0,
        'dayPeriod must be contiguous: ' + dayPeriod);
    }
  }

  assert.sameValue(
    longNumeric.format(inputs[h]),
    // Hour "00" is represented as "12".
    ((h % 12) || 12) + ' ' + dayPeriod,
    'numeric hour must precede dayPeriod');

  counts[i]++;
  prevDayPeriod = dayPeriod;
}
if (transitionTargets.length > 1) {
  if (transitionTargets[transitionTargets.length - 1] === transitionTargets[0]) {
    transitionTargets.pop();
  }
  // Rotate the first expected dayPeriod into initial position.
  var i = transitionTargets.indexOf(expectedDayPeriods[0]);
  if (i > 0) {
    var values = transitionTargets.splice(0, i);
    transitionTargets = transitionTargets.concat(values);
  }
}
assert.compareArray(transitionTargets, expectedDayPeriods,
  'dayPeriods must appear in the right sequence');

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.

2 participants