📰 Add tests for Tile and TileGroup

chrome/android/junit/src/org/chromium/chrome/browser/EnableFeatures.java

 // Copyright 2016 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.chrome.browser;
 
 import org.chromium.testing.local.AnnotationProcessor;
 
 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.util.Arrays;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.Set;
 
 /**
  * Annotation used to set Feature flags during JUnit tests. To have an effect, the associated
  * {@link Processor} rule needs to be registered on the test class.
  *
  * Sample code:
  *
  * <pre>
  * public class Test {
  *    &#64;Rule
  *    public EnableFeatures.Processor processor = new EnableFeatures.Processor();
  *
  *    &#64;EnableFeatures(ChromeFeatureList.NTP_SNIPPETS_OFFLINE_BADGE)
  *    public void testFoo() { ... }
  * }
  * </pre>
  */
 @Inherited
 @Retention(RetentionPolicy.RUNTIME)
 @Target({ElementType.METHOD, ElementType.TYPE})
 public @interface EnableFeatures {
 
     String[] value();
 
     /**
-     * Add this rule to tests to activate the {@link EnableFeatures} annotations and choose flags
-     * to enable, or get rid of exceptions when the production code tries to check for enabled
-     * features.
+     * Add this rule to tests get rid of exceptions when the production code tries to check for
+     * enabled features. It also activates the {@link EnableFeatures} annotation and allows to
+     * choose which features to enable. All are disabled by default.

[Bernhard Bauer, 2017/03/03 11:54:38]

Ooh, there is one thing I had been thinking of: would it be possible / worth it
to use a Map<String, Boolean> for feature state? The idea being that tests would
explicitly set any feature to enabled or disabled that is checked by the code
under test, and the code would assert if an unknown feature is checked (as
opposed to now, where all features get disabled as soon as you add the rule). We
could do that in a followup though.

[dgn, 2017/03/03 13:48:30]

Yes we could do that. But I think it becomes interesting only when we define
default states for features. Otherwise turning on only what you need is easy
enough for unit tests, considering that we mock most things. Which features do
you think would benefit most? I'll do it as follow up, yes.

[Bernhard Bauer, 2017/03/03 14:01:22]

Well, what I'm a bit worried about is if we make a change that makes our code
check a feature it didn't check before. We will just default to off, which may
not match the real-world behavior if the feature in Chrome defaults to on, so we
would avoid catching a hypothetical bug that only happens if the feature is on.
If we require the feature to be set, we would at least have to explicitly define
the feature state we're assuming.

[dgn, 2017/03/03 15:53:45]

Okay got it. In that case, I reverted the change that makes the rule have some
effect by default, since it would not be useful for that later state. I just
added the current empty annotation at the class level.

      */
-    public static class Processor extends AnnotationProcessor<EnableFeatures> {
+    class Processor extends AnnotationProcessor<EnableFeatures> {
         public Processor() {
-            super(EnableFeatures.class);
+            super(EnableFeatures.class, true);
         }
 
         @Override
         protected void before() throws Throwable {
-            Set<String> enabledFeatures = new HashSet<>();
-            String[] values = getAnnotation().value();
-            enabledFeatures.addAll(Arrays.asList(values));
+            Set<String> enabledFeatures;
+
+            if (getAnnotation() != null) {
+                String[] values = getAnnotation().value();
+                enabledFeatures = new HashSet<>(Arrays.asList(values));
+            } else {
+                enabledFeatures = Collections.emptySet();
+            }
+
             ChromeFeatureList.setTestEnabledFeatures(enabledFeatures);
         }
 
         @Override
         protected void after() {
             ChromeFeatureList.setTestEnabledFeatures(null);
         }
     }
 }