| Summary: | Convert residual-style.html test to a reftest (and fix typos) | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Christian Biesinger <cbiesinger> | ||||||||||||||
| Component: | Tools / Tests | Assignee: | Christian Biesinger <cbiesinger> | ||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||
| Severity: | Normal | CC: | gyuyoung.kim, ojan, pdr, rakuco, tony, toyoshim, webkit.review.bot | ||||||||||||||
| Priority: | P2 | ||||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||
| OS: | Unspecified | ||||||||||||||||
| Bug Depends on: | |||||||||||||||||
| Bug Blocks: | 109994 | ||||||||||||||||
| Attachments: |
|
||||||||||||||||
|
Description
Christian Biesinger
2013-02-15 16:00:04 PST
Created attachment 188661 [details]
Patch
Comment on attachment 188661 [details]
Patch
Can we convert this to a ref test instead of updating the pixel results?
Created attachment 188669 [details]
Patch
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. Created attachment 188686 [details]
Patch
The bot errors seem unrelated. 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. (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. Created attachment 189138 [details]
Patch
Created attachment 189139 [details]
Patch
Comment on attachment 189139 [details]
Patch
Thanks for making those changes. Off to the queue!
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 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 on attachment 189139 [details] Patch Clearing flags on attachment: 189139 Committed r143393: <http://trac.webkit.org/changeset/143393> All reviewed patches have been landed. Closing bug. Test fails in Chromium Mac 10.7 and 10.8. Scroll bar size looks different. I'll revert this change. Re-opened since this is blocked by bug 110302 Reverted: https://bugs.webkit.org/show_bug.cgi?id=110302 See also http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=@ToT - chromium.org&tests=fast/invalid/residual-style.html http://build.chromium.org/f/chromium/layout_test_results/WebKit_Mac10_7__dbg_/183340/layout-test-results.zip http://build.chromium.org/f/chromium/layout_test_results/WebKit_Mac10_7/183340/layout-test-results.zip http://build.chromium.org/f/chromium/layout_test_results/WebKit_Mac10_8/183340/layout-test-results.zip Reopening because it got reverted Created attachment 189427 [details]
Fix Mac test failure
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 on attachment 189427 [details] Fix Mac test failure Clearing flags on attachment: 189427 Committed r143548: <http://trac.webkit.org/changeset/143548> All reviewed patches have been landed. Closing bug. |