test262 icon indicating copy to clipboard operation
test262 copied to clipboard

Adds PYM.since rounding mode specs and related helper

Open sarahghp opened this issue 3 years ago • 5 comments

Like it says on the tin, this one adds rounding mode specs and a proposed helper.

I broke this out from the other updates (https://github.com/tc39/test262/pull/3313, https://github.com/tc39/test262/pull/3314) so we can discuss the helper and its context.

sarahghp avatar Nov 17 '21 18:11 sarahghp

Hey, @rwaldron (long time, no see!) — I was curious what you all might think about adding this testEach helper. Using the tagged template literal to make a table is a bit more readable (imo) than an array of array arguments, but otherwise it's just sugar on forEach.

Thanks!

sarahghp avatar Nov 17 '21 18:11 sarahghp

@sarahghp omg hi! Excited to see you working on Test262 tests :D!!

rwaldron avatar Nov 30 '21 19:11 rwaldron

was curious what you all might think about adding this testEach helper

While I do think this is interesting, I'm concerned that it abstracts too much away into a helper. Frankly, I think most of the existing Temporal harness helpers need to be unrolled and the tests themselves should be written out in each file *. I think @jugglinmike probably agrees and will have additional thoughts on the matter. But again, I do think this is clever and cool :)

* More on that: I'm on a quest to eliminate harness stuff whenever possible. Admitting the Temporal helpers was done to allow the stage 3 work to continue expediently, with a sort of loose plan to one day write a tool that would generate the assertions that are currently hidden away in the helpers, putting them directly in the appropriate test files.

rwaldron avatar Nov 30 '21 19:11 rwaldron

@rwaldron's thoughts match my own. While stewarding Test262, he and I have set a pretty high bar for abstractions, both due to their many drawbacks and due to the aspects of standards testing which limit their benefit.

The drawbacks to abstraction include:

  • it degrades the tests by introducing unrelated semantics
  • it discourages contributors by requiring them to learn more
  • it frustrates implementers by making it harder to understand what's being tested and what has failed

One of abstraction's common motivations is its tendency to reduce maintenance costs by limiting duplication. TC39 has a commitment to never break backwards compatibility in ECMA262. This gives us a certain assurance in Test262 that maintainers of other test suites do not enjoy: Test262's tests will never be invalidated. (There are some exceptions for when TC39 intentionally reserves extensions for future standardization, e.g. in the ImportCall production, but these aren't common.) We take advantage of this by using a more declarative, readable, and verbose style, recognizing that we aren't likely to revisit the files.

jugglinmike avatar Jan 24 '22 20:01 jugglinmike

Having thought more about this, I still don't think this case meets the bar for new harness API. A minor refactoring produces less code and achieves the same result (both organizationally and within the goals of the test material itself), without the overhead of learning a new assertion mechanism:

const roundingModes = [
  ['halfExpand', 'years',  'P3Y', '-P3Y'],
  ['halfExpand', 'months', 'P2Y8M', '-P2Y8M'],
  ['ceil', 'years',  'P3Y',   '-P2Y'],
  ['ceil', 'months', 'P2Y8M', '-P2Y8M'],
  ['floor', 'years',  'P2Y',   '-P3Y'],
  ['floor', 'months', 'P2Y8M', '-P2Y8M'],
  ['trunc', 'years',  'P2Y',   '-P2Y'],
  ['trunc', 'months', 'P2Y8M', '-P2Y8M'],
];

for (const [roundingMode, smallestUnit, expectedPositive, expectedNegative] of roundingModes) {
  const positiveDescription = `${roundingMode}, ${smallestUnit} rounds positive as expected`;
  const negativeDescription = `${roundingMode}, ${smallestUnit} rounds negative as expected`;

  const positiveComparison = later.since(earlier, { smallestUnit, roundingMode }).toString();
  const negativeComparison = earlier.since(later, { smallestUnit, roundingMode }).toString();

  assert.sameValue(positiveComparison, expectedPositive, positiveDescription);
  assert.sameValue(negativeComparison, expectedNegative, negativeDescription);
}

rwaldron avatar Jan 27 '22 22:01 rwaldron

In the end this was covered by https://github.com/tc39/test262/pull/3657, so it can be closed. I'd love to discuss having more facilities for table-driven tests, as a lot of Temporal and also Intl tests would use them, but let's use the RFC process for that.

ptomato avatar Sep 21 '22 19:09 ptomato