Bug 32014

Summary: Rewrite two tests with dumpAsText
Product: WebKit Reporter: Shinichiro Hamaji <hamaji>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Severity: Normal CC: commit-queue, hamaji, hyatt, mitz, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Description Flags
Patch v1
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.

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.

Comment 8 Darin Adler 2009-12-02 10:40:05 PST
Comment on attachment 44128 [details]
Patch v2

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.