| Summary: | Skip failing test Tests/WebKit2Cocoa/FixedLayoutSize.mm on iOS | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Daniel Bates <dbates> | ||||||
| Component: | Tools / Tests | Assignee: | Daniel Bates <dbates> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | simon.fraser, thorton | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | iPhone / iPad | ||||||||
| OS: | Unspecified | ||||||||
| URL: | InRadar | ||||||||
| Attachments: |
|
||||||||
|
Description
Daniel Bates
2015-04-13 21:40:24 PDT
Created attachment 250691 [details]
Skip the test on iOS
I have not had a chance to investigate the test failure. If we feel that we cannot fix the test in a reasonable amount of time then I suggest we temporarily skip it so that the test bot is not red. Comment on attachment 250691 [details] Skip the test on iOS View in context: https://bugs.webkit.org/attachment.cgi?id=250691&action=review > Tools/TestWebKitAPI/Tests/WebKit2Cocoa/FixedLayoutSize.mm:55 > +#if PLATFORM(IOS) Is this how we usually do this? I was expecting a big PLATFORM #if around the whole thing. I think we should skip it, since I don't have any motivation to make fixed layout take over from the viewport code on iOS at the moment. That would be more like a feature than fixing a test. (In reply to comment #3) > Comment on attachment 250691 [details] > Skip the test on iOS > > View in context: > https://bugs.webkit.org/attachment.cgi?id=250691&action=review > > > Tools/TestWebKitAPI/Tests/WebKit2Cocoa/FixedLayoutSize.mm:55 > > +#if PLATFORM(IOS) > > Is this how we usually do this? I was expecting a big PLATFORM #if around > the whole thing. I will update the patch to disable the test by conditionally compiling it on non-iOS platforms given that you do not "have any motivation to make fixed layout take over from the viewport code on iOS at the moment" (comment #4). When I wrote the original patch I was unclear if we wanted to skip this test permanently on iOS as implied by your remarks in comment #4. So, I chose to prefix the name of the test with "DISABLED_" to demarcate that the test is temporarily disabled. Among of the benefits of this approach is that GoogleTest will print a banner about the disabled tests (as a reminder to fix it). Created attachment 250711 [details]
Skip the test on iOS
(In reply to comment #5) > (In reply to comment #3) > > Comment on attachment 250691 [details] > > Skip the test on iOS > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=250691&action=review > > > > > Tools/TestWebKitAPI/Tests/WebKit2Cocoa/FixedLayoutSize.mm:55 > > > +#if PLATFORM(IOS) > > > > Is this how we usually do this? I was expecting a big PLATFORM #if around > > the whole thing. > > I will update the patch to disable the test by conditionally compiling it on > non-iOS platforms given that you do not "have any motivation to make fixed > layout take over from the viewport code on iOS at the moment" (comment #4). > > When I wrote the original patch I was unclear if we wanted to skip this test > permanently on iOS as implied by your remarks in comment #4. So, I chose to > prefix the name of the test with "DISABLED_" to demarcate that the test is > temporarily disabled. Among of the benefits of this approach is that > GoogleTest will print a banner about the disabled tests (as a reminder to > fix it). Ah! That makes sense. Thanks! Committed r182802: <http://trac.webkit.org/changeset/182802> |