Create CommandLineTestRule

base/test/android/javatests/src/org/chromium/base/test/util/CommandLineFlags.java

Index: base/test/android/javatests/src/org/chromium/base/test/util/CommandLineFlags.java
diff --git a/base/test/android/javatests/src/org/chromium/base/test/util/CommandLineFlags.java b/base/test/android/javatests/src/org/chromium/base/test/util/CommandLineFlags.java
index 3e97532defd47333206199f04b187891c359919a..f4cb27c946f53c4500708f5ea9ed0d2ede6526fb 100644
--- a/base/test/android/javatests/src/org/chromium/base/test/util/CommandLineFlags.java
+++ b/base/test/android/javatests/src/org/chromium/base/test/util/CommandLineFlags.java
@@ -5,8 +5,12 @@
 package org.chromium.base.test.util;
 
 import android.content.Context;
+import android.support.test.InstrumentationRegistry;
 
-import junit.framework.Assert;
+import org.junit.Assert;
+import org.junit.rules.TestRule;
+import org.junit.runner.Description;
+import org.junit.runners.model.Statement;
 
 import org.chromium.base.BaseChromiumApplication;
 import org.chromium.base.CommandLine;
@@ -67,29 +71,43 @@ public final class CommandLineFlags {
      * trying to remove a flag set externally, i.e. by the command-line flags file, will not work.
      */
     public static void setUp(Context targetContext, AnnotatedElement element) {
+        Set<String> flags = getFlags(element);
+        setUp(targetContext, flags);
+    }
+
+    public static void setUp(Context targetContext, Set<String> flags) {

[jbudorick, 2016/11/23 16:07:36]

Public methods ought to have javadoc comments.

[the real yoland, 2016/11/23 20:44:36]

Done

         Assert.assertNotNull("Unable to get a non-null target context.", targetContext);
-        CommandLine.reset();
         BaseChromiumApplication.initCommandLine(targetContext);
-        Set<String> flags = getFlags(element);
         for (String flag : flags) {
             CommandLine.getInstance().appendSwitch(flag);
         }
     }
 
-    private static Set<String> getFlags(AnnotatedElement element) {
+    public static Set<String> getFlags(AnnotatedElement element) {
         AnnotatedElement parent = (element instanceof Method)
                 ? ((Method) element).getDeclaringClass()
-                : ((Class) element).getSuperclass();
+                : ((Class<?>) element).getSuperclass();
         Set<String> flags = (parent == null) ? new HashSet<String>() : getFlags(parent);
+        flags = addAndRemoveFlags(flags, element.getAnnotation(CommandLineFlags.Add.class),

[jbudorick, 2016/11/23 16:07:36]

nit: The indentation here, while maybe right, looks weird w/ addAndRemoveFlags
matching element.getAnnotation

[the real yoland, 2016/11/23 20:44:36]

Hmm, it does looks weird, but it was formatted by `git cl format base` and
presubmit would give me warning if I don't have indentation this way.

+                element.getAnnotation(CommandLineFlags.Remove.class));
+        return flags;
+    }
 
-        if (element.isAnnotationPresent(CommandLineFlags.Add.class)) {
-            flags.addAll(
-                    Arrays.asList(element.getAnnotation(CommandLineFlags.Add.class).value()));
-        }
+    public static Set<String> getFlags(Description desc) {
+        Set<String> flags = getFlags(desc.getTestClass());
+        flags = addAndRemoveFlags(flags, desc.getAnnotation(CommandLineFlags.Add.class),
+                desc.getAnnotation(CommandLineFlags.Remove.class));
+        return flags;
+    }
 
-        if (element.isAnnotationPresent(CommandLineFlags.Remove.class)) {
-            List<String> flagsToRemove =
-                    Arrays.asList(element.getAnnotation(CommandLineFlags.Remove.class).value());
+    public static Set<String> addAndRemoveFlags(Set<String> flags,

[mikecase (-- gone --), 2016/11/23 16:14:34]

nit: updateFlags might be a better name

[the real yoland, 2016/11/23 20:44:36]

Done

+            CommandLineFlags.Add addAnnotation, CommandLineFlags.Remove removeAnnotation) {
+        Assert.assertNotNull("flags set can not be null", flags);
+        if (addAnnotation != null) {
+            flags.addAll(Arrays.asList(addAnnotation.value()));
+        }
+        if (removeAnnotation != null) {
+            List<String> flagsToRemove = Arrays.asList(removeAnnotation.value());
             for (String flagToRemove : flagsToRemove) {
                 // If your test fails here, you have tried to remove a command-line flag via
                 // CommandLineFlags.Remove that was loaded into CommandLine via something other
@@ -99,7 +117,6 @@ public final class CommandLineFlags {
             }
             flags.removeAll(flagsToRemove);
         }
-
         return flags;
     }
 
@@ -150,4 +167,45 @@ public final class CommandLineFlags {
             super(PARAMETER_TAG, parameterReader);
         }
     }
+
+    /**
+     * TestRule to used by JUnit4 style instrumentation test to set CommandLine flags.
+     *
+     * If you need to add CommandLine flag to your junit4 style instrumentation test,
+     * use CommandLineFlag.Add and CommandLineFlags.Remove annotation on your test method or test
+     * class. Then add CommandLineTestRule in your test. For example
+     *
+     * <code>
+     * @CommandLineFlags.Add(...)
+     * public class ChromeActivityTest {
+     *     @Rule CommandLineTestRule mCommandLineRule = new CommandLineTestRule();

[jbudorick, 2016/11/23 16:07:36]

So what happens if someone uses @CommandLineFlags.{Add,Remove} without the
CommandLineTestRule? Nothing? If so, could we warn people somehow?

[mikecase (-- gone --), 2016/11/23 16:14:34]

Is there any way to warn people if they added the annotations but forgot to add
this rule?
Can we add this rule as default for all test?

[the real yoland, 2016/11/23 20:44:36]

I can throw error in BaseJUnit4ClassRunner when a test has Add/Remove annotation
without a CommandLineTestRule field
I would like to think of an expandible way to do this in BaseJunit4ClassRunner
in case other Rules are required for other annotations.
Added a temporary TODO in the code, will implement in this CL

+     *
+     *     @CommandLineFlags.Remove(...)
+     *     @Test
+     *     public void test() {...}
+     * </code>
+     */
+    public static class CommandLineTestRule implements TestRule {
+        private class CommandLineTestRuleStatement extends Statement {
+            private Statement mBase;
+            private Set<String> mFlags;
+
+            public CommandLineTestRuleStatement(Statement base, Set<String> flags) {
+                mBase = base;
+                mFlags = flags;
+            }
+
+            @Override
+            public void evaluate() throws Throwable {
+                Context targetContext = InstrumentationRegistry.getContext();
+                CommandLineFlags.setUp(targetContext, mFlags);
+                mBase.evaluate();
+            }
+        }
+
+        @Override
+        public Statement apply(final Statement base, Description desc) {
+            return new CommandLineTestRuleStatement(base, getFlags(desc));
+        }
+    }
 }