Disable NPLB test cases which are not applicable for Android #4721
Conversation
There was a problem hiding this comment.
These changes LGTM (modulo my small comments), would like to solicit @y4vor 's opinion (and @andrewsavage1 FYI).
| @@ -34,6 +35,11 @@ TEST(SbDirectoryCanOpenTest, SunnyDay) { | |||
| } | |||
|
|
|||
| TEST(SbDirectoryCanOpenTest, SunnyDayStaticContent) { | |||
| // TODO, b/380344836, the test cases are not applicable on Android | |||
| if (GetRuntimePlatform() == PlatformType::kPlatformTypeAndroid) { | |||
| GTEST_SKIP() << "Not applicable on Android"; | |||
There was a problem hiding this comment.
With this and subsequent SKIPs, could we remove some suppressions from test_filters.py ?
| @@ -34,6 +35,11 @@ TEST(SbDirectoryCanOpenTest, SunnyDay) { | |||
| } | |||
|
|
|||
| TEST(SbDirectoryCanOpenTest, SunnyDayStaticContent) { | |||
| // TODO, b/380344836, the test cases are not applicable on Android | |||
There was a problem hiding this comment.
QQ: Shouldn't this be // TODO(b/380344836): ? (Same throughout).
There was a problem hiding this comment.
How about remove TODO and leave the ticket number only as there is not follow up work for this on Android
There was a problem hiding this comment.
it is updated to // TODO(b/380344836): Skip test case(s) not applicable to Android.
| @@ -34,6 +35,11 @@ TEST(SbDirectoryCanOpenTest, SunnyDay) { | |||
| } | |||
|
|
|||
| TEST(SbDirectoryCanOpenTest, SunnyDayStaticContent) { | |||
| // TODO, b/380344836, the test cases are not applicable on Android | |||
There was a problem hiding this comment.
It's not clear to me what the follow up work here actually is
There was a problem hiding this comment.
I keep the TODO as a reminder that this can be refined
| @@ -34,6 +35,11 @@ TEST(SbDirectoryCanOpenTest, SunnyDay) { | |||
| } | |||
|
|
|||
| TEST(SbDirectoryCanOpenTest, SunnyDayStaticContent) { | |||
| // TODO, b/380344836, the test cases are not applicable on Android | |||
There was a problem hiding this comment.
How did you determine which tests should be disabled? Android on c25 is expected to use the entire Starboard layer, so I don't see how we're able to disable any of these
There was a problem hiding this comment.
The disabled test cases are identified because they are failed on Android. For each failed test case, a ticket is created to review them. Only the reviewed test cases are removed from Android platform. Most of the test cases are removed because they are not available no Android platform.
jonastsai
force-pushed
the
b-380032222
branch
from
8ebff95 to
b4ad82a
Compare
b/338229737 b/368228941 b/379602017 b/380030274 b/380031306 b/380032222 b/380344836 b/380347735
jonastsai
force-pushed
the
b-380032222
branch
from
b4ad82a to
3ae0293
Compare
b/338229737
b/368228941
b/379602017
b/380030274
b/380031306
b/380032222
b/380344836
b/380347735