🔬 Improve @ChromeHome usability in unit and instrumentation tests

- Introduces @ChromeHome.Enable and @ChromeHome.Disable
- @ChromeHome.Enable automatically causes the test test to be skipped
  when running on a tablet
- @ChromeHome now works for unit tests, ensures the cached state is
  reset between tests.

The plain @ChromeHome should eventually be removed in favour of the
dedicated annotations, that will happen in a subsequent cleanup patch.

Bug: 783160,754778
Change-Id: I60c69c2fb97a08aa1d0bcf533a8378cbbc1a548b
Reviewed-on: https://chromium-review.googlesource.com/796672
Commit-Queue: Nicolas Dossou-Gbété <dgn@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521664}

chrome/android/javatests/src/org/chromium/chrome/browser/FeaturesAnnotationsTest.java

@@ -9,6 +9,7 @@ import static junit.framework.Assert.assertTrue;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.core.IsCollectionContaining.hasItems;
+import static org.junit.Assert.assertFalse;
 
 import android.support.test.filters.SmallTest;
 
@@ -20,12 +21,14 @@ import org.junit.runner.RunWith;
 import org.chromium.base.CommandLine;
 import org.chromium.base.test.util.CommandLineFlags;
 import org.chromium.base.test.util.Restriction;
+import org.chromium.chrome.browser.util.FeatureUtilities;
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
 import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
 import org.chromium.chrome.test.util.browser.ChromeHome;
 import org.chromium.chrome.test.util.browser.Features;
 import org.chromium.chrome.test.util.browser.Features.DisableFeatures;
 import org.chromium.chrome.test.util.browser.Features.EnableFeatures;
+import org.chromium.ui.base.DeviceFormFactor;
 import org.chromium.ui.test.util.UiRestriction;
 
 import java.util.Arrays;
@@ -121,6 +124,14 @@ public class FeaturesAnnotationsTest {
         assertThat(finalEnabledList.size(), equalTo(4));
     }
 
