Skip to content

[immutable-arraybuffer] Include immutable ArrayBuffers in TypedArray testing#4545

Open
gibson042 wants to merge 9 commits intotc39:mainfrom
gibson042:2025-07-generalize-testwithtypedarrayconstructors
Open

[immutable-arraybuffer] Include immutable ArrayBuffers in TypedArray testing#4545
gibson042 wants to merge 9 commits intotc39:mainfrom
gibson042:2025-07-generalize-testwithtypedarrayconstructors

Conversation

@gibson042
Copy link
Copy Markdown
Member

@gibson042 gibson042 commented Jul 23, 2025

Extends #4921 (diff)

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.

@gibson042 gibson042 requested a review from a team as a code owner July 23, 2025 05:27
@gibson042 gibson042 force-pushed the 2025-07-generalize-testwithtypedarrayconstructors branch 4 times, most recently from 4c418a7 to 9e12279 Compare July 23, 2025 15:55
@gibson042 gibson042 force-pushed the 2025-07-generalize-testwithtypedarrayconstructors branch 2 times, most recently from 23d89d1 to 3b45d8f Compare July 23, 2025 16:11
@Ms2ger Ms2ger force-pushed the 2025-07-generalize-testwithtypedarrayconstructors branch from 94c4b3d to 335a3d2 Compare July 29, 2025 09:13
Copy link
Copy Markdown
Contributor

@ptomato ptomato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@gibson042 gibson042 changed the title Generalize testWithTypedArrayConstructors to invoke its callback with a constructor and an argument factory [immutable-arraybuffer] Include immutable ArrayBuffers in TypedArray testing Feb 12, 2026
@gibson042 gibson042 force-pushed the 2025-07-generalize-testwithtypedarrayconstructors branch from 1bfe263 to 56e8cd3 Compare February 12, 2026 01:45
…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 gibson042 force-pushed the 2025-07-generalize-testwithtypedarrayconstructors branch from 56e8cd3 to a52e22a Compare February 22, 2026 20:31
@gibson042 gibson042 requested a review from ptomato February 22, 2026 20:33
Copy link
Copy Markdown
Contributor

@ptomato ptomato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I've looked this over again and all the issues seem to have been resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants