Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TextEncoder/TextDecoder SharedArrayBuffer tests #19531

Merged

Conversation

@Bnaya
Copy link
Contributor

Bnaya commented Oct 4, 2019

Add tests for TextEncoder/TextEncoder with SharedArrayBuffer

@wpt-pr-bot wpt-pr-bot added the encoding label Oct 4, 2019
@wpt-pr-bot wpt-pr-bot requested review from annevk and inexorabletash Oct 4, 2019
@Bnaya Bnaya force-pushed the Bnaya:shared-array-buffer-text-encode-decode branch from f388330 to f29981a Oct 4, 2019
@Bnaya Bnaya changed the title TextEncoder/TextEncoder SharedArrayBuffer tests TextEncoder/TextDecoder SharedArrayBuffer tests Oct 4, 2019
encoding/encodeInto.any.js Outdated Show resolved Hide resolved
encoding/encodeInto.any.js Outdated Show resolved Hide resolved
encoding/encodeInto.any.js Outdated Show resolved Hide resolved
encoding/streams/decode-utf8.any.js Outdated Show resolved Hide resolved
@Bnaya Bnaya force-pushed the Bnaya:shared-array-buffer-text-encode-decode branch 3 times, most recently from 37961aa to 1df43ba Oct 7, 2019
Copy link
Member

annevk left a comment

@hsivonen do you have any other tests you'd like to see added/modified? (Can also be done incrementally in a new PR perhaps.)

encoding/encodeInto.any.js Outdated Show resolved Hide resolved
encoding/encodeInto.any.js Outdated Show resolved Hide resolved
@Bnaya Bnaya force-pushed the Bnaya:shared-array-buffer-text-encode-decode branch from 1df43ba to 5b40312 Oct 7, 2019
encoding/encodeInto.any.js Outdated Show resolved Hide resolved
@hsivonen

This comment has been minimized.

Copy link
Contributor

hsivonen commented Oct 8, 2019

do you have any other tests you'd like to see added/modified? (Can also be done incrementally in a new PR perhaps.)

I think it's enough to have SharedArrayBuffer testing with some contents. At least the initial implementations will likely be indifferent to actual content.

If implementations ever want to do fancy optimizations, it would be good to test things like a stretch of more than 16 ASCII bytes followed by UTF-8 sequences with a variety of lead bytes followed by buffer end, but the kind of failure modes triggered would be very timing-dependent anyway and not really testable.

In summary, I think there's no need to add anything specific regarding buffer content at this point.

@Bnaya Bnaya force-pushed the Bnaya:shared-array-buffer-text-encode-decode branch from c506534 to e033767 Oct 8, 2019
@annevk
annevk approved these changes Oct 9, 2019
Copy link
Member

annevk left a comment

This looks good to me, module some style nits I may or may not care enough to fix myself. @ricea any feedback?

@ricea

This comment has been minimized.

Copy link
Contributor

ricea commented Oct 9, 2019

This looks good to me, module some style nits I may or may not care enough to fix myself. @ricea any feedback?

Looks good.

@annevk annevk merged commit a910ad1 into web-platform-tests:master Oct 9, 2019
8 of 10 checks passed
8 of 10 checks passed
wpt.fyi - firefox[experimental] Firefox results
Details
wpt.fyi - safari[experimental] Safari results
Details
Azure Pipelines Build #20191009.29 succeeded
Details
Azure Pipelines (./wpt test-jobs) ./wpt test-jobs succeeded
Details
Azure Pipelines (affected tests without changes: Safari Technology Preview) affected tests without changes: Safari Technology Preview succeeded
Details
Azure Pipelines (affected tests: Safari Technology Preview) affected tests: Safari Technology Preview succeeded
Details
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests) wpt.fyi hook: safari-preview-affected-tests succeeded
Details
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests-without-changes) wpt.fyi hook: safari-preview-affected-tests-without-changes succeeded
Details
Taskcluster (pull_request) TaskGroup: success
Details
wpt.fyi - chrome[experimental] Chrome results
Details
annevk added a commit to whatwg/encoding that referenced this pull request Oct 9, 2019
@Bnaya Bnaya deleted the Bnaya:shared-array-buffer-text-encode-decode branch Oct 9, 2019
@Bnaya

This comment has been minimized.

Copy link
Contributor Author

Bnaya commented Oct 9, 2019

If the repo had prettier, editor config, eslint or so,
That would have made contributing abit more friendly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.