Bug 109981 - Convert residual-style.html test to a reftest (and fix typos)
Summary: Convert residual-style.html test to a reftest (and fix typos)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Christian Biesinger
URL:
Keywords:
Depends on:
Blocks: 109994
  Show dependency treegraph
 
Reported: 2013-02-15 16:00 PST by Christian Biesinger
Modified: 2013-02-20 18:22 PST (History)
7 users (show)

See Also:


Attachments
Patch (95.61 KB, patch)
2013-02-15 16:07 PST, Christian Biesinger
no flags Details | Formatted Diff | Diff
Patch (895.38 KB, patch)
2013-02-15 17:00 PST, Christian Biesinger
no flags Details | Formatted Diff | Diff
Patch (895.36 KB, patch)
2013-02-15 19:02 PST, Christian Biesinger
no flags Details | Formatted Diff | Diff
Patch (900.47 KB, patch)
2013-02-19 12:11 PST, Christian Biesinger
no flags Details | Formatted Diff | Diff
Patch (900.47 KB, patch)
2013-02-19 12:13 PST, Christian Biesinger
no flags Details | Formatted Diff | Diff
Fix Mac test failure (900.75 KB, patch)
2013-02-20 17:37 PST, Christian Biesinger
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Christian Biesinger 2013-02-15 16:00:04 PST
Found typos in a few tests. Want to fix those, because I have another patch coming that touches the same files, and this simplifies things a bit.
Comment 1 Christian Biesinger 2013-02-15 16:07:18 PST
Created attachment 188661 [details]
Patch
Comment 2 Tony Chang 2013-02-15 16:25:45 PST
Comment on attachment 188661 [details]
Patch

Can we convert this to a ref test instead of updating the pixel results?
Comment 3 Christian Biesinger 2013-02-15 17:00:33 PST
Created attachment 188669 [details]
Patch
Comment 4 Tony Chang 2013-02-15 17:19:07 PST
Comment on attachment 188669 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=188669&action=review

LGTM.  I'm waiting on the bots before setting the cq flag.

> LayoutTests/fast/invalid/residual-style-expected.html:5
> +.fail { color: red }

This appears to be unused.
Comment 5 Christian Biesinger 2013-02-15 19:02:20 PST
Created attachment 188686 [details]
Patch
Comment 6 Christian Biesinger 2013-02-15 19:04:58 PST
The bot errors seem unrelated.
Comment 7 Darin Adler 2013-02-16 12:53:50 PST
Comment on attachment 188686 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=188686&action=review

> LayoutTests/fast/flexbox/box-orient-button.html:47
> -<button id="horizontal" clsas="box horizontal">
> +<button id="horizontal" class="box horizontal">

How can we land this change without updating test results?

> LayoutTests/fast/invalid/residual-style-expected.html:1
> +<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0//EN">

Why an old DOCTYPE? Why not an HTML5 DOCTYPE?

> LayoutTests/fast/invalid/residual-style-expected.html:5
> +.xfail { color: red }

Does "x" here stand for "expected"? If so, why abbreviate it to one letter? Mysterious for people dealing with this test later. Iā€™d name this "expected-failure" since these are expected failures.

> LayoutTests/fast/invalid/residual-style-expected.html:12
> +This test case illustrates some residual style tag examples.  Unless otherwise noted, the behavior should match Firefox.

Why the two spaces here after the period? These will collapse into one.

> LayoutTests/fast/invalid/residual-style-expected.html:18
> +<hr>
> +A: <font>All of this should be green</font>
> +

Why all the blank lines? I think this would be much easier to read if more of it fit on the screen.
Comment 8 Christian Biesinger 2013-02-19 12:03:32 PST
(In reply to comment #7)
> > LayoutTests/fast/flexbox/box-orient-button.html:47
> > -<button id="horizontal" clsas="box horizontal">
> > +<button id="horizontal" class="box horizontal">
> 
> How can we land this change without updating test results?

There are no expected pixel results, and it dumps as text, not as a render tree (various PASS messages). So there's no change to -expected.txt.


I'll make the remaining changes.
Comment 9 Christian Biesinger 2013-02-19 12:11:42 PST
Created attachment 189138 [details]
Patch
Comment 10 Christian Biesinger 2013-02-19 12:13:57 PST
Created attachment 189139 [details]
Patch
Comment 11 Philip Rogers 2013-02-19 12:18:08 PST
Comment on attachment 189139 [details]
Patch

Thanks for making those changes. Off to the queue!
Comment 12 WebKit Review Bot 2013-02-19 13:57:59 PST
Comment on attachment 189139 [details]
Patch

Rejecting attachment 189139 [details] from commit-queue.

New failing tests:
platform/chromium/fast/forms/calendar-picker/week-picker-appearance-step.html
Full output: http://queues.webkit.org/results/16616870
Comment 13 Philip Rogers 2013-02-19 14:01:50 PST
Comment on attachment 189139 [details]
Patch

The commit queue failure was unrelated. Lets try the queue again and if it fails I'll manually commit.
Comment 14 WebKit Review Bot 2013-02-19 15:22:28 PST
Comment on attachment 189139 [details]
Patch

Clearing flags on attachment: 189139

Committed r143393: <http://trac.webkit.org/changeset/143393>
Comment 15 WebKit Review Bot 2013-02-19 15:22:33 PST
All reviewed patches have been landed.  Closing bug.
Comment 16 Takashi Toyoshima 2013-02-19 21:51:02 PST
Test fails in Chromium Mac 10.7 and 10.8.
Scroll bar size looks different.
I'll revert this change.
Comment 17 WebKit Review Bot 2013-02-19 21:54:43 PST
Re-opened since this is blocked by bug 110302
Comment 19 Christian Biesinger 2013-02-20 16:55:07 PST
Reopening because it got reverted
Comment 20 Christian Biesinger 2013-02-20 17:37:39 PST
Created attachment 189427 [details]
Fix Mac test failure
Comment 21 Christian Biesinger 2013-02-20 17:38:51 PST
I missed a few tags that do change layout, which I now added back to the -expected file. The test now passes for me on chromium-mac, and the full test now looks identical.

Please review again :)
Comment 22 WebKit Review Bot 2013-02-20 18:21:59 PST
Comment on attachment 189427 [details]
Fix Mac test failure

Clearing flags on attachment: 189427

Committed r143548: <http://trac.webkit.org/changeset/143548>
Comment 23 WebKit Review Bot 2013-02-20 18:22:04 PST
All reviewed patches have been landed.  Closing bug.