+    @Test
+    @SmallTest
+    @ChromeHome.Enable
+    public void testChromeHomeSkipping() {
+        assertFalse("The test should only run on phones.", DeviceFormFactor.isTablet());
+        assertTrue("ChromeHome should be enabled.", FeatureUtilities.isChromeHomeEnabled());
+    }
+
     private static List<String> getArgsList(boolean enabled) {
         String switchName = enabled ? "enable-features" : "disable-features";
         return Arrays.asList(CommandLine.getInstance().getSwitchValue(switchName).split(","));

chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSectionTest.java

@@ -45,7 +45,6 @@ import org.chromium.base.Callback;
 import org.chromium.base.ContextUtils;
 import org.chromium.base.metrics.RecordUserAction;
 import org.chromium.base.test.util.Feature;
-import org.chromium.chrome.browser.ChromeFeatureList;
 import org.chromium.chrome.browser.DisableHistogramsRule;
 import org.chromium.chrome.browser.ntp.cards.NewTabPageViewHolder.PartialBindCallback;
 import org.chromium.chrome.browser.ntp.snippets.CategoryStatus;
@@ -58,8 +57,8 @@ import org.chromium.chrome.browser.suggestions.SuggestionsNavigationDelegate;
 import org.chromium.chrome.browser.suggestions.SuggestionsRanker;
 import org.chromium.chrome.browser.suggestions.SuggestionsUiDelegate;
 import org.chromium.chrome.browser.util.FeatureUtilities;
+import org.chromium.chrome.test.util.browser.ChromeHome;
 import org.chromium.chrome.test.util.browser.Features;
-import org.chromium.chrome.test.util.browser.Features.DisableFeatures;
 import org.chromium.chrome.test.util.browser.offlinepages.FakeOfflinePageBridge;
 import org.chromium.chrome.test.util.browser.suggestions.ContentSuggestionsTestUtils.CategoryInfoBuilder;
 import org.chromium.chrome.test.util.browser.suggestions.FakeSuggestionsSource;
@@ -79,8 +78,8 @@ import java.util.TreeSet;
  */
 @RunWith(LocalRobolectricTestRunner.class)
 @Config(manifest = Config.NONE)
-// TODO(bauerb): Enable these tests with chrome home enabled.
-@DisableFeatures(ChromeFeatureList.CHROME_HOME)
+// TODO(https://crbug.com/673092): Run as much as possible parameterized on ChromeHome state.
+@ChromeHome.Enable
 public class SuggestionsSectionTest {
     @Rule
     public DisableHistogramsRule mDisableHistogramsRule = new DisableHistogramsRule();
@@ -88,6 +87,9 @@ public class SuggestionsSectionTest {
     @Rule
     public TestRule mFeatureProcessor = new Features.JUnitProcessor();
 
+    @Rule
+    public TestRule mChromeHomeRule = new ChromeHome.Processor();
+
     private static final int TEST_CATEGORY_ID = 42;
 
     private static final int REMOTE_TEST_CATEGORY =
@@ -104,11 +106,15 @@ public class SuggestionsSectionTest {
 
     private FakeOfflinePageBridge mBridge;
 
+    public SuggestionsSectionTest() {
+        // The ChromeHome.Processor rule needs an available context when it is applied.
+        ContextUtils.initApplicationContextForTests(RuntimeEnvironment.application);
+    }
+
     @Before
     public void setUp() {
         RecordUserAction.setDisabledForTests(true);
         MockitoAnnotations.initMocks(this);
-        ContextUtils.initApplicationContextForTests(RuntimeEnvironment.application);
         FeatureUtilities.resetChromeHomeEnabledForTests();
 
         mBridge = new FakeOfflinePageBridge();
@@ -132,6 +138,7 @@ public class SuggestionsSectionTest {
 
     @Test
     @Feature({"Ntp"})
+    @ChromeHome.Disable // Sections can't be dismissed in ChromeHome
     public void testDismissSibling() {
         List<SnippetArticle> snippets = createDummySuggestions(3, TEST_CATEGORY_ID);
         SuggestionsSection section = createSectionWithFetchAction(true);
@@ -158,6 +165,7 @@ public class SuggestionsSectionTest {
 
     @Test
     @Feature({"Ntp"})
+    @ChromeHome.Disable // Sections can't be dismissed in ChromeHome
     public void testGetDismissalGroupWithoutHeader() {
         SuggestionsSection section = createSectionWithFetchAction(true);
         section.setHeaderVisibility(false);
@@ -171,6 +179,7 @@ public class SuggestionsSectionTest {
 
     @Test
     @Feature({"Ntp"})
+    @ChromeHome.Disable // Sections can't be dismissed in ChromeHome
     public void testGetDismissalGroupWithoutAction() {
         SuggestionsSection section = createSectionWithFetchAction(false);
 
@@ -180,6 +189,7 @@ public class SuggestionsSectionTest {
 
     @Test
     @Feature({"Ntp"})
+    @ChromeHome.Disable // Sections can't be dismissed in ChromeHome
     public void testGetDismissalGroupActionAndHeader() {
         SuggestionsSection section = createSectionWithFetchAction(false);
         section.setHeaderVisibility(false);
@@ -190,6 +200,7 @@ public class SuggestionsSectionTest {
 
     @Test
     @Feature({"Ntp"})
+    @ChromeHome.Disable // Assumes status card on empty state, irrelevant in ChromeHome.
     public void testAddSuggestionsNotification() {
         final int suggestionCount = 5;
         List<SnippetArticle> snippets = createDummySuggestions(suggestionCount,
@@ -212,6 +223,7 @@ public class SuggestionsSectionTest {
 
     @Test
     @Feature({"Ntp"})
+    @ChromeHome.Disable // Assumes status card on empty state, irrelevant in ChromeHome.
     public void testSetStatusNotification() {
         final int suggestionCount = 5;
         List<SnippetArticle> snippets = createDummySuggestions(suggestionCount,
@@ -252,6 +264,7 @@ public class SuggestionsSectionTest {
 
     @Test
     @Feature({"Ntp"})
+    @ChromeHome.Disable // Assumes status card on empty state, irrelevant in ChromeHome.
     public void testRemoveSuggestionNotification() {
         final int suggestionCount = 2;
         List<SnippetArticle> snippets = createDummySuggestions(suggestionCount,
@@ -277,6 +290,7 @@ public class SuggestionsSectionTest {
 
     @Test
     @Feature({"Ntp"})
+    @ChromeHome.Disable // Assumes status card on empty state, irrelevant in ChromeHome.
     public void testRemoveSuggestionNotificationWithButton() {
         final int suggestionCount = 2;
         List<SnippetArticle> snippets = createDummySuggestions(suggestionCount,
@@ -308,6 +322,7 @@ public class SuggestionsSectionTest {
 
     @Test
     @Feature({"Ntp"})
+    @ChromeHome.Disable // Sections can't be dismissed in ChromeHome
     public void testDismissSection() {
         SuggestionsSection section = createSectionWithFetchAction(false);
         section.setStatus(CategoryStatus.AVAILABLE);
@@ -874,6 +889,7 @@ public class SuggestionsSectionTest {
 
     @Test
     @Feature({"Ntp"})
+    @ChromeHome.Disable // Assumes paired status card and action item, irrelevant in ChromeHome.
     public void testGetItemDismissalGroupWithActionItem() {
         SuggestionsSection section = createSectionWithFetchAction(true);
         assertThat(section.getItemDismissalGroup(1).size(), is(2));
@@ -882,6 +898,7 @@ public class SuggestionsSectionTest {
 
     @Test
     @Feature({"Ntp"})
+    @ChromeHome.Disable // Sections can't be dismissed in ChromeHome
     public void testGetItemDismissalGroupWithoutActionItem() {
         SuggestionsSection section = createSectionWithFetchAction(false);
         assertThat(section.getItemDismissalGroup(1).size(), is(1));

chrome/android/junit/src/org/chromium/chrome/browser/suggestions/TileGroupUnitTest.java

@@ -49,6 +49,7 @@ import org.chromium.chrome.browser.ntp.cards.CardsVariationParameters;
 import org.chromium.chrome.browser.offlinepages.OfflinePageBridge;
 import org.chromium.chrome.browser.suggestions.TileView.Style;
 import org.chromium.chrome.browser.widget.displaystyle.UiConfig;
+import org.chromium.chrome.test.util.browser.ChromeHome;
 import org.chromium.chrome.test.util.browser.Features;
 import org.chromium.chrome.test.util.browser.Features.DisableFeatures;
 import org.chromium.chrome.test.util.browser.Features.EnableFeatures;
@@ -64,8 +65,9 @@ import java.util.List;
  */
 @RunWith(LocalRobolectricTestRunner.class)
 @Config(manifest = Config.NONE)
-@EnableFeatures({ChromeFeatureList.NTP_OFFLINE_PAGES_FEATURE_NAME, ChromeFeatureList.CHROME_HOME})
+@EnableFeatures(ChromeFeatureList.NTP_OFFLINE_PAGES_FEATURE_NAME)
 @DisableFeatures({ChromeFeatureList.NTP_MODERN_LAYOUT})
+@ChromeHome.Enable
 public class TileGroupUnitTest {
     private static final int MAX_TILES_TO_FETCH = 4;
     private static final int TILE_TITLE_LINES = 1;
@@ -73,6 +75,8 @@ public class TileGroupUnitTest {
 
     @Rule
     public TestRule mFeaturesProcessor = new Features.JUnitProcessor();
+    @Rule
+    public TestRule mChromeHomeProcessor = new ChromeHome.Processor();
 
     @Mock
     private TileGroup.Observer mTileGroupObserver;
@@ -83,11 +87,15 @@ public class TileGroupUnitTest {
     private FakeImageFetcher mImageFetcher;
     private TileRenderer mTileRenderer;
 
+    public TileGroupUnitTest() {
+        // The ChromeHome.Processor rule needs an available context when it is applied.
+        ContextUtils.initApplicationContextForTests(RuntimeEnvironment.application);
+    }
+
     @Before
     public void setUp() {
         CardsVariationParameters.setTestVariationParams(new HashMap<>());
         MockitoAnnotations.initMocks(this);
-        ContextUtils.initApplicationContextForTests(RuntimeEnvironment.application);
 
         mImageFetcher = new FakeImageFetcher();
         mTileRenderer = new TileRenderer(

chrome/test/android/javatests/src/org/chromium/chrome/test/util/browser/ChromeHome.java

@@ -8,14 +8,15 @@ import static junit.framework.Assert.assertEquals;
 import static junit.framework.Assert.assertNotNull;
 
 import android.os.StrictMode;
-import android.text.TextUtils;
 
-import org.chromium.base.CommandLine;
 import org.chromium.base.test.util.AnnotationRule;
+import org.chromium.base.test.util.Restriction;
 import org.chromium.chrome.browser.ChromeFeatureList;
 import org.chromium.chrome.browser.preferences.ChromePreferenceManager;
 import org.chromium.chrome.browser.util.FeatureUtilities;
+import org.chromium.ui.test.util.UiRestriction;
 
+import java.lang.annotation.Annotation;
 import java.lang.annotation.ElementType;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
@@ -31,32 +32,49 @@ import java.lang.annotation.Target;
 @Retention(RetentionPolicy.RUNTIME)
 @Target({ElementType.METHOD, ElementType.TYPE})
 public @interface ChromeHome {
+    @Retention(RetentionPolicy.RUNTIME)
+    @Target({ElementType.METHOD, ElementType.TYPE})
+    @Restriction(UiRestriction.RESTRICTION_TYPE_PHONE)
+    @Features.EnableFeatures(ChromeFeatureList.CHROME_HOME)
+    @interface Enable {}
+
+    @Retention(RetentionPolicy.RUNTIME)
+    @Target({ElementType.METHOD, ElementType.TYPE})
+    @Features.DisableFeatures(ChromeFeatureList.CHROME_HOME)
+    @interface Disable {}
+
     boolean value() default true;
 
     String FEATURES = ChromeFeatureList.CHROME_HOME;
     String ENABLE_FLAGS = "enable-features=" + FEATURES;
 
     /**
-     * Rule to Handle setting and resetting the feature flag and preference for ChromeHome. Can be
-     * used by explicitly calling methods ({@link #setPrefs(boolean)} and {@link #clearTestState()})
-     * or by using the {@link ChromeHome} annotation on tests.
+     * Rule to handle setting and resetting the cached feature state for ChromeHome. Can be used by
+     * explicitly calling methods ({@link #setPrefs(boolean)} and {@link #clearTestState()}) or by
+     * using the {@link ChromeHome.Enable} and {@link ChromeHome.Disable} annotations on tests.
      */
     class Processor extends AnnotationRule {
         private Boolean mOldState;
 
         public Processor() {
-            super(ChromeHome.class);
+            super(ChromeHome.class, ChromeHome.Enable.class, ChromeHome.Disable.class);
         }
 
         @Override
         protected void before() throws Throwable {
-            boolean enabled = getAnnotation(ChromeHome.class).value();
-            if (enabled) {
+            boolean featureEnabled;
+            Annotation annotation = getClosestAnnotation();
+            if (annotation instanceof ChromeHome) {
+                featureEnabled = ((ChromeHome) annotation).value();
+                if (featureEnabled) {
                     Features.getInstance().enable(ChromeFeatureList.CHROME_HOME);
                 } else {
                     Features.getInstance().disable(ChromeFeatureList.CHROME_HOME);
                 }
-            setPrefs(enabled);
+            } else {
+                featureEnabled = annotation instanceof ChromeHome.Enable;
+            }
+            setPrefs(featureEnabled);
         }
 
         @Override
@@ -89,22 +107,5 @@ public @interface ChromeHome {
             assertNotNull(mOldState);
             ChromePreferenceManager.getInstance().setChromeHomeEnabled(mOldState);
         }
-
-        private void updateCommandLine() {
-            // TODO(dgn): Possible extra work: detect the flag and combine them with existing ones
-            // or support explicitly disabling the feature. Ideally this will be done by refactoring
-            // the entire features setting system for tests instead or string manipulation.
-            CommandLine commandLine = CommandLine.getInstance();
-            String existingFeatures = commandLine.getSwitchValue("enable-features");
-
-            if (TextUtils.equals(existingFeatures, FEATURES)) return;
-
-            if (existingFeatures != null) {
-                throw new IllegalStateException("Unable to enable ChromeHome, the feature flag is"
-                        + " already set to " + existingFeatures);
-            }
-
-            commandLine.appendSwitch(ENABLE_FLAGS);
-        }
     }
 }