Bug 143693 - Skip failing test Tests/WebKit2Cocoa/FixedLayoutSize.mm on iOS
Summary: Skip failing test Tests/WebKit2Cocoa/FixedLayoutSize.mm on iOS
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: iPhone / iPad Unspecified
: P2 Normal
Assignee: Daniel Bates
URL: InRadar
Keywords:
Depends on:
Blocks:
 
Reported: 2015-04-13 21:40 PDT by Daniel Bates
Modified: 2015-04-14 10:58 PDT (History)
2 users (show)

See Also:


Attachments
Skip the test on iOS (1.61 KB, patch)
2015-04-13 21:44 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
Skip the test on iOS (1.41 KB, patch)
2015-04-14 09:32 PDT, Daniel Bates
thorton: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2015-04-13 21:40:24 PDT
The test Tests/WebKit2Cocoa/FixedLayoutSize.mm is failing on iOS. See <rdar://problem/20530316> for more details.
Comment 1 Daniel Bates 2015-04-13 21:44:21 PDT
Created attachment 250691 [details]
Skip the test on iOS
Comment 2 Daniel Bates 2015-04-13 21:46:55 PDT
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 3 Tim Horton 2015-04-14 01:06:02 PDT
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.
Comment 4 Tim Horton 2015-04-14 01:06:58 PDT
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.
Comment 5 Daniel Bates 2015-04-14 09:23:41 PDT
(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).
Comment 6 Daniel Bates 2015-04-14 09:32:43 PDT
Created attachment 250711 [details]
Skip the test on iOS
Comment 7 Tim Horton 2015-04-14 10:33:01 PDT
(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!
Comment 8 Daniel Bates 2015-04-14 10:58:38 PDT
Committed r182802: <http://trac.webkit.org/changeset/182802>