Bug 32014

Summary:

Rewrite two tests with dumpAsText

Product:

WebKit

Reporter:

Shinichiro Hamaji <hamaji>

Component:

Tools / Tests

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, hamaji, hyatt, mitz, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

All

Attachments:

Description	Flags
Patch v1	none
Patch v2	none

Description Shinichiro Hamaji 2009-12-01 03:17:44 PST

I've marked some tests as skipped for Qt in Bug 23264. Two of them can be rewritten using dumpAsText.

fast/block/positioning/complex-percentage-height.html
fast/lists/item-not-in-list-line-wrapping.html

Comment 1 Shinichiro Hamaji 2009-12-01 03:18:51 PST

Created attachment 44064 [details]
Patch v1

Comment 2 WebKit Review Bot 2009-12-01 03:21:07 PST

style-queue ran check-webkit-style on attachment 44064 [details] without any errors.

Comment 3 Darin Adler 2009-12-01 11:29:20 PST

Comment on attachment 44064 [details]
Patch v1

The plain text tests cover a lot less than the render tree dump tests. It's more portable between platforms, and more focused, but losing the coverage is not so great. I am not certain enough about this to say review+ myself at the moment. Sorry; I won't stand in the way if someone else is more sure this is the right thing to do.

Comment 4 Shinichiro Hamaji 2009-12-01 22:23:37 PST

Created attachment 44128 [details]
Patch v2

Comment 5 WebKit Review Bot 2009-12-01 22:28:16 PST

style-queue ran check-webkit-style on attachment 44128 [details] without any errors.

Comment 6 Shinichiro Hamaji 2009-12-01 22:31:49 PST

> Patch v2

Fixing a silly mistake in a test case, this doesn't change the logic
and the result of the test case though.

-            var redHeight = gebi("warper").offsetHeight;
+            var blueHeight = gebi("warper").offsetHeight;
             var yellowHeight = gebi("abs_100_height").offsetHeight;
-            var blueHeight = gebi("inner_abs_100_height").offsetHeight;
+            var redHeight = gebi("inner_abs_100_height").offsetHeight;

Comment 7 Shinichiro Hamaji 2009-12-01 22:33:55 PST

CCing original authors of these tests.

Darin, I understand your concern. I confirmed that these tests fail if
I reverse-patch the following fixes. Of course, I know this fact
doesn't show I didn't lose any information from the original test,
though. Please let me know if there are some more information I can
tell. Or, if you feel you'd never be confident, please let me know and
I'll just update the expectations of these tests.

https://bugs.webkit.org/show_bug.cgi?id=13887
https://bugs.webkit.org/show_bug.cgi?id=12746

Comment 8 Darin Adler 2009-12-02 10:40:05 PST

Comment on attachment 44128 [details]
Patch v2

OK

Comment 9 Shinichiro Hamaji 2009-12-02 23:12:49 PST

Comment on attachment 44128 [details]
Patch v2

Let me check commit-queue can handle binary deletion in git patch.

Comment 10 WebKit Commit Bot 2009-12-02 23:27:43 PST

Comment on attachment 44128 [details]
Patch v2

Clearing flags on attachment: 44128

Committed r51626: <http://trac.webkit.org/changeset/51626>

Comment 11 WebKit Commit Bot 2009-12-02 23:27:48 PST

All reviewed patches have been landed.  Closing bug.

Comment 12 Shinichiro Hamaji 2009-12-02 23:53:45 PST

(In reply to comment #11)
> All reviewed patches have been landed.  Closing bug.

It seems working. Thanks Darin for your review.