Generalize testWithTypedArrayConstructors to invoke its callback with a constructor and an argument factory
#4545
Conversation
gibson042
force-pushed
the
2025-07-generalize-testwithtypedarrayconstructors
branch
4 times, most recently
from
4c418a7 to
9e12279
Compare
gibson042
force-pushed
the
2025-07-generalize-testwithtypedarrayconstructors
branch
2 times, most recently
from
23d89d1 to
3b45d8f
Compare
Ms2ger
force-pushed
the
2025-07-generalize-testwithtypedarrayconstructors
branch
from
94c4b3d to
335a3d2
Compare
With `sedi` being a portable `sed -i`, e.g. `sed -i '' "$@"` on BSD and `sed -i "$@"` on Linux: ```sh git grep -l 'testWith[A-Za-z]*TypedArrayConstructors[(].*, N.*' test/built-ins/ | \ xargs sedi -E 's#(testWith[A-Za-z]*TypedArrayConstructors[(].*), N([^a-zA-Z].*)#\1\2#' ```
With `sedi` being a portable `sed -i`, e.g. `sed -i '' "$@"` on BSD and `sed -i "$@"` on Linux: ```sh git grep -l 'testBigIntTypedArray[.]js' | \ xargs sedi 's#testBigIntTypedArray[.]js#testTypedArray.js#' ```
… a TA constructor and an argument factory
…uctors
With `sedi` being a portable `sed -i`, e.g. `sed -i '' "$@"` on BSD and
`sed -i "$@"` on Linux:
```sh
git grep -l 'testWith[A-Za-z]*TypedArrayConstructors[(]' test/built-ins/ | \
while read f; do
grep -qF detachArrayBuffer.js "$f" && continue
grep -qF '.resize(' "$f" && continue
taCtorAliases="$(sed -nE \
's#testWith[A-Za-z]*TypedArrayConstructors[(] *(function[^(]*[(] *|[^a-zA-Z0-9_]*)([a-zA-Z0-9_]+).*#\2#p' \
"$f")"
sedi -E '
s#(testWith[A-Za-z]*TypedArrayConstructors[(] *(function[^(]*[(] *|[(] *)[a-zA-Z0-9_]+)#\1, makeCtorArg#; t;
s#(testWith[A-Za-z]*TypedArrayConstructors[(] *)([a-zA-Z0-9_]+)#\1(\2, makeCtorArg)#; t;
'"$(printf '%s' "$taCtorAliases" | sed -E 's/(.*)/s#\\b\1[(]([0-9.]+n?|[[][^]]*[]])[)]#\1(makeCtorArg(\\1))#/')" \
"$f"
done
git diff --numstat -- test/built-ins/ | \
awk '{ print $NF }' | \
xargs grep -c '\bmakeCtorArg\b' | \
sed -n 's/:1$//p' | \
xargs sedi -E '
/makeCtorArg/,$ { s#^[}][)]#}, null, ["passthrough"])#; }
/makeCtorArg/ { s#, makeCtorArg##; s#[(]([a-zA-Z0-9_]+)[)] =>#\1 =>#; }
'
git diff --numstat -- test/built-ins/ | \
awk '{ print $NF }' | \
xargs grep -l '\bdefineProperty\b' | \
xargs sedi -E \
'/testWith[A-Za-z]*TypedArrayConstructors[(]/,$s#^[}][)]#}, null, ["passthrough"])#'
```
…rray method tests
…e contents
With `sedi` being a portable `sed -i`, e.g. `sed -i '' "$@"` on BSD and
`sed -i "$@"` on Linux:
```sh
git grep -l 'testWith[A-Za-z]*TypedArrayConstructors[(]' \
test/built-ins/TypedArray/prototype/{copyWithin,fill,reverse,set,sort} | \
xargs sedi -E \
'/testWith[A-Za-z]*TypedArrayConstructors[(]/,$s#^[}][)]#}, null, null, ["immutable"])#'
git grep -l 'testWith[A-Za-z]*TypedArrayConstructors[(]' \
test/built-ins/TypedArray/prototype | \
grep -E 'set-value-during-|predicate-call-changes-value|values-are-not-cached' | \
xargs sedi -E \
'/testWith[A-Za-z]*TypedArrayConstructors[(]/,$s#^[}][)]#}, null, null, ["immutable"])#'
```
gibson042
force-pushed
the
2025-07-generalize-testwithtypedarrayconstructors
branch
from
335a3d2 to
1bfe263
Compare
There was a problem hiding this comment.
The new testTypedArrayConstructors helper could use some more explanation in
doc comments. For example, do I understand correctly that the overall purpose is
to expand the test matrix to not only test each piece of functionality with as
many TypedArray constructors, as possible, but also test with as many different
argument types as possible to each TypedArray constructor?
I don't understand the purpose of passing , null, ["passthrough"] in tests
that don't even use the factory argument. (It seems like some but not all
instances of that are removed in 53a9ecb?)
Particularly in 5855df7 (but also b7e4f20) I think a prose description of
what I'm looking at would be more helpful than the contents of the sed script,
although it's probably good to include the script for reference. I can read the
sed scripts eventually, but given limited time to spend on review that's not the
most effective place for me to spend it on.
It might be helpful to split this PR into two, where one is just a refactor of
the helper and increase of coverage in the existing tests without immutable ABs,
and the second adds the facilities for immutable ABs to the helper.
harness/testTypedArray.js
Outdated
| * @param {Array} constructors - An optional Array with filtered typed arrays | ||
| */ | ||
| function testWithBigIntTypedArrayConstructors(f, constructors) { | ||
| if (!constructors) constructors = [BigInt64Array, BigUint64Array]; |
There was a problem hiding this comment.
Re. the merge of testBigIntTypedArray.js into testTypedArray.js, fair enough! I guess they were separated originally so that implementations without BigInt could still run the rest of the TypedArray tests, but this seems like a good solution that preserves that capability, as there won't be a ReferenceError unless you actually call testWithBigIntTypedArrayConstructors.
| argFactory === makeResizableArrayBuffer || | ||
| argFactory === makeGrownArrayBuffer || | ||
| argFactory === makeShrunkArrayBuffer || | ||
| argFactory === makeImmutableArrayBuffer |
There was a problem hiding this comment.
Nitpick, makeImmutableArrayBuffer isn't yet available at this point in the commit stack.
| case "immutable": | ||
| return argFactory === makeImmutableArrayBuffer; |
| * @param {Function} Constructor the constructor object to test with. | ||
| * @param {Function} TypedArrayConstructor the constructor object to test with | ||
| * @param {Function} [TypedArrayConstructorArgFactory] a function for making | ||
| * a TypedArrayConstructor arguments from a primitive (usually a number) or |
There was a problem hiding this comment.
Typo: arguments → argument
| * @param {typedArrayConstructorCallback} f - the function to call for each typed array constructor. | ||
| * @param {Array} selected - An optional Array with filtered typed arrays | ||
| * @param {typedArrayConstructorCallback} f - the function to call | ||
| * @param {Array} [constructors] - an explict list of TypedArray constructors |
This is foundational for many remaining tests of #4509, particularly coverage of existing %TypedArray%.prototype properties.
Best reviewed commit-by-commit, because much of the work is mechanical text replacement.