Collection of helper constants and functions for testing RABs #4030
Conversation
harness/resizableArrayBufferUtils.js
Outdated
| resized = true; | ||
| } | ||
| } | ||
| assert.compareArray(values.flat(), expected.flat()); |
There was a problem hiding this comment.
what about envs that don't support .flat?
There was a problem hiding this comment.
I addressed all the comments except this, as I am not sure what's the best way to proceed. One idea would be to define a new comparison function that doesn't need to use .flat here. Do you think there is a simpler way to take care of this?
There was a problem hiding this comment.
i guess that makes me wonder why they need to be flattened at all. what's the diff if you just compare them directly?
There was a problem hiding this comment.
compareArray doesn't do deep comparison, it does SameValue elementwise
There was a problem hiding this comment.
ahhh gotcha. so what we need here is zip(values, expected).forEach((v, e) => assert.compareArray(v, e)) or something :-/
probably best to do just a normal for loop then, here?
ioannad
force-pushed
the
harness-RAB-utils
branch
from
761e774
to
171ae22
Compare
harness/resizableArrayBufferUtils.js
Outdated
| let resized = false; | ||
| var arrayValues = false; | ||
|
|
||
| for (const value of ta) { |
There was a problem hiding this comment.
@ljharb I switched this back to a for..of loop because some of the tas are actually iterators. IIUC for..of was made exactly for this sort of usage, where it could be an array or an iterator being looped over. WDYT?
|
@ptomato @ljharb I think this is now ready for another round of reviews. I think everything is addressed except the name of the argument |
|
@ljharb just a friendly ping since my previous comment is now marked outdated: WDYT about leaving the for-of loop in |
…ray buffers. These are the parts of the code in RAB staging tests that are heavily repeated. In order to somewhat compact the migration of RAB staging tests (see PR tc39#3888).
Co-authored-by: Jordan Harband <[email protected]>
…6Array and BigInt
Co-authored-by: Philip Chimento <[email protected]>
Co-authored-by: Jordan Harband <[email protected]>
…and adjusted central assertion. - After some errors from `TestIterationAndResize` when trying to fix the comparison between `values` and `expected` without using .flat(), reverted this particular usage of for-of so that it easily works with iterators. - Also adjusted the central assertion to work for both cases of whether `values` is an array of number arrays or not.
ljharb
force-pushed
the
harness-RAB-utils
branch
from
3fb86d7
to
30b4ba9
Compare
|
I certainly won't block on it, it's just nice if tests don't rely on syntax that isn't necessary to run the test :-) |
|
OK, let's move forward with this. If there's an opportunity to get rid of the for-of by not using iterables elsewhere, I'd be happy to merge it in the future, but the RAB tests have been waiting for long enough 😄 |
These are the parts of the code in resizable array buffer staging tests that are heavily repeated. In order to somewhat compact the migration of RAB staging tests (see PR #3888).