Bug 143693 – Skip failing test Tests/WebKit2Cocoa/FixedLayoutSize.mm on iOS

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Daniel Bates]

The test Tests/WebKit2Cocoa/FixedLayoutSize.mm is failing on iOS. See <rdar://problem/20530316> for more details.

[Daniel Bates]

Created attachment 250691 [details]
Skip the test on iOS

[Daniel Bates]

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.

[Tim Horton]

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.

[Tim Horton]

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.

[Daniel Bates]

(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).

[Daniel Bates]

Created attachment 250711 [details]
Skip the test on iOS

[Tim Horton]

(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!

[Daniel Bates]

Committed r182802: <http://trac.webkit.org/changeset/182802>