Bug 63632 - [CSS Regions] Add support for skipped tests
Summary: [CSS Regions] Add support for skipped tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 57312
  Show dependency treegraph
 
Reported: 2011-06-29 08:46 PDT by Mihnea Ovidenie
Modified: 2011-06-29 16:54 PDT (History)
3 users (show)

See Also:


Attachments
Patch (3.51 KB, patch)
2011-06-29 09:08 PDT, Mihnea Ovidenie
no flags Details | Formatted Diff | Diff
Add testcase to populate path to skip (5.05 KB, patch)
2011-06-29 15:40 PDT, Alan Stearns
tony: review-
Details | Formatted Diff | Diff
Changed testfile on feedback in Comment 6 (5.04 KB, patch)
2011-06-29 16:22 PDT, Alan Stearns
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.