Create CommandLineTestRule

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

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 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;
 
 import java.lang.annotation.ElementType;
 import java.lang.annotation.Inherited;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 import java.lang.reflect.AnnotatedElement;
 import java.lang.reflect.Method;
@@ skipping 30 matching lines @@
      * Note that this can only remove flags added via {@link Add} above.
      */
     @Inherited
     @Retention(RetentionPolicy.RUNTIME)
     @Target({ElementType.METHOD, ElementType.TYPE})
     public @interface Remove {
         String[] value();
     }
 
     /**
-     * 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;
+    }
+
+    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 (element.isAnnotationPresent(CommandLineFlags.Remove.class)) {
-            List<String> flagsToRemove =
-                    Arrays.asList(element.getAnnotation(CommandLineFlags.Remove.class).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
                 // than CommandLineFlags.Add (probably the command-line flag file).
                 Assert.assertFalse("Unable to remove command-line flag \"" + flagToRemove + "\".",
                         CommandLine.getInstance().hasSwitch(flagToRemove));
             }
             flags.removeAll(flagsToRemove);
         }
-
         return flags;
     }
 
     private CommandLineFlags() {
         throw new AssertionError("CommandLineFlags is a non-instantiable class");
     }
 
     public static PreTestHook getRegistrationHook() {
         return new PreTestHook() {
             @Override
@@ skipping 30 matching lines @@
      */
     public static class Parameter extends BaseParameter {
         public static final String PARAMETER_TAG = "cmdlinearg-parameter";
         public static final String ADD_ARG = "add";
         public static final String REMOVE_ARG = "remove";
 
         public Parameter(org.chromium.base.test.util.parameter.Parameter.Reader parameterReader) {
             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));
+        }
+    }
 }