Bug 81936 – Integrate IETC CSS : textshadow tests

• Component: CSS
• Status: RESOLVED FIXED

[Dave Tharp]

Integrate the 10 textshadow tests.  4 of these are failing, and will have bugs written against them.

This is the first batch of IETC tests, so we will be uploading the associated support directories for all the IETC CSS tests as well.

[Dave Tharp]

Created attachment 133298 [details]
Patch

[WebKit Review Bot]

Comment on attachment 133298 [details]
Patch

Attachment 133298 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12121113

[WebKit Review Bot]

Comment on attachment 133298 [details]
Patch

Rejecting attachment 133298 [details] from commit-queue.

New failing tests:
ietestcenter/css3/text/textshadow-004.htm
ietestcenter/css3/text/textshadow-006.htm
ietestcenter/css3/text/textshadow-007.htm
ietestcenter/css3/text/textshadow-009.htm
ietestcenter/css3/text/textshadow-005.htm
ietestcenter/css3/text/textshadow-008.htm
ietestcenter/css3/text/textshadow-001.htm
ietestcenter/css3/text/textshadow-002.htm
ietestcenter/css3/text/textshadow-003.htm
ietestcenter/css3/text/textshadow-010.htm
Full output: http://queues.webkit.org/results/12044051

[Adam Barth]

You can't use the commit-queue to land patches that have test failures.  We'll need to land this manually.  It would be nice if you'd notify the Chromium WebKit gardener that you're going to landing a patch that will cause tests to go red.

[Dave Tharp]

(In reply to comment #4)
> You can't use the commit-queue to land patches that have test failures.  We'll need to land this manually.  It would be nice if you'd notify the Chromium WebKit gardener that you're going to landing a patch that will cause tests to go red.

These tests weren't supposed to fail.  They pass on QT.  The "failing" tests have -expected.txt and .pngs that derive from the failed behavior... in other words they should all pass.  I will build chromium in the morning and see what's going on.

[Adam Barth]

Almost all pixel tests have different PNGs on different ports.

[Dave Tharp]

(In reply to comment #6)
> Almost all pixel tests have different PNGs on different ports.

I understand that pixel tests are troublesome, I would have liked to have avoided them here.  So I need to generate port specific pngs and put them in the platform directories then?

[Adam Barth]

> So I need to generate port specific pngs and put them in the platform directories then?

Ideally, yes, but that's probably going to be tough without access to al the different platforms.  In practice, what folks do is they add the appropriate lines to test_expectations.txt when landing their change.  Then, once all the bots have cycled, you can use webkit-patch garden-o-matic to download the PNGs from the bots to your working copy, and then you can commit them.

[Dave Tharp]

(In reply to comment #8)
> > So I need to generate port specific pngs and put them in the platform directories then?
> 
> Ideally, yes, but that's probably going to be tough without access to al the different platforms.  In practice, what folks do is they add the appropriate lines to test_expectations.txt when landing their change.  Then, once all the bots have cycled, you can use webkit-patch garden-o-matic to download the PNGs from the bots to your working copy, and then you can commit them.

Ah, that makes sense.  Thanks for the pointer!

[Tom Zakrajsek]

Committed r111910: <http://trac.webkit.org/changeset/111910>

[Dave Tharp]

Reopening to attach new patch.

[Dave Tharp]

Created attachment 134374 [details]
Patch

[WebKit Review Bot]

Attachment 134374 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/iete..." exit_code: 1
LayoutTests/platform/chromium-win/ietestcenter/css3/text/textshadow-005-expected.png:0:  Have to enable auto props in the subversion config file (/home/ubuntu/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/ubuntu/.subversion/config "*.png = svn:mime-type=image/png").  [image/png] [5]
Total errors found: 1 in 22 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Eric Seidel (no email)]

Yeah, I'm not sure if that's a bot problem or a script problem.  We certainly can change the bots.  They use git checkouts.

[Tony Chang]

(In reply to comment #13)
> Attachment 134374 [details] did not pass style-queue:
> 
> Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/iete..." exit_code: 1
> LayoutTests/platform/chromium-win/ietestcenter/css3/text/textshadow-005-expected.png:0:  Have to enable auto props in the subversion config file (/home/ubuntu/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/ubuntu/.subversion/config "*.png = svn:mime-type=image/png").  [image/png] [5]
> Total errors found: 1 in 22 files
> 
> 
> If any of these errors are false positives, please file a bug against check-webkit-style.

Heh, this is due to https://bugs.webkit.org/show_bug.cgi?id=75824, which checks that the system that runs check-webkit-style has the right subversion config file.  The style bot doesn't, so it's complaining.

We probably shouldn't run this check on the style bot. What's the right way to disable this check only on the style bot?

[Tony Chang]

(In reply to comment #14)
> Yeah, I'm not sure if that's a bot problem or a script problem.  We certainly can change the bots.  They use git checkouts.

I guess you could also fix the bots, but it's not a very useful check if the bot doesn't land the patch.

[Eric Seidel (no email)]

I've installed Chromium's recommended svn config (http://src.chromium.org/viewvc/chrome/trunk/tools/build/slave/config) on the style-bot.

[Tom Zakrajsek]

Note to reviewer: determined that there were 4 distinct expected image sets required because of font differences.

All Chromium Linux
All Chromium Windows
Chromium Mac Lion and Chromium Mac Leopard
Chromium Mac Snow Leopard

"Expected" placement looks right to me.

[WebKit Review Bot]

Comment on attachment 134374 [details]
Patch

Attachment 134374 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12100043

New failing tests:
ietestcenter/css3/text/textshadow-001.htm

[WebKit Review Bot]

Created attachment 134408 [details]
Archive of layout-test-results from ec2-cr-linux-03

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-03  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[Dave Tharp]

Created attachment 134427 [details]
Patch

[Adam Barth]

Comment on attachment 134427 [details]
Patch

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

> LayoutTests/ietestcenter/css3/text/textshadow-001.htm:48
> +            if (window.layoutTestController) {
> +                window.layoutTestController.dumpAsText(true);
> +            }

Why add dumpAsText(true)?  Are the render tree dumps not useful?

[Dave Tharp]

Created attachment 134679 [details]
Patch

[WebKit Review Bot]

Comment on attachment 134679 [details]
Patch

Clearing flags on attachment: 134679

Committed r112716: <http://trac.webkit.org/changeset/112716>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Dave Tharp]

Reopening to attach new patch.

[Dave Tharp]

Created attachment 136492 [details]
Patch

[Dave Tharp]

(In reply to comment #27)
> Created an attachment (id=136492) [details]
> Patch

Patch is for adding -expected.txt for non-chromium qt, gtk, and mac

[Tom Zakrajsek]

Comment on attachment 136492 [details]
Patch

Committed r113882: <http://trac.webkit.org/changeset/113882>

[Dave Tharp]

Comment on attachment 136492 [details]
Patch

tomz landed this gardening patch