Add tests for Array.prototype.includes#643
Conversation
|
I missed tests for non-number values of length and out of range values as well. I'll add them. |
|
Alright; I'll hold off on reviewing until you're ready |
|
It's done. But I've got this change I didn't like to avoid breaking runtimes around 2**53. Without a fromIndex I got issues with V8, SM, es6draft, etc. |
| 8. Return false. | ||
| ---*/ | ||
|
|
||
| var sample = new Array(42).map(function() { return 7; }); |
There was a problem hiding this comment.
This complexity doesn't really add much value, and it's a little inconsistent with the way you seed arrays in other tests. Would you be satisfied with something like var sample = [7, 7, 7, 7];?
|
Just a few suggestions for this one Leo. We're almost done with arrays! |
|
One extra test I missed: values are not cached |
|
fixes are up including the missing test |
| assert.sameValue([].includes.call(obj, "ecma262"), true, "'ecma262' is true"); | ||
|
|
||
| obj = getCleanObj(); | ||
| assert.sameValue([].includes.call(obj, "cake"), false, "'cake' is false"); |
| } | ||
| }); | ||
| obj[1] = "ecma262"; | ||
| obj[2] = "tc39"; |
There was a problem hiding this comment.
As nice as the simplification is, this change invalidated the test. The final assertion in this file expects obj[2] to have the value "cake", but (after this change) it actually takes the value "tc39".
|
The newest test is now invalid (my in-line comment is above but rendering as collapsed for some reason). If we address that problem, this patch should be ready to land. |
|
oops. It looked as such an easy change I didn't test, I should never guess something is just a piece of cake. |
|
I was just checking this patch for coverage parity with gh-95 and found that this version is missing a test for the extension of This makes me wonder (a little late, now that gh-641 has landed) if it would be better to use gh-95 as a "base" and work from there. Probably you considered this prior to starting. Can you say a bit about why you preferred a fresh start? |
oops, I missed that.
yes, yes
I used it as a quick reference only, but I preferred using the other tests for typedArray methods (mostly indexOf) as the base for the tests on gh-641, and then I reused them for this one. I've done a coverage check in a piece of paper, as I've been doing to check coverage on other tests to find missing parts for the steps of Array#includes and quickly compared with what we have on gh-95, that's where I missed unscopables. |
|
the test for unscopables is up |
2 participants
No description provided.