Use testharness.js instead of js-test.js in LayoutTests/fast/canvas tests.

Use testharness.js instead of js-test.js in LayoutTests/fast/canvas tests.

This change replaces js-test.js assertions with testharness.js assertions.
The expectation files are removed as they are not needed anymore.
To simplify review and landing, each change corrects at most 10 tests.

BUG=639732
Review-Url: https://codereview.chromium.org/2695963004
Cr-Commit-Position: refs/heads/master@{#451832}
Committed: https://chromium.googlesource.com/chromium/src/+/08f777fab5f1d19fec21ee57be3a4ce845bef9ce

[zakerinasab, 2017-02-15 16:33:22]

Description was changed from

==========
Use testharness.js instead of js-test.js in LayoutTests/fast/canvas tests.

This change replaces js-test.js assertions with testharness.js assertions.
The expectation files are removed as they are not needed anymore.
To simplify review and landing, each change corrects at most 10 tests.

BUG=639732
==========

to

==========
Use testharness.js instead of js-test.js in LayoutTests/fast/canvas tests.

This change replaces js-test.js assertions with testharness.js assertions.
The expectation files are removed as they are not needed anymore.
To simplify review and landing, each change corrects at most 10 tests.

BUG=639732
==========

[zakerinasab, 2017-02-15 16:33:23]

zakerinasab@chromium.org changed reviewers:
+ junov@chromium.org

[zakerinasab, 2017-02-15 16:33:36]

New CL uploaded.

[Justin Novosad, 2017-02-15 19:55:28]

https://codereview.chromium.org/2695963004/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/fast/canvas/canvas-toBlob-case-insensitive-mimetype.html
(right):

https://codereview.chromium.org/2695963004/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/canvas/canvas-toBlob-case-insensitive-mimetype.html:12:
assert_true(blob.type === expectedMimeType);
calls to assert* need to be inside t.step()

https://codereview.chromium.org/2695963004/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/canvas/canvas-toBlob-case-insensitive-mimetype.html:18:
tryMimeType("image/PNG", "image/png");
I know you can't use generate_tests for async, but you should still refactor
this so that each mime type gets its own test.  Just move tryMimeType to global
scope, pass 't' as an argument.

https://codereview.chromium.org/2695963004/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/fast/canvas/canvas-toBlob-invalid.html
(right):

https://codereview.chromium.org/2695963004/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/canvas/canvas-toBlob-invalid.html:18:
assert_throws(null, function() {canvas.toBlob();});
assert_throws allows you to test for specific exception types.  We should do
that here, especially considering this test is likely to get upstreamed to WPT.

[zakerinasab, 2017-02-16 17:16:18]

Comments addressed.

https://codereview.chromium.org/2695963004/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/fast/canvas/canvas-toBlob-case-insensitive-mimetype.html
(right):

https://codereview.chromium.org/2695963004/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/canvas/canvas-toBlob-case-insensitive-mimetype.html:12:
assert_true(blob.type === expectedMimeType);
On 2017/02/15 19:55:28, Justin Novosad wrote:
> calls to assert* need to be inside t.step()

Done.

https://codereview.chromium.org/2695963004/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/canvas/canvas-toBlob-case-insensitive-mimetype.html:18:
tryMimeType("image/PNG", "image/png");
On 2017/02/15 19:55:28, Justin Novosad wrote:
> I know you can't use generate_tests for async, but you should still refactor
> this so that each mime type gets its own test.  Just move tryMimeType to
global
> scope, pass 't' as an argument.

Done.

https://codereview.chromium.org/2695963004/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/fast/canvas/canvas-toBlob-invalid.html
(right):

https://codereview.chromium.org/2695963004/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/canvas/canvas-toBlob-invalid.html:18:
assert_throws(null, function() {canvas.toBlob();});
On 2017/02/15 19:55:28, Justin Novosad wrote:
> assert_throws allows you to test for specific exception types.  We should do
> that here, especially considering this test is likely to get upstreamed to
WPT.

Done.

[Justin Novosad, 2017-02-16 22:32:42]

lgtm with nit

https://codereview.chromium.org/2695963004/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/fast/canvas/canvas-toBlob-invalid.html
(right):

https://codereview.chromium.org/2695963004/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/canvas/canvas-toBlob-invalid.html:18:
assert_throws(new TypeError(), function() {canvas.toBlob();}, "TypeError: Failed
to execute 'toBlob' on 'HTMLCanvasElement': 1 argument required, but only 0
present.");
Nit: better not to use the third argument of assert_throws. It makes the test
non-interoperable and we'll be forced to update the test every time the message
text is tweaked, which is annoying.

[zakerinasab, 2017-02-17 23:05:20]

On 2017/02/16 22:32:42, Justin Novosad wrote:
> lgtm with nit
> 
>
https://codereview.chromium.org/2695963004/diff/20001/third_party/WebKit/Layo...
> File third_party/WebKit/LayoutTests/fast/canvas/canvas-toBlob-invalid.html
> (right):
> 
>
https://codereview.chromium.org/2695963004/diff/20001/third_party/WebKit/Layo...
> third_party/WebKit/LayoutTests/fast/canvas/canvas-toBlob-invalid.html:18:
> assert_throws(new TypeError(), function() {canvas.toBlob();}, "TypeError:
Failed
> to execute 'toBlob' on 'HTMLCanvasElement': 1 argument required, but only 0
> present.");
> Nit: better not to use the third argument of assert_throws. It makes the test
> non-interoperable and we'll be forced to update the test every time the
message
> text is tweaked, which is annoying.

Done.

[Justin Novosad, 2017-02-21 18:09:28]

lgtm

[zakerinasab, 2017-02-21 18:25:13]

The CQ bit was checked by zakerinasab@chromium.org

[commit-bot: I haz the power, 2017-02-21 18:25:49]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-21 21:11:28]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1487701513995470,
"parent_rev": "80325c70ccf2bb0cc17cca4fe30f6eaae9e278dc", "commit_rev":
"08f777fab5f1d19fec21ee57be3a4ce845bef9ce"}

[commit-bot: I haz the power, 2017-02-21 21:12:31]

Description was changed from

==========
Use testharness.js instead of js-test.js in LayoutTests/fast/canvas tests.

This change replaces js-test.js assertions with testharness.js assertions.
The expectation files are removed as they are not needed anymore.
To simplify review and landing, each change corrects at most 10 tests.

BUG=639732
==========

to

==========
Use testharness.js instead of js-test.js in LayoutTests/fast/canvas tests.

This change replaces js-test.js assertions with testharness.js assertions.
The expectation files are removed as they are not needed anymore.
To simplify review and landing, each change corrects at most 10 tests.

BUG=639732

Review-Url: https://codereview.chromium.org/2695963004
Cr-Commit-Position: refs/heads/master@{#451832}
Committed:
https://chromium.googlesource.com/chromium/src/+/08f777fab5f1d19fec21ee57be3a...
==========

[commit-bot: I haz the power, 2017-02-21 21:12:32]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/08f777fab5f1d19fec21ee57be3a...