Bug 73997 – [Chromium] Exclude the four RenderTableCellDeathTest death tests for Android

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Peter Beverloo]

[Chromium] Exclude the four RenderTableCellDeathTest death tests for Android

[Peter Beverloo]

Created attachment 118201 [details]
Patch

[Peter Beverloo]

Julien, could you have a look? Alternative would be to exclude the file from the build, but having some (in this case, 4 out of 8) tests is still better than none.

[Julien Chaffraix]

Comment on attachment 118201 [details]
Patch

Those tests are more FYI than anything however disabling tests like that would need more explanations. Why is ASSERT_DEATH not available? Not implemented? Not possible to support on the Android platform? Do we have something tracking re-adding them? (skipping / disabling tests is easy but then we forget about them and that's what I fear here)

[Peter Beverloo]

Google Test has not been updated to recognize and support Android yet, and this is a requirement in order to enable the ASSERT_DEATH (or EXPECT_DEATH) macros as these are guarded per platform.

http://code.google.com/p/googletest/source/browse/trunk/include/gtest/internal/gtest-port.h#545

Would it work if I file a bug here to track this, and refer to that in the comment?

[Julien Chaffraix]

(In reply to comment #4)
> Google Test has not been updated to recognize and support Android yet, and this is a requirement in order to enable the ASSERT_DEATH (or EXPECT_DEATH) macros as these are guarded per platform.
> 
> http://code.google.com/p/googletest/source/browse/trunk/include/gtest/internal/gtest-port.h#545
> 
> Would it work if I file a bug here to track this, and refer to that in the comment?

Totally, it's just to make sure that we don't forget about the tests. Would be nice to refer to a Google Test bug too so that we can easily determine when to toggle the test on.

[Peter Beverloo]

Created attachment 118390 [details]
Updated patch

(In reply to comment #5)
> Totally, it's just to make sure that we don't forget about the tests. Would be nice to refer to a Google Test bug too so that we can easily determine when to toggle the test on.

I filed a WebKit Bug 74089. Upstream CL is http://codereview.appspot.com/5464048, but it may take quite a while before the updated GTest becomes available in Source/WebKit/chromium/, while this currently is a compile error.

[Julien Chaffraix]

Comment on attachment 118390 [details]
Updated patch

Thanks for filing the different bugs!

[WebKit Review Bot]

Comment on attachment 118390 [details]
Updated patch

Clearing flags on attachment: 118390

Committed r102362: <http://trac.webkit.org/changeset/102362>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.