Disable some EncryptedMediaTests

Disable some EncryptedMediaTests

Those, for which "assertion `GTK_IS_WIDGET (widget)' failed" on Linux Debug.

BUG=174294
TBR=shadi@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=180711

[Bernhard Bauer, 2013-02-05 14:57:16]

Drive-by review!

https://codereview.chromium.org/12223005/diff/6001/content/browser/media/encr...
File content/browser/media/encrypted_media_browsertest.cc (right):

https://codereview.chromium.org/12223005/diff/6001/content/browser/media/encr...
content/browser/media/encrypted_media_browsertest.cc:130:
IN_PROC_BROWSER_TEST_P(EncryptedMediaTest, DISABLED_BasicPlayback_AudioOnly) {
We usually use a MAYBE_ prefix for the test and expand only that via #ifdefs.

[vabr (Chromium), 2013-02-05 15:06:17]

On 2013/02/05 14:57:16, Bernhard Bauer wrote:
> Drive-by review!
> 
>
https://codereview.chromium.org/12223005/diff/6001/content/browser/media/encr...
> File content/browser/media/encrypted_media_browsertest.cc (right):
> 
>
https://codereview.chromium.org/12223005/diff/6001/content/browser/media/encr...
> content/browser/media/encrypted_media_browsertest.cc:130:
> IN_PROC_BROWSER_TEST_P(EncryptedMediaTest, DISABLED_BasicPlayback_AudioOnly) {
> We usually use a MAYBE_ prefix for the test and expand only that via #ifdefs.

Yeah, but as the comment for Patch Set 4 indicates, IN_PROC_BROWSER_TEST_P
prevents expanding it's arguments as macro names. Once you are at your desk,
I'll ride by to discuss your drive-by review in more detail.

Currently, this CL is not planned for landing, due to some substantial changes
to the way these tests are run, just made by maruel.

[vabr (Chromium), 2013-02-05 16:10:45]

Hi Bernhard,

It looks like I'll be committing this after all.
Please take a look if the comments about MAYBE_ are OK.

Vaclav

[vabr (Chromium), 2013-02-05 16:18:57]

I'm going to land this soon, to see if it improves the linux debug tests. If it
does not, I'll revert.

Vaclav

[Bernhard Bauer, 2013-02-05 16:21:52]

https://codereview.chromium.org/12223005/diff/9002/content/browser/media/encr...
File content/browser/media/encrypted_media_browsertest.cc (right):

https://codereview.chromium.org/12223005/diff/9002/content/browser/media/encr...
content/browser/media/encrypted_media_browsertest.cc:130: // not expand the test
name macro correctly, TODO(vabr, bauerb)
The correct format for TODOs is `TODO(ldap):`

(Picking nits here because having a standardized format allows grepping for
TODOs.)

Also, see https://codereview.chromium.org/12209016.

[shadi, 2013-02-05 18:44:28]

https://codereview.chromium.org/12223005/diff/9002/content/browser/media/encr...
File content/browser/media/encrypted_media_browsertest.cc (right):

https://codereview.chromium.org/12223005/diff/9002/content/browser/media/encr...
content/browser/media/encrypted_media_browsertest.cc:118:
::testing::Values(kExternalClearKeyKeySystem));
Is not easier to just disable adding this parametrized test for
defined(OS_LINUX) && !defined(NDEBUG), rather than adding it for each test case
below?

[vabr (Chromium), 2013-02-05 18:52:14]

https://codereview.chromium.org/12223005/diff/9002/content/browser/media/encr...
File content/browser/media/encrypted_media_browsertest.cc (right):

https://codereview.chromium.org/12223005/diff/9002/content/browser/media/encr...
content/browser/media/encrypted_media_browsertest.cc:118:
::testing::Values(kExternalClearKeyKeySystem));
On 2013/02/05 18:44:28, shadi wrote:
> Is not easier to just disable adding this parametrized test for
> defined(OS_LINUX) && !defined(NDEBUG), rather than adding it for each test
case
> below? 

Well, there was still FrameChangeVideo which did not seem to be affected in the
same way as the others, and I did not want to disable that.

[shadi, 2013-02-05 18:58:04]

`GTK_IS_WIDGET (widget)' fail should affect any external clear key test.
ddowrin@ had patched a fix into webkit and it will land soon. So, there should
not be a problem in disabling all external clear key tests for those build
conditions.

Thanks!