Summary: | [CSS Regions] Add support for skipped tests | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mihnea Ovidenie <mihnea> | ||||||||
Component: | CSS | Assignee: | 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
Mihnea Ovidenie
2011-06-29 08:46:20 PDT
Created attachment 99096 [details]
Patch
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.
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. (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. Created attachment 99163 [details]
Add testcase to populate path to skip
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. Created attachment 99175 [details] Changed testfile on feedback in Comment 6 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. Of course, this test can be in changed in the patch that lands the code. 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 on attachment 99175 [details] Changed testfile on feedback in Comment 6 Clearing flags on attachment: 99175 Committed r90067: <http://trac.webkit.org/changeset/90067> All reviewed patches have been landed. Closing bug. |