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;
+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.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;
@@ skipping 33 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.

[jbudorick, 2017/01/11 20:21:33]

nit: "... using the given test method or class."

[the real yoland, 2017/03/16 22:52:59]

No longer relevant

      *
      * 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);

[jbudorick, 2017/01/11 20:21:33]

nit: just do

  setUp(targetContext, getFlags(element));

[the real yoland, 2017/03/16 22:52:59]

No longer relevant

+    }
+
+    /**
+     * 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}

[jbudorick, 2017/01/11 20:21:33]

Update this sentence:

  "This will add {@code flags} to the {@link org.chromium.base.CommandLine}."

updateFlags, which this does not call, is responsible for getting the set of
flags specified by various @CommandLineFlags.Add and @CommandLineFlags.Remove
annotations.

[the real yoland, 2017/03/16 22:52:59]

Done

+     * 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

[jbudorick, 2017/01/11 20:21:33]

This should say something like

  /** Determines the set of command-line flags that should be added for the
given element.
   *
   * This determines the difference of the sets of flags specified by {@link
CommandLineFlags.Add}
   * and {@link CommandLineFlags.Remove}.
   */

[the real yoland, 2017/03/16 22:52:59]

No longer relevant

+     */
+    @VisibleForTesting

[jbudorick, 2017/01/11 20:21:33]

This shouldn't be necessary.

[the real yoland, 2017/03/16 22:52:59]

No longer relevant

+    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);
+        return updateFlags(flags, element.getAnnotation(CommandLineFlags.Add.class),
+                element.getAnnotation(CommandLineFlags.Remove.class));
+    }
 
-        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

[jbudorick, 2017/01/11 20:21:33]

Same as above.

[the real yoland, 2017/03/16 22:52:59]

No longer relevant

+     */
+    @VisibleForTesting

[jbudorick, 2017/01/11 20:21:33]

This shouldn't be necessary.

[the real yoland, 2017/03/16 22:52:59]

No longer relevant

+    public static Set<String> getFlags(Description desc) {

[jbudorick, 2017/01/11 20:21:33]

Do we need to provide both versions of getFlags?

[the real yoland, 2017/03/16 22:52:59]

No longer relevant

+        Set<String> flags = getFlags(desc.getTestClass());
+        return updateFlags(flags, desc.getAnnotation(CommandLineFlags.Add.class),

[jbudorick, 2017/01/11 20:21:32]

Why does this call updateFlags a second time?

[the real yoland, 2017/03/16 22:52:59]

No longer relevant

+                desc.getAnnotation(CommandLineFlags.Remove.class));
+    }
+
+    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 {

[jbudorick, 2017/01/11 20:21:32]

A client would use this as

  @Rule CommandLineFlags.CommandLineTestRule mCommandLineRule = new
CommandLineFlags.CommandLineTestRule()

Could we either:
 1) rename this s.t. CommandLineFlags.<new name> isn't as repetitive (e.g. name
it "Rule" s.t. a client would do CommandLineFlags.Rule), or
 2) move this to its own file & have it be standalone.

[the real yoland, 2017/03/16 22:52:59]

Moved CommandLineTestRule to its own file

+        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));
+        }
+    }
 }