From 0a8fc3936802379c756b39e7353425b1b4c6eb00 Mon Sep 17 00:00:00 2001 From: Christian Dullweber Date: Thu, 28 Jan 2021 18:35:36 +0000 Subject: [PATCH] Split grouped tests by Feature annotation Feature flags in instrumentation tests only change when Chrome is restarted. To allow to run tests in Batches that have methods with @Features annotation, split tests into groups based on enabled/disabled Features by adding a Batch.SplitByFeature annotation. Bug: 1170735 Change-Id: Id90d704bcd949eb8630172ed1051cb3fdfc5435f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2645136 Reviewed-by: Yaron Friedman Reviewed-by: Michael Thiessen Reviewed-by: benjamin joyce Reviewed-by: Rohit Agarwal Commit-Queue: Christian Dullweber Cr-Commit-Position: refs/heads/master@{#848181} --- .../src/org/chromium/base/test/util/Batch.java | 8 ++++++++ .../local_device_instrumentation_test_run.py | 18 +++++++++++++++--- .../browser/bookmarks/BookmarkBridgeTest.java | 3 +-- docs/testing/batching_instrumentation_tests.md | 4 ++++ 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/base/test/android/javatests/src/org/chromium/base/test/util/Batch.java b/base/test/android/javatests/src/org/chromium/base/test/util/Batch.java index 17425613d91d93..ce2ab4487e3d43 100644 --- a/base/test/android/javatests/src/org/chromium/base/test/util/Batch.java +++ b/base/test/android/javatests/src/org/chromium/base/test/util/Batch.java @@ -29,6 +29,14 @@ @Target(ElementType.TYPE) @Retention(RetentionPolicy.RUNTIME) public @interface Batch { + /** + * This annotation can be added in addition to @Batch to split batches based on @Features + * annotation. This will ensure that native features are configured correctly. + */ + @Target(ElementType.TYPE) + @Retention(RetentionPolicy.RUNTIME) + public @interface SplitByFeature {} + public String value(); /** diff --git a/build/android/pylib/local/device/local_device_instrumentation_test_run.py b/build/android/pylib/local/device/local_device_instrumentation_test_run.py index 69bce58ec63820..8bd5a000b0367b 100644 --- a/build/android/pylib/local/device/local_device_instrumentation_test_run.py +++ b/build/android/pylib/local/device/local_device_instrumentation_test_run.py @@ -480,11 +480,23 @@ def _GroupTests(self, tests): batched_tests = dict() other_tests = [] for test in tests: - if 'Batch' in test['annotations'] and 'RequiresRestart' not in test[ - 'annotations']: - batch_name = test['annotations']['Batch']['value'] + annotations = test['annotations'] + if 'Batch' in annotations and 'RequiresRestart' not in annotations: + batch_name = annotations['Batch']['value'] if not batch_name: batch_name = test['class'] + + # Feature flags won't work in instrumentation tests unless the activity + # is restarted. + # Tests with identical features are grouped to minimize restarts. + if 'Batch$SplitByFeature' in annotations: + if 'Features$EnableFeatures' in annotations: + batch_name += '|enabled:' + ','.join( + sorted(annotations['Features$EnableFeatures']['value'])) + if 'Features$DisableFeatures' in annotations: + batch_name += '|disabled:' + ','.join( + sorted(annotations['Features$DisableFeatures']['value'])) + if not batch_name in batched_tests: batched_tests[batch_name] = [] batched_tests[batch_name].append(test) diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/bookmarks/BookmarkBridgeTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/bookmarks/BookmarkBridgeTest.java index 52d11c97e17b9f..62cb0988fd5887 100644 --- a/chrome/android/javatests/src/org/chromium/chrome/browser/bookmarks/BookmarkBridgeTest.java +++ b/chrome/android/javatests/src/org/chromium/chrome/browser/bookmarks/BookmarkBridgeTest.java @@ -17,7 +17,6 @@ import org.chromium.base.test.UiThreadTest; import org.chromium.base.test.util.Batch; import org.chromium.base.test.util.Feature; -import org.chromium.base.test.util.RequiresRestart; import org.chromium.chrome.browser.bookmarks.BookmarkBridge.BookmarkItem; import org.chromium.chrome.browser.flags.ChromeFeatureList; import org.chromium.chrome.browser.profiles.Profile; @@ -39,6 +38,7 @@ */ @RunWith(BaseJUnit4ClassRunner.class) @Batch(Batch.PER_CLASS) +@Batch.SplitByFeature public class BookmarkBridgeTest { @Rule public final ChromeBrowserTestRule mChromeBrowserTestRule = new ChromeBrowserTestRule(); @@ -332,7 +332,6 @@ public void testSearch_MaxResults() { @SmallTest @UiThreadTest @Features.EnableFeatures({ChromeFeatureList.READ_LATER}) - @RequiresRestart public void testAddToReadingList() { Assert.assertNull("Should return null for non http/https URLs.", mBookmarkBridge.addToReadingList("a", new GURL("chrome://flags"))); diff --git a/docs/testing/batching_instrumentation_tests.md b/docs/testing/batching_instrumentation_tests.md index ec96a3934a48bd..1c9ea9e34207d7 100644 --- a/docs/testing/batching_instrumentation_tests.md +++ b/docs/testing/batching_instrumentation_tests.md @@ -61,6 +61,10 @@ will run the suite in its own batch. This will reduce the complexity of managing and leaking state from these tests as you only have to think about tests within the suite. For smaller and less complex test suites, see Custom below. +If you use different @Features annotations on test methods, you can use the +@Batch.SplitByFeature annotation to run tests with different features in +separate batches. + ### Custom This batching type is best for smaller and less complex test suites, that