Conversation
db1258d to
3d1f99f
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
82b5e97 to
1f61034
Compare
| assert.compareArray(arrays, expectedArrays, 'arrays (grow)'); | ||
| assert.sameValue(result, true, 'result (grow)'); | ||
| }); | ||
| }, null, ["passthrough"]); |
There was a problem hiding this comment.
I don't understand the purpose of passing
, null, ["passthrough"]in tests that don't even use the factory argument.
@ptomato Including only the "passthrough" argument factory preserves a single callback invocation per TypedArray constructor rather than invoking the callback with a variety of argument factories per TypedArray constructor, which would be pointless (albeit harmless) for tests like this that either don't call the constructor or necessarily call it with a specific kind of input (here, with a resizable ArrayBuffer).
There was a problem hiding this comment.
Particularly in < ce2f932 > 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.
That commit relates to the above need; adding , null, ["passthrough"] arguments for testWithTypedArrayConstructors calls that don't invoke the TypedArray constructor with an input that can pass through an argument factory (while updating others to use that argument for e.g. replacing new TA([]) with new TA(makeCtorArg([])) and new TA(42) with new TA(makeCtorArg(42))). I wouldn't spend too much time reviewing that full commit as opposed to e.g. confirming that any tests broken by it are fixed in a subsequent commit.
Correct. That's hopefully now clear from the doc comments. |
ptomato
left a comment
There was a problem hiding this comment.
@gibson042 Thanks for the extra effort to make this easier to understand. Everything looks good now to me.
Before merging I wanted to flag these CI results. If you've checked these and they're legit, feel free to merge.
Note, I've only copied the sloppy mode results here, but the strict mode results are identical.
The following tests previously passed on V8 and now fail:
FAIL test/built-ins/TypedArray/prototype/subarray/speciesctor-get-species-custom-ctor-invocation.js (default)
Actual [[object ArrayBuffer], 8, undefined] and expected [[object ArrayBuffer], 8] should have the same contents. Constructor called with arguments (Testing with Float64Array and makeResizableArrayBuffer.)
FAIL test/built-ins/TypedArray/prototype/subarray/BigInt/speciesctor-get-species-custom-ctor-invocation.js (default)
Actual [[object ArrayBuffer], 8, undefined] and expected [[object ArrayBuffer], 8] should have the same contents. Constructor called with arguments (Testing with BigInt64Array and makeResizableArrayBuffer.)
(Might be an actual V8 bug that was previously hidden? But I can't tell without looking closer.)
The following tests previously failed on JSC and now pass, so the harness might be swallowing an error:
FAIL test/built-ins/TypedArrayConstructors/internals/Set/key-is-in-bounds-receiver-is-not-typed-array.js (default)
valueOf is not called Expected SameValue(«1», «0») to be true
FAIL test/built-ins/TypedArrayConstructors/internals/Set/key-is-out-of-bounds-receiver-is-not-object.js (default)
valueOf is not called Expected SameValue(«1», «0») to be true
FAIL test/built-ins/TypedArrayConstructors/internals/Set/key-is-out-of-bounds-receiver-is-not-typed-array.js (default)
valueOf is not called Expected SameValue(«1», «0») to be true
FAIL test/built-ins/TypedArrayConstructors/internals/Set/key-is-out-of-bounds-receiver-is-proto.js (default)
[[Set]] succeeeds
The first three of these look suspiciously like the new harness code might be accidentally triggering a hidden valueOf call? But again, can't tell without looking closer.
Before this PR, those tests were limited to running against $ eshost -si /tmp/auto-length-ta-subarray-custom-ctor-args.js
## Source
const typedArrayCtors = [
...[Int32Array, Int16Array, Int8Array],
...[Uint32Array, Uint16Array, Uint8Array],
Uint8ClampedArray,
...[Float64Array, Float32Array, globalThis.Float16Array],
...[globalThis.BigInt64Array, globalThis.BigUint64Array],
].filter(x => x);
const groups = {};
for (const TA of typedArrayCtors) {
const ta = new TA(new ArrayBuffer(32, { maxByteLength: 64 }));
const speciesCtor = function(...args) {
const argTypes = args.map(x => typeof x);
(groups[argTypes] ||= []).push(TA);
return new TA(...args);
};
ta.constructor = { [Symbol.species]: speciesCtor };
ta.subarray(1);
}
print(Object.keys(groups).join("\n"));
#### GraalJS, V8
object,number,undefined
#### JavaScriptCore, Moddable XS, SpiderMonkey
object,number
Hmm, none of those are changed by this PR, and I see a fresh esvu download of JSC still failing them all anyway: $ npx test262-harness -t 8 --hostType=jsc --hostPath="$HOME/.esvu/bin/jsc" test/built-ins/TypedArrayConstructors/internals/Set/key-is-*bounds*
FAIL test/built-ins/TypedArrayConstructors/internals/Set/key-is-in-bounds-receiver-is-not-typed-array.js (default)
valueOf is not called Expected SameValue(«1», «0») to be true
FAIL test/built-ins/TypedArrayConstructors/internals/Set/key-is-in-bounds-receiver-is-not-typed-array.js (strict mode)
valueOf is not called Expected SameValue(«1», «0») to be true
FAIL test/built-ins/TypedArrayConstructors/internals/Set/key-is-out-of-bounds-receiver-is-not-object.js (default)
valueOf is not called Expected SameValue(«1», «0») to be true
FAIL test/built-ins/TypedArrayConstructors/internals/Set/key-is-out-of-bounds-receiver-is-not-object.js (strict mode)
valueOf is not called Expected SameValue(«1», «0») to be true
FAIL test/built-ins/TypedArrayConstructors/internals/Set/key-is-out-of-bounds-receiver-is-not-typed-array.js (default)
valueOf is not called Expected SameValue(«1», «0») to be true
FAIL test/built-ins/TypedArrayConstructors/internals/Set/key-is-out-of-bounds-receiver-is-proto.js (strict mode)
[[Set]] succeeeds
FAIL test/built-ins/TypedArrayConstructors/internals/Set/key-is-out-of-bounds-receiver-is-proto.js (default)
[[Set]] succeeeds
FAIL test/built-ins/TypedArrayConstructors/internals/Set/key-is-out-of-bounds-receiver-is-not-typed-array.js (strict mode)
valueOf is not called Expected SameValue(«1», «0») to be true
Ran 10 tests
2 passed
8 failedI also inspected So I'm satisfied! Thanks for the thorough review; I'll merge this and start my rebasing with #4545. |
Split from #4545 as requested at #4545 (review) . This PR is just refactoring and increased coverage that does not include extension to immutable ArrayBuffer functionality.
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.