[Android] Support conditional test disabling based on android.os.Build values.

chrome/android/javatests/src/org/chromium/chrome/browser/NavigateTest.java

Index: chrome/android/javatests/src/org/chromium/chrome/browser/NavigateTest.java
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/NavigateTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/NavigateTest.java
index be7e7faff9d6007c590a28dbe4e78bd464e470ee..ba1bad888d71f086156911d700cc7f49cc96ba6c 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/NavigateTest.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/NavigateTest.java
@@ -13,7 +13,7 @@ import android.text.TextUtils;
 import android.view.KeyEvent;
 
 import org.chromium.base.ThreadUtils;
-import org.chromium.base.test.util.DisabledTest;
+import org.chromium.base.test.util.DisableIf;
 import org.chromium.base.test.util.Feature;
 import org.chromium.base.test.util.Restriction;
 import org.chromium.base.test.util.UrlUtils;
@@ -355,7 +355,7 @@ public class NavigateTest extends ChromeTabbedActivityTestBase {
         }
     }
 
-    @DisabledTest // Fails on android-one: crbug.com/540723
+    @DisableIf.Build(hardware_is = "sprout") // Fails on android-one: crbug.com/540723

[jbudorick, 2015/12/10 15:23:15]

There are, unsurprisingly, other tests where this can be used, but I just wanted
to include one as a sample.

I'm a bit concerned here about how this could facilitate accidental leaks, but
I'm not sure how to resolve or mitigate that. Maybe a PRESUBMIT check of some
kind?

[Yaron, 2015/12/11 02:22:16]

:(
Should we include a message= attribute which can link to the bug (so you can see
in the test runner that it's skipping some tsets?)
Ya, I could definitely see that happen. You could have a PRESUBMIT of
whitelisted devices based on what we run on waterfalls. Of course that can still
fail if it's on the internal waterfall. Another option is to wait to introduce
this until more of the tests are moved up (I know, not the greatest)

[jbudorick, 2015/12/17 02:08:13]

I like this idea, but I'll address how the test runner handles this annotation
in general (and message= specifically) in a separate CL if at all.
I'm less worried about the tests and more worried about the possibility of
wanting to disable an upstream test on prerelease hardware.

[Yaron, 2015/12/17 15:12:37]

Right. So what options do we provide for sheriffs now? The SDK check <= (which I
complained about) seemed likes the easiest solution. I think in general it's
expected that we're testing with unreleased software and there's always a new
android coming - but hardware codenames are more sensitive. I guess I take it
back - feel free to add back sdk_is_less_than and we'll see to what extent it
gets (ab)used. It seems like the easiest way to support this restriction and
minimize leaks.

Relatedly - is there a significant difference between DisableIf and
Restrictions? DisableIf means it's broken and Restriction means it's never
intended to work?

[jbudorick, 2015/12/18 17:43:06]

Hm, ok.
That's what I had in mind, but perhaps that's a more nuanced split than we want.

     @MediumTest
     @Feature({"Navigation"})
     public void testWindowOpenUrlSpoof() throws Exception {