Add tests for Array.prototype.sort stability #1977
Conversation
f9cc19a to
c408098
Compare
c408098 to
490292d
Compare
|
|
||
| const reduced = array.reduce((acc, element) => acc + element.name, ''); | ||
| const isProbablyStable = reduced === 'DGBEFHACIJK'; | ||
| assert(isProbablyStable); |
There was a problem hiding this comment.
can we use assert.sameValue(reduced, 'DGBEFHACIJK') instead? same for the other tests.
the current way is also correct but we could use sameValue here for convention, please.
There was a problem hiding this comment.
I did it this way intentionally, as it’s important to clarify that this is only a best-effort test. An engine could implement a completely randomized sorting algorithm, and still sometimes get lucky and pass the test. There is no way to write a single test that actually proves stability. The isProbablyStable name is important, IMHO.
I guess I’ll turn it into a comment.
0b902ec to
c97adc2
Compare
|
Feedback addressed. |
|
Just thought I should note that the stable sort I wrote for Chakracore switches algorithm (from insertion sort to merge sort) at length = 2048 - so this wouldn't fully test the stability of CC's sort. (Note the stable sort in question is still only in chakracore master, not yet in any release - I wrote it as an external contributor and do not know when it will be included in a release - it is enabled by default in master though) |
|
Thanks, @rhuanjl — I’ll add another test with exactly 2048 elements then. |
tc39/ecma262#1340