Cleanup unit tests for HLS mime types on Android

Cleanup unit tests for HLS mime types on Android

BUG=none

Committed: https://crrev.com/1f9ed5eb41d2e8d65984a851973d1a7183f4553e
Cr-Commit-Position: refs/heads/master@{#330258}

[servolk, 2015-05-06 17:03:41]

servolk@chromium.org changed reviewers:
+ ddorwin@chromium.org, rsleevi@chromium.org

[ddorwin, 2015-05-06 20:00:21]

LGTM with suggestion.

https://codereview.chromium.org/1128983002/diff/1/media/base/mime_util_unitte...
File media/base/mime_util_unittest.cc (right):

https://codereview.chromium.org/1128983002/diff/1/media/base/mime_util_unitte...
media/base/mime_util_unittest.cc:57:
EXPECT_TRUE(IsSupportedMediaMimeType("application/x-mpegurl"));
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]

https://codereview.chromium.org/1128983002/diff/1/media/base/mime_util_unitte...
File media/base/mime_util_unittest.cc (right):

https://codereview.chromium.org/1128983002/diff/1/media/base/mime_util_unitte...
media/base/mime_util_unittest.cc:57:
EXPECT_TRUE(IsSupportedMediaMimeType("application/x-mpegurl"));
On 2015/05/06 20:00:20, ddorwin wrote:
> 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.

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]

https://codereview.chromium.org/1128983002/diff/1/media/base/mime_util_unitte...
File media/base/mime_util_unittest.cc (right):

https://codereview.chromium.org/1128983002/diff/1/media/base/mime_util_unitte...
media/base/mime_util_unittest.cc:57:
EXPECT_TRUE(IsSupportedMediaMimeType("application/x-mpegurl"));
On 2015/05/06 20:32:22, servolk wrote:
> On 2015/05/06 20:00:20, ddorwin wrote:
> > 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.
> 
> Wait, but that would be the same code as before? 

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.

[Ryan Sleevi, 2015-05-06 21:40:20]

Not like you LGTM, but +1 ddorwin's suggestion.

[servolk, 2015-05-06 21:48:54]

On 2015/05/06 21:40:20, Ryan Sleevi wrote:
> Not like you LGTM, but +1 ddorwin's suggestion.

Done in patchset #2

[Ryan Sleevi, 2015-05-06 22:15:28]

On 2015/05/06 21:48:54, servolk wrote:
> On 2015/05/06 21:40:20, Ryan Sleevi wrote:
> > Not like you LGTM, but +1 ddorwin's suggestion.
> 
> Done in patchset #2

That was meant to be "not like you need my LGTM" - sorry about that. You look
quite alright to me ;)

[servolk, 2015-05-16 00:37:00]

The CQ bit was checked by servolk@chromium.org

[servolk, 2015-05-16 00:37:00]

The patchset sent to the CQ was uploaded after l-g-t-m from ddorwin@chromium.org
Link to the patchset: https://codereview.chromium.org/1128983002/#ps20001
(title: "CR feedback")

[commit-bot: I haz the power, 2015-05-16 00:38:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1128983002/20001

[commit-bot: I haz the power, 2015-05-16 01:09:47]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-05-18 11:30:28]

Patchset 2 (id:??) landed as
https://crrev.com/1f9ed5eb41d2e8d65984a851973d1a7183f4553e
Cr-Commit-Position: refs/heads/master@{#330258}