| Summary: | [Mac Lion] [WK2] tiled-drawing tests are being run when they shouldn’t be | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Ryosuke Niwa <rniwa> | ||||
| Component: | Layout and Rendering | Assignee: | Nobody <webkit-unassigned> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | bdakin, dpranke, eric.carlson, ggaren, jberlin, ojan, simon.fraser, thorton, webkit-bug-importer | ||||
| Priority: | P1 | Keywords: | InRadar, LayoutTestFailure | ||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
Ryosuke Niwa
2013-01-05 16:02:04 PST
Added test expectations in http://trac.webkit.org/changeset/138912. When did this start? Can this be the same as bug 106205? tiled drawing tests should be skipped on Lion. This used to be impossible because of TestExpectations order, but that was fixed. Bug 105583 fixed the TestExpectations fallback order. These tests should not be skipped for WK2 on all platforms; that's just disabling testing. (In reply to comment #6) > These tests should not be skipped for WK2 on all platforms; that's just disabling testing. Oh oops :/ Somehow I forgot to add [ Lion ] there. Fixing that now and skipping them instead. This should be fixed in http://trac.webkit.org/changeset/141323 That didn't remove these lines from platform/mac-wk2/TestExpectations: webkit.org/b/106187 platform/mac/tiled-drawing/fixed/absolute-inside-out-of-view-fixed.html [ Failure ] webkit.org/b/106187 platform/mac/tiled-drawing/fixed/four-bars-zoomed.html [ Failure ] webkit.org/b/106187 platform/mac/tiled-drawing/fixed/negative-scroll-offset.html [ Failure ] webkit.org/b/106187 platform/mac/tiled-drawing/sticky/sticky-horizontal.html [ Failure ] webkit.org/b/106187 platform/mac/tiled-drawing/sticky/sticky-vertical.html [ Failure ] (In reply to comment #9) > That didn't remove these lines from platform/mac-wk2/TestExpectations: > > webkit.org/b/106187 platform/mac/tiled-drawing/fixed/absolute-inside-out-of-view-fixed.html [ Failure ] > webkit.org/b/106187 platform/mac/tiled-drawing/fixed/four-bars-zoomed.html [ Failure ] > webkit.org/b/106187 platform/mac/tiled-drawing/fixed/negative-scroll-offset.html [ Failure ] > webkit.org/b/106187 platform/mac/tiled-drawing/sticky/sticky-horizontal.html [ Failure ] > webkit.org/b/106187 platform/mac/tiled-drawing/sticky/sticky-vertical.html [ Failure ] Removed those lines in http://trac.webkit.org/changeset/141329 Apparently platform/mac/tiled-drawing/ [ Pass ] is considered more specific than [ Lion ] platform/mac/tiled-drawing/ [ Skip ] ? http://build.webkit.org/builders/Apple%20Lion%20Release%20WK2%20%28Tests%29/builds/7471/steps/layout-test/logs/stdio 15:30:00.382 68230 LayoutTests/platform/mac-wk2/TestExpectations:413 More specific entry for platform/mac/tiled-drawing/ on line LayoutTests/platform/mac-wk2/TestExpectations:412 overrides line LayoutTests/platform/mac-wk2/TestExpectations:413. platform/mac/tiled-drawing/ 15:30:00.382 68230 LayoutTests/platform/mac-wk2/TestExpectations:413 More specific entry for platform/mac/tiled-drawing/ on line LayoutTests/platform/mac-wk2/TestExpectations:412 overrides line LayoutTests/platform/mac-wk2/TestExpectations:413. platform/mac/tiled-drawing/ So even that fix is not working here. Looking at test_expectations.py:
# At this point we know we have seen a previous exact match on this
# base path, so we need to check the two sets of modifiers.
# FIXME: This code was originally designed to allow lines that matched
# more modifiers to override lines that matched fewer modifiers.
# However, we currently view these as errors.
#
# To use the "more modifiers wins" policy, change the errors for overrides
# to be warnings and return False".
Looking ... I think perhaps this is a buggy error message; at the very least, line 413 is more specific than line 412. I'm also not sure why you're getting that error so many times. (In reply to comment #13) > Looking ... I think perhaps this is a buggy error message; at the very least, line 413 is more specific than line 412. I'm also not sure why you're getting that error so many times. I think I have a fix, I am just testing it. Yup, buggy error message (the two line numbers are swapped). At any rate, you can't have both qualified and unqualified lines in a single file like that. As the comment you found indicates, having multiple lines that both match a single configuration is an error. We used to support more-specific overriding less-specific, but it seemed to be unwarranted complexity. In this case, I would replace the two lines: platform/mac/tiled-drawing/ [ Pass ] [ Lion ] platform/mac/tiled-drawing/ [ Skip ] With just: [ MountainLion ] platform/mac/tiled-drawing/ [ Pass ] There is no current way to "futureproof" things so that all WK1 and only Lion WK2 is skipped. You would need to add either a specific WK1 TestExpectations file or add support for WK1 and WK2 keywords. I would recommend doing the former, especially if you plan to continue deprecating WK1 on Apple Mac more and more. (In reply to comment #15) > Yup, buggy error message (the two line numbers are swapped). > > At any rate, you can't have both qualified and unqualified lines in a single file like that. As the comment you found indicates, having multiple lines that both match a single configuration is an error. We used to support more-specific overriding less-specific, but it seemed to be unwarranted complexity. > > In this case, I would replace the two lines: > > platform/mac/tiled-drawing/ [ Pass ] > [ Lion ] platform/mac/tiled-drawing/ [ Skip ] > > With just: > > [ MountainLion ] platform/mac/tiled-drawing/ [ Pass ] That wouldn't work because we need to make it pass on MacFuture as well. (In reply to comment #16) > > In this case, I would replace the two lines: > > > > platform/mac/tiled-drawing/ [ Pass ] > > [ Lion ] platform/mac/tiled-drawing/ [ Skip ] > > > > With just: > > > > [ MountainLion ] platform/mac/tiled-drawing/ [ Pass ] > > That wouldn't work because we need to make it pass on MacFuture as well. That is what I was getting at with my last paragraph. I don't think "Future" is a supported keyword, but we could probably add it. It's not really the right solution, though. For this particular bug, I think moving the tests from platform/mac to platform/mac-wk2 and adding an entry in the mac-lion TestExpectations file to skip them is more appropriate, since the feature is mac-wk2 only. I will post a patch that does that shortly. Created attachment 185649 [details]
Patch
bug 108146 filed to discuss adding -wk1 specific directories and expectations files. Comment on attachment 185649 [details] Patch Committed in http://trac.webkit.org/changeset/141362 |