Cleanup unit tests for HLS mime types on Android

media/base/mime_util_unittest.cc

Index: media/base/mime_util_unittest.cc
diff --git a/media/base/mime_util_unittest.cc b/media/base/mime_util_unittest.cc
index 29752334383c84363f1d444eedbc458df9fbaf62..c46ea9f1c6845b0cd74b3d82536d9e8a2f1c3f13 100644
--- a/media/base/mime_util_unittest.cc
+++ b/media/base/mime_util_unittest.cc
@@ -7,20 +7,8 @@
 #include "media/base/mime_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-#if defined(OS_ANDROID)
-#include "base/android/build_info.h"
-#endif
-
 namespace media {
 
-TEST(MimeUtilTest, LookupTypes) {
-#if defined(OS_ANDROID)
-  EXPECT_TRUE(IsSupportedMediaMimeType("application/vnd.apple.mpegurl"));
-  EXPECT_TRUE(IsSupportedMediaMimeType("application/x-mpegurl"));
-  EXPECT_TRUE(IsSupportedMediaMimeType("Application/X-MPEGURL"));
-#endif
-}
-
 TEST(MimeUtilTest, StrictMediaMimeType) {
   EXPECT_TRUE(IsStrictMediaMimeType("video/webm"));
   EXPECT_TRUE(IsStrictMediaMimeType("Video/WEBM"));
@@ -53,14 +41,6 @@ TEST(MimeUtilTest, StrictMediaMimeType) {
 }
 
 TEST(MimeUtilTest, CommonMediaMimeType) {
-#if defined(OS_ANDROID)
-  bool HLSSupported;
-  if (base::android::BuildInfo::GetInstance()->sdk_int() < 14)
-    HLSSupported = false;
-  else
-    HLSSupported = true;
-#endif
-
   EXPECT_TRUE(IsSupportedMediaMimeType("audio/webm"));
   EXPECT_TRUE(IsSupportedMediaMimeType("video/webm"));
 
@@ -71,9 +51,12 @@ TEST(MimeUtilTest, CommonMediaMimeType) {
   EXPECT_TRUE(IsSupportedMediaMimeType("application/ogg"));
 #if defined(OS_ANDROID)
   EXPECT_FALSE(IsSupportedMediaMimeType("video/ogg"));
-  EXPECT_EQ(HLSSupported, IsSupportedMediaMimeType("application/x-mpegurl"));
-  EXPECT_EQ(HLSSupported,
-            IsSupportedMediaMimeType("application/vnd.apple.mpegurl"));
+
+  // HLS is supported on Android API level 14 and higher and Chrome supports
+  // API levels 15 and higher, so these are expected to be supported.
+  EXPECT_TRUE(IsSupportedMediaMimeType("application/x-mpegurl"));

[ddorwin, 2015/05/06 20:00:20]

alternatively, you could do this:
  bool kHlsSupported = true;
#else
  bool kHlsSupported = false;
#endif

EXPECT_EQ(kHlsSupported, ...)

This ensures equal coverage, no typos in one test vs. the other, etc.

[servolk, 2015/05/06 20:32:22]

Wait, but that would be the same code as before? I'm removing the API level
check and simplifying tests as suggested by rsleevi@ at
https://codereview.chromium.org/1129643002/diff/80001/media/base/mime_util_un....
Chromium now doesn't support Android API levels lower than 15 or 16, so there's
no need for this conditional check, we can just assume HLS is going to be
supported.

[Ryan Sleevi, 2015/05/06 21:40:02]

No it wouldn't

bool hls_supported = false;
#if defined(OS_ANDROID)
hls_supported = true;
#endif

EXPECT_EQ(hls_supported, IsSupportedMediaMimeType(...));

This ensures that you don't have copy-pasta issues between lines 57/59 and lines
62/63. There's no API level check involved.

+  EXPECT_TRUE(IsSupportedMediaMimeType("Application/X-MPEGURL"));
+  EXPECT_TRUE(IsSupportedMediaMimeType("application/vnd.apple.mpegurl"));
 #else
   EXPECT_TRUE(IsSupportedMediaMimeType("video/ogg"));
   EXPECT_FALSE(IsSupportedMediaMimeType("application/x-mpegurl"));