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

Math.sumPrecise testing plan #4054

Open
13 of 29 tasks
ptomato opened this issue Apr 12, 2024 · 3 comments
Open
13 of 29 tasks

Math.sumPrecise testing plan #4054

ptomato opened this issue Apr 12, 2024 · 3 comments

Comments

@ptomato
Copy link
Contributor

ptomato commented Apr 12, 2024

Boilerplate

  • Property descriptor
  • Function length
  • Function name
  • Is not a constructor
  • prototype is Function.prototype
  • is extensible

Success cases

  • Sum of finite numbers (same sign, mixed signs)
  • Sum of one number is that number (negative, -0, 0, positive, NaN, ∞, -∞)
  • Sum of empty is -0
  • Sum of finite numbers with infinite result (∞, -∞)
  • Sum of multiple numbers including any NaN is NaN
  • Sum including any ∞ and no NaN or -∞ is ∞
  • Sum including any -∞ and no NaN or ∞ is -∞
  • Sum of ∞ and -∞ is NaN
  • Sum of multiple -0 is -0
  • Sum of multiple 0 is 0
  • Sum of -0 and 0 is 0
  • Sum of a non-array iterable works like sum of an array
  • Sum of an empty non-array iterable is -0

Failure cases

  • rejects non-Number values in iterable (Boolean, String, Symbol, BigInt, Object, null, undefined)

Other edge cases

  • Not passing an argument throws TypeError
  • summing each type of primitive throws TypeError
  • summing a non-iterable object throws TypeError
  • iterable yields more than 2⁵³ items (probably not testable)
  • A case where sumPrecise disagrees with naive sum via reduce
  • Order of observable operations
  • iterator is closed (return is called) after yielding a non-Number
  • an Object value in the iterable does not have its valueOf, toString, or [Symbol.toPrimitive] methods called
  • iterator is still exhausted after the sum has reached the NaN or ±∞ states
  • array is iterated via its [Symbol.iterator] method, not by getting length
@ptomato
Copy link
Contributor Author

ptomato commented Apr 12, 2024

I'm trying to put together guidance on writing a testing plan, so I thought I'd use this proposal as a sample.

There may be some things in this list that are past the point of diminishing returns, like repeating all of the success cases for both arrays and non-array iterables, or all of the iterator protocol boilerplate.

@bakkot
Copy link
Contributor

bakkot commented Apr 14, 2024

This looks like a good amount of detail for a testing plan to me, but I agree that many of the tests are past the point of diminishing returns. In particular:

  • I wouldn't bother with "is extensible" or "prototype is Function.prototype"; those are not tested for most existing functions and it is hard to see how an implementation might get them wrong.
  • I wouldn't bother with any of the tests for conforming to the iterator protocol other than basic tests to confirm that it does consume its argument using the iterable protocol, rather than e.g. as an array-like: it is very unlikely that implementations are going to re-implement the entire protocol for this function, so there's no benefit to testing all the various edge cases assuming those are covered in other tests already.
  • I wouldn't bother confirming that each type of primitive is rejected (both in the "argument" case and in the "non-number value in the iterable" case) - it is hard to imagine an implementation which rejects numeric strings and bigints but does not reject symbols, for example, and so there is little additional value in writing tests for symbols being rejected once you already have tests for numeric strings and bigints being rejected.
  • I wouldn't bother duplicating tests for array vs non-array iterables. Yes, it is conceivable that an implementation could have different paths for those. But it's also conceivable (and in fact likely) that an implementation could have different paths for an array which has only ever contained numbers and one which previously contained some other kind of value (but no longer does), and I certainly wouldn't duplicate tests for that case.

Cases you've missed:

  • Confirming that the iterator is closed (i.e., .return is called) when a non-number value is yielded. (It's implicitly covered in the "failure to conform to iterable protocol" section, but you don't have one for .return behaving correctly, which I think is the only thing actually worth testing.)
  • When the iterable yields an object, its valueOf and toString methods are not called.

@ptomato
Copy link
Contributor Author

ptomato commented Apr 15, 2024

  • is extensible / Function.prototype: I've found that many existing functions do actually test these! That's how they found their way onto my "boilerplate" list 😄 I've been toying with the idea of just automatically generating the "boilerplate" tests for all functions, that would take the burden off of proposal writers to write these.

  • iterator protocol: Agreed. Now that I've written this list out, though, seems like a candidate for automation as well... I'll note it down elsewhere for the future.

  • Each type of primitive: Sure, but it's also very little extra cost to write a loop over an array of [undefined, null, true, '25', 25n, Symbol('25'), {valueOf() {return 25;}}]

  • Array vs non-array iterables: Agreed. I'd still suggest testing an empty iterable distinct from an empty array though.

  • Missed cases: Added, thanks.

I thought of a few more missed cases as well:

  • The iterable is still exhausted after the NaN / ±∞ states have been reached
  • An array argument is iterated via its Symbol.iterator method, not by getting length

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

No branches or pull requests

2 participants