Bug 63632

Summary: [CSS Regions] Add support for skipped tests
Product: WebKit Reporter: Mihnea Ovidenie <mihnea>
Component: CSSAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: stearns, tony, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 57312    
Attachments:
Description Flags
Patch
none
Add testcase to populate path to skip
tony: review-
Changed testfile on feedback in Comment 6 none

Description Mihnea Ovidenie 2011-06-29 08:46:20 PDT
The LayoutTests for CSSRegions will be placed in LayoutTests/fast/regions. For now, these tests will be skipped as the CSSRegions support is not enabled by default. 

I will modify the following files to avoid those tests being run.  
LayoutTests/platform/gtk/Skipped
LayoutTests/platform/mac/Skipped
LayoutTests/platform/qt/Skipped
LayoutTests/platform/win/Skipped
LayoutTests/platform/chromium/test_expectations.txt
Comment 1 Mihnea Ovidenie 2011-06-29 09:08:16 PDT
Created attachment 99096 [details]
Patch
Comment 2 WebKit Review Bot 2011-06-29 09:10:43 PDT
Attachment 99096 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1

LayoutTests/platform/chromium/test_expectations.txt:3934:  Path does not exist. fast/regions  [test/expectations] [2]
Total errors found: 1 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Mihnea Ovidenie 2011-06-29 09:21:30 PDT
The path LayoutTests/fast/regions does not exits for the moment. I wanted to add this patch so that i will further minimize the chance to get merge conflict for further CSSRegions patches.
Comment 4 Tony Chang 2011-06-29 11:43:32 PDT
(In reply to comment #3)
> The path LayoutTests/fast/regions does not exits for the moment. I wanted to add this patch so that i will further minimize the chance to get merge conflict for further CSSRegions patches.

Can you just add in the patch where you add the new test?  One way to avoid merge conflicts is to not put the new lines at the bottom of the Skipped/test_expectations.txt files.
Comment 5 Alan Stearns 2011-06-29 15:40:44 PDT
Created attachment 99163 [details]
Add testcase to populate path to skip
Comment 6 Tony Chang 2011-06-29 16:09:08 PDT
Comment on attachment 99163 [details]
Add testcase to populate path to skip

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

Some minor nits to the test file.

> LayoutTests/fast/regions/flow-content-basic.html:1
> +<head>

<!DOCTYPE HTML>

> LayoutTests/fast/regions/flow-content-basic.html:6
> +    <!-- this test checks that content can be redirected to a region. It also
> +            tries out a validation strategy that recreates the intended display
> +            using non-region markup. If top does not match the bottom then the 
> +            result should be rejected -->

This indenting looks weird, maybe some tabs snuck in?  Also 'this test' -> 'This test' and end the last sentence with a period.

> LayoutTests/fast/regions/flow-content-basic.html:26
> +<body onload="runTests();">

This onload doesn't look used.
Comment 7 Alan Stearns 2011-06-29 16:22:44 PDT
Created attachment 99175 [details]
Changed testfile on feedback in Comment 6
Comment 8 Tony Chang 2011-06-29 16:43:54 PDT
BTW, you may want to come up with ways to make the tests non-pixel tests (i.e., using layoutTestController.dumpAsText() or dump-as-markup.js).  This allows us to have a single cross-platform result.
Comment 9 Tony Chang 2011-06-29 16:44:27 PDT
Of course, this test can be in changed in the patch that lands the code.
Comment 10 Alan Stearns 2011-06-29 16:50:29 PDT
My plan is to mostly rely on DumpRenderTree, since the result that needs to be checked for regions and exclusions is what gets rendered. It would be especially cool if I could get the RenderTree result from the layoutTestController and perform my validation steps in javascript, but I haven't found a way of doing that.
Comment 11 WebKit Review Bot 2011-06-29 16:54:49 PDT
Comment on attachment 99175 [details]
Changed testfile on feedback in Comment 6

Clearing flags on attachment: 99175

Committed r90067: <http://trac.webkit.org/changeset/90067>
Comment 12 WebKit Review Bot 2011-06-29 16:54:54 PDT
All reviewed patches have been landed.  Closing bug.