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..9816b6f193184e13603963cfe7270431962bca26 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,11 +5,16 @@
 package org.chromium.base.test.util;
 
 import android.content.Context;
+import android.support.test.InstrumentationRegistry;
 
-import junit.framework.Assert;
+import org.junit.Assert;

[jbudorick, 2016/12/01 00:30:14]

Does this work with the existing JUnit3-based code?

[the real yoland, 2017/01/11 19:57:50]

yes, it does

+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;
+import org.chromium.base.VisibleForTesting;

[jbudorick, 2016/12/01 00:30:14]

This shouldn't be necessary...

[the real yoland, 2017/01/11 19:57:50]

Deleted

 import org.chromium.base.test.BaseTestResult.PreTestHook;
 import org.chromium.base.test.util.parameter.BaseParameter;
 
@@ -60,36 +65,66 @@ public final class CommandLineFlags {
     }
 
     /**
-     * Sets up the CommandLine with the appropriate flags.
+     * Sets up the CommandLine flags using test method or class.
      *
      * This will add the difference of the sets of flags specified by {@link CommandLineFlags.Add}
      * and {@link CommandLineFlags.Remove} to the {@link org.chromium.base.CommandLine}. Note that
      * 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);
+    }
+
+    /**
+     * Sets up the CommandLine flags using a set of flag strings.
+     *
+     * This will add the difference of the sets of flags specified by {@link CommandLineFlags.Add}
+     * and {@link CommandLineFlags.Remove} to the {@link org.chromium.base.CommandLine}. Note that
+     * trying to remove a flag set externally, i.e. by the command-line flags file, will not work.
+
+     */
+    public static void setUp(Context targetContext, Set<String> flags) {
         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) {
+    /**
+     * @return a set of strings to represent the commandline flags
+     */
+    @VisibleForTesting
+    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 = updateFlags(flags, element.getAnnotation(CommandLineFlags.Add.class),

[jbudorick, 2016/12/01 00:30:14]

nit: return updateFlags(...)

[the real yoland, 2017/01/11 19:57:50]

Done

+                element.getAnnotation(CommandLineFlags.Remove.class));
+        return flags;
+    }
 
-        if (element.isAnnotationPresent(CommandLineFlags.Add.class)) {
-            flags.addAll(
-                    Arrays.asList(element.getAnnotation(CommandLineFlags.Add.class).value()));
-        }
+    /**
+     * @return a set of strings to represent the commandline flags
+     */
+    @VisibleForTesting
+    public static Set<String> getFlags(Description desc) {
+        Set<String> flags = getFlags(desc.getTestClass());
+        flags = updateFlags(flags, desc.getAnnotation(CommandLineFlags.Add.class),

[jbudorick, 2016/12/01 00:30:14]

nit: return updateFlags(...)

[the real yoland, 2017/01/11 19:57:50]

Done

+                desc.getAnnotation(CommandLineFlags.Remove.class));
+        return flags;
+    }
 
-        if (element.isAnnotationPresent(CommandLineFlags.Remove.class)) {
-            List<String> flagsToRemove =
-                    Arrays.asList(element.getAnnotation(CommandLineFlags.Remove.class).value());
+    private static Set<String> updateFlags(Set<String> flags, 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 +134,6 @@ public final class CommandLineFlags {
             }
             flags.removeAll(flagsToRemove);
         }
-
         return flags;
     }
 
@@ -150,4 +184,48 @@ 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();
+     *
+     *     @CommandLineFlags.Remove(...)
+     *     @Test
+     *     public void test() {...}
+     * </code>
+     *
+     * TODO(yolandyan): Add error message in BaseJUnit4ClassRunner for tests with CommandLineFlag
+     * annotations but did not have a CommandLineTestRule field
+     */
+    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();

[mikecase (-- gone --), 2016/12/01 00:27:54]

what does mBase.evaluate() do?

[the real yoland, 2017/01/11 19:57:50]

This is the the onion structure Rule have to wrap one statement around another.
mBase.evaluate() call the inner statement's evaluate, which would it's inner
statement, etc.

It's not really clean design, but that's just junit4 for you :(

+            }
+        }
+
+        @Override
+        public Statement apply(final Statement base, Description desc) {
+            return new CommandLineTestRuleStatement(base, getFlags(desc));
+        }
+    }
 }