Add macro for consistent logging when disabling tests.

Add macro for consistent logging when disabling tests.

There are two macros, one for disabled tests and ones where a test
is skipped (e.g. unsupported platform, rather than a bug).

BUG=635935
Committed: https://crrev.com/d674e7bc7369c5fd08b8caffa1a9765c14b6cf75
Cr-Commit-Position: refs/heads/master@{#410860}

[baxley, 2016-08-09 20:00:43]

baxley@chromium.org changed reviewers:
+ cmasso@google.com, eugenebut@chromium.org

[Eugene But (OOO till 7-30), 2016-08-09 20:47:38]

lgtm

https://codereview.chromium.org/2224313003/diff/1/ios/testing/earl_grey/disab...
File ios/testing/earl_grey/disabled_test_macros.h (right):

https://codereview.chromium.org/2224313003/diff/1/ios/testing/earl_grey/disab...
ios/testing/earl_grey/disabled_test_macros.h:17: #define
EARL_GREY_TEST_DISABLED(message)                                \
Optional since KIF also has this problem. This thing will not work as expected:

if (base::ios::IsRunningOnIOS10OrLater())
  EARL_GREY_TEST_DISABLED(@"Disabled on iOS 10.");

How about this?:

while (true) {
  NSLog(@"-- Earl Grey Test Disabled -- %@", message);
  return;
}

[cmasso1, 2016-08-09 21:00:21]

cmasso@chromium.org changed reviewers:
+ cmasso@chromium.org

[cmasso1, 2016-08-09 21:00:22]

lgtm with nits

https://codereview.chromium.org/2224313003/diff/1/ios/testing/earl_grey/disab...
File ios/testing/earl_grey/disabled_test_macros.h (right):

https://codereview.chromium.org/2224313003/diff/1/ios/testing/earl_grey/disab...
ios/testing/earl_grey/disabled_test_macros.h:11: // at runtime. Disabling at
complile-time is always preferred.
nit: s/complile/compile

https://codereview.chromium.org/2224313003/diff/1/ios/testing/earl_grey/disab...
ios/testing/earl_grey/disabled_test_macros.h:11: // at runtime. Disabling at
complile-time is always preferred.
Also, Isn't it helpful to highlight here what should be used when disabling at
compile time?

[baxley, 2016-08-09 21:05:57]

Comments addressed! Thanks for the review.

https://codereview.chromium.org/2224313003/diff/1/ios/testing/earl_grey/disab...
File ios/testing/earl_grey/disabled_test_macros.h (right):

https://codereview.chromium.org/2224313003/diff/1/ios/testing/earl_grey/disab...
ios/testing/earl_grey/disabled_test_macros.h:11: // at runtime. Disabling at
complile-time is always preferred.
On 2016/08/09 21:00:22, cmasso1 wrote:
> nit: s/complile/compile

Done.

https://codereview.chromium.org/2224313003/diff/1/ios/testing/earl_grey/disab...
ios/testing/earl_grey/disabled_test_macros.h:11: // at runtime. Disabling at
complile-time is always preferred.
On 2016/08/09 21:00:22, cmasso1 wrote:
> Also, Isn't it helpful to highlight here what should be used when disabling at
> compile time?

I thought about it, however we haven't set the precedent of how we'll do it in
Earl Grey. It's likely to be the same as KIF, but I'd rather not put in a
comment that becomes incorrect, and I never remember to update it. This way it
will stay correct, though not as specific as it could be. WDYT?

https://codereview.chromium.org/2224313003/diff/1/ios/testing/earl_grey/disab...
ios/testing/earl_grey/disabled_test_macros.h:17: #define
EARL_GREY_TEST_DISABLED(message)                                \
On 2016/08/09 20:47:37, Eugene But wrote:
> Optional since KIF also has this problem. This thing will not work as
expected:
> 
> if (base::ios::IsRunningOnIOS10OrLater())
>   EARL_GREY_TEST_DISABLED(@"Disabled on iOS 10.");
> 
> How about this?:
> 
> while (true) {
>   NSLog(@"-- Earl Grey Test Disabled -- %@", message);
>   return;
> }
> 

Nice catch! I don't think not having it in KIF is reason to leave this out.
Done.

[cmasso1, 2016-08-09 21:12:12]

https://codereview.chromium.org/2224313003/diff/1/ios/testing/earl_grey/disab...
File ios/testing/earl_grey/disabled_test_macros.h (right):

https://codereview.chromium.org/2224313003/diff/1/ios/testing/earl_grey/disab...
ios/testing/earl_grey/disabled_test_macros.h:11: // at runtime. Disabling at
complile-time is always preferred.
On 2016/08/09 21:05:57, baxley wrote:
> On 2016/08/09 21:00:22, cmasso1 wrote:
> > nit: s/complile/compile
> 
> Done.

Fine by me. Thanks!

[baxley, 2016-08-09 21:16:55]

The CQ bit was checked by baxley@chromium.org

[baxley, 2016-08-09 21:16:56]

The patchset sent to the CQ was uploaded after l-g-t-m from
eugenebut@chromium.org, cmasso@chromium.org
Link to the patchset: https://codereview.chromium.org/2224313003/#ps40001
(title: "typo!")

[commit-bot: I haz the power, 2016-08-09 21:17:48]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-09 22:57:25]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-09 22:59:21]

Description was changed from

==========
Add macro for consistent logging when disabling tests.

There are two macros, one for disabled tests and ones where a test
is skipped (e.g. unsupported platform, rather than a bug).

BUG=635935
==========

to

==========
Add macro for consistent logging when disabling tests.

There are two macros, one for disabled tests and ones where a test
is skipped (e.g. unsupported platform, rather than a bug).

BUG=635935

Committed: https://crrev.com/d674e7bc7369c5fd08b8caffa1a9765c14b6cf75
Cr-Commit-Position: refs/heads/master@{#410860}
==========

[commit-bot: I haz the power, 2016-08-09 22:59:22]

Patchset 3 (id:??) landed as
https://crrev.com/d674e7bc7369c5fd08b8caffa1a9765c14b6cf75
Cr-Commit-Position: refs/heads/master@{#410860}