Issue 2255213002: Use testharness.js to test move image to new document.

Issue 2255213002: Use testharness.js to test move image to new document. (Closed)

Created:
4 years, 4 months ago by sivag

Modified:
4 years, 4 months ago

Reviewers:
Srirama, fs

CC:
blink-reviews, chromium-reviews

Base URL:
https://chromium.googlesource.com/chromium/src.git@master

Target Ref:
refs/pending/heads/master

Project:
chromium

Visibility:
Public.

More Reviews

Description

Use testharness.js to test move image to new document. 1.Change template to use testharness.js instead of js-test.js. 2.Remove move-image-to-new-document.js and move js code to html file. 3.Change script it test to use testharness.js. BUG=637859 Committed: https://crrev.com/afe2f0fbde66f6a890d357f9a0243b3102b3bd1f Cr-Commit-Position: refs/heads/master@{#413111}

Patch Set 1 #

Patch Set 2 : Remove unnecessary append. #

Total comments: 8

Patch Set 3 : Code changed as per review comments. #

Total comments: 2

Patch Set 4 : Corrected indentation. #

Created: 4 years, 4 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+38 lines, -79 lines)			Patch
M	third_party/WebKit/LayoutTests/fast/images/move-image-to-new-document.html	View	1 2 3	1 chunk	+28 lines, -8 lines	0 comments	Download
D	third_party/WebKit/LayoutTests/fast/images/move-image-to-new-document-expected.txt	View		1 chunk	+0 lines, -13 lines	0 comments	Download
M	third_party/WebKit/LayoutTests/fast/images/script-tests/TEMPLATE.html	View	1 2	1 chunk	+10 lines, -9 lines	0 comments	Download
D	third_party/WebKit/LayoutTests/fast/images/script-tests/move-image-to-new-document.js	View		1 chunk	+0 lines, -49 lines	0 comments	Download

Messages

Total messages: 16 (8 generated)

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages

https://codereview.chromium.org/2255213002/diff/20001/third_party/WebKit/LayoutTests/fast/images/move-image-to-new-document.html File third_party/WebKit/LayoutTests/fast/images/move-image-to-new-document.html (right): https://codereview.chromium.org/2255213002/diff/20001/third_party/WebKit/LayoutTests/fast/images/move-image-to-new-document.html#newcode5 third_party/WebKit/LayoutTests/fast/images/move-image-to-new-document.html:5: <body> Don't need this (should be created by <iframe> ...

4 years, 4 months ago (2016-08-18 14:31:47 UTC) #6

sivag

fs, ptal.. https://codereview.chromium.org/2255213002/diff/20001/third_party/WebKit/LayoutTests/fast/images/move-image-to-new-document.html File third_party/WebKit/LayoutTests/fast/images/move-image-to-new-document.html (right): https://codereview.chromium.org/2255213002/diff/20001/third_party/WebKit/LayoutTests/fast/images/move-image-to-new-document.html#newcode5 third_party/WebKit/LayoutTests/fast/images/move-image-to-new-document.html:5: <body> On 2016/08/18 14:31:46, fs wrote: > ...

4 years, 4 months ago (2016-08-19 07:26:05 UTC) #7

fs, ptal..

https://codereview.chromium.org/2255213002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/fast/images/move-image-to-new-document.html
(right):

https://codereview.chromium.org/2255213002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/images/move-image-to-new-document.html:5:
<body>
On 2016/08/18 14:31:46, fs wrote:
> Don't need this (should be created by <iframe> I think.)

Done.

https://codereview.chromium.org/2255213002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/images/move-image-to-new-document.html:10:
testRunner.setPluginsEnabled(false);
On 2016/08/18 14:31:46, fs wrote:
> Move this out of the test body.

Done.

https://codereview.chromium.org/2255213002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/images/move-image-to-new-document.html:13:
var types = [ "img", "input", "object", "embed" ];
On 2016/08/18 14:31:46, fs wrote:
> It'd probably be easier to have one test per type, so something like:
> 
> for (var elementType of [ "img", ...])
>     async_test(..., document.title + ', ' + elementType);
> 
> (and one iframe per type/test etc.)
> 

Done.

https://codereview.chromium.org/2255213002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/fast/images/script-tests/TEMPLATE.html
(right):

https://codereview.chromium.org/2255213002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/images/script-tests/TEMPLATE.html:8: <script
src="YOUR_JS_FILE_HERE"></script>
On 2016/08/18 14:31:46, fs wrote:
> While we're modifying this file, maybe additionally:
> 
> * Remove <html>, <head> and <body>
> * Add <title>
> * Change the <script src="YOUR..."></script> to:
> 
>   <script>
>   test(function() {
>     YOUR_TEST_CODE_HERE
>   });
>   </script>
> 
>   (possibly test -> async_test; I guess the later could be more common in
these
> tests?)

Added a async_test template here.

lgtm https://codereview.chromium.org/2255213002/diff/40001/third_party/WebKit/LayoutTests/fast/images/move-image-to-new-document.html File third_party/WebKit/LayoutTests/fast/images/move-image-to-new-document.html (right): https://codereview.chromium.org/2255213002/diff/40001/third_party/WebKit/LayoutTests/fast/images/move-image-to-new-document.html#newcode14 third_party/WebKit/LayoutTests/fast/images/move-image-to-new-document.html:14: var element = document.createElement(elementType); Nit: Inconsistent indentation for ...

4 years, 4 months ago (2016-08-19 08:28:45 UTC) #8

sivag

https://codereview.chromium.org/2255213002/diff/40001/third_party/WebKit/LayoutTests/fast/images/move-image-to-new-document.html File third_party/WebKit/LayoutTests/fast/images/move-image-to-new-document.html (right): https://codereview.chromium.org/2255213002/diff/40001/third_party/WebKit/LayoutTests/fast/images/move-image-to-new-document.html#newcode14 third_party/WebKit/LayoutTests/fast/images/move-image-to-new-document.html:14: var element = document.createElement(elementType); On 2016/08/19 08:28:45, fs wrote: ...

4 years, 4 months ago (2016-08-19 09:10:05 UTC) #9

commit-bot: I haz the power

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

4 years, 4 months ago (2016-08-19 09:10:40 UTC) #12

commit-bot: I haz the power

4 years, 4 months ago (2016-08-19 12:07:15 UTC) #16

Message was sent while issue was closed.

Patchset 4 (id:??) landed as
https://crrev.com/afe2f0fbde66f6a890d357f9a0243b3102b3bd1f
Cr-Commit-Position: refs/heads/master@{#413111}

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages