[AW] batch non-embedded services tests

Here is a per-class breakdown:
- CrashReceiverServiceTest: batch under UNIT_TESTS, all existing tests
    are unit-tests-like, they don't launch any services or any UI.
- MetricsBridgeServiceTest: two tests don't actually launch any service
    or UI so they are moved to a separate class and batched as
    UNIT_TESTS, remaining two tests are marked best left unbatched
    because they bind and unbind to a service and depend on having a
    fresh instance of that service every time, restarting the process
    makes sure they start with a clean state.
- MinidumpUploadJobTest: These tests have n TestRule that launches a
    browser process and loads native library, it's safer to leave them
    unbatched.
- VariationsSeedServerTest: Tests launches (bind/unbind) to service,
    leaving them unbatched makes sure that the service will be killed
    after each test leaving a clean state.
- VisualStateCallbackTest: It has only one test that depends on
    multiprocess mode and it should remain unbatched.

Bug: 1120847
Test: run_webview_instrumentation_test_apk -f "*.android_webview.test.services*"
Change-Id: I7dbf0d4d5573eb9865f612d1c087627b705c2b19
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2440658
Auto-Submit: Hazem Ashmawy <hazems@chromium.org>
Commit-Queue: Changwan Ryu <changwan@chromium.org>
Reviewed-by: Changwan Ryu <changwan@chromium.org>
Reviewed-by: Nate Fischer <ntfschr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813781}

android_webview/javatests/src/org/chromium/android_webview/test/services/CrashReceiverServiceTest.java

@@ -22,6 +22,7 @@ import org.chromium.android_webview.common.crash.SystemWideCrashDirectories;
 import org.chromium.android_webview.services.CrashReceiverService;
 import org.chromium.android_webview.test.AwJUnit4ClassRunner;
 import org.chromium.android_webview.test.OnlyRunIn;
+import org.chromium.base.test.util.Batch;
 
 import java.io.File;
 import java.io.FileInputStream;
@@ -30,10 +31,12 @@ import java.util.HashMap;
 import java.util.Map;
 
 /**
- * Instrumentation tests for CrashReceiverService.
+ * Instrumentation tests for CrashReceiverService. These tests are batched as UNIT_TESTS because
+ * they don't actually launch any services or other components.
  */
 @RunWith(AwJUnit4ClassRunner.class)
 @OnlyRunIn(SINGLE_PROCESS)
+@Batch(Batch.UNIT_TESTS)
 public class CrashReceiverServiceTest {
     private static final String TEST_CRASH_LOCAL_ID = "abc1234";
     private static final String TEST_CRASH_FILE_NAME =

android_webview/javatests/src/org/chromium/android_webview/test/services/MetricsBridgeServiceTest.java

@@ -5,16 +5,14 @@
 package org.chromium.android_webview.test.services;
 
 import static org.chromium.android_webview.test.OnlyRunIn.ProcessMode.SINGLE_PROCESS;
+import static org.chromium.android_webview.test.services.MetricsBridgeServiceUnitTest.RETRIEVE_METRICS_TASK_STATUS_SUCCESS_RECORD;
 
 import android.content.Context;
 import android.content.Intent;
-import android.os.IBinder;
 
 import androidx.test.filters.MediumTest;
 
-import org.junit.After;
 import org.junit.Assert;
-import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
@@ -22,152 +20,19 @@ import org.chromium.android_webview.common.services.IMetricsBridgeService;
 import org.chromium.android_webview.proto.MetricsBridgeRecords.HistogramRecord;
 import org.chromium.android_webview.proto.MetricsBridgeRecords.HistogramRecord.RecordType;
 import org.chromium.android_webview.services.MetricsBridgeService;
-import org.chromium.android_webview.test.AwActivityTestRule;
 import org.chromium.android_webview.test.AwJUnit4ClassRunner;
 import org.chromium.android_webview.test.OnlyRunIn;
 import org.chromium.base.ContextUtils;
-import org.chromium.base.FileUtils;
 
-import java.io.ByteArrayOutputStream;
-import java.io.File;
-import java.io.FileInputStream;
-import java.io.FileOutputStream;
-import java.io.IOException;
-import java.io.OutputStream;
 import java.util.List;
-import java.util.concurrent.FutureTask;
 
 /**
- * Test MetricsBridgeService.
+ * Instrumentation tests MetricsBridgeService. These tests are not batched to make sure all unbinded
+ * services are properly killed between tests.
  */
 @RunWith(AwJUnit4ClassRunner.class)
 @OnlyRunIn(SINGLE_PROCESS)
 public class MetricsBridgeServiceTest {
-    private static final byte[] PARSING_LOG_RESULT_SUCCESS_RECORD =
-            HistogramRecord.newBuilder()
-                    .setRecordType(RecordType.HISTOGRAM_LINEAR)
-                    .setHistogramName("Android.WebView.NonEmbeddedMetrics.ParsingLogResult")
-                    .setSample(MetricsBridgeService.ParsingLogResult.SUCCESS)
-                    .setMin(1)
-                    .setMax(MetricsBridgeService.ParsingLogResult.COUNT)
-                    .setNumBuckets(MetricsBridgeService.ParsingLogResult.COUNT + 1)
-                    .build()
-                    .toByteArray();
-
-    private static final byte[] RETRIEVE_METRICS_TASK_STATUS_SUCCESS_RECORD =
-            HistogramRecord.newBuilder()
-                    .setRecordType(RecordType.HISTOGRAM_LINEAR)
-                    .setHistogramName(
-                            "Android.WebView.NonEmbeddedMetrics.RetrieveMetricsTaskStatus")
-                    .setSample(MetricsBridgeService.RetrieveMetricsTaskStatus.SUCCESS)
-                    .setMin(1)
-                    .setMax(MetricsBridgeService.RetrieveMetricsTaskStatus.COUNT)
-                    .setNumBuckets(MetricsBridgeService.RetrieveMetricsTaskStatus.COUNT + 1)
-                    .build()
-                    .toByteArray();
-
-    private File mTempFile;
-
-    @Before
-    public void setUp() throws IOException {
-        mTempFile = File.createTempFile("test_webview_metrics_bridge_logs", null);
-    }
-
-    @After
-    public void tearDown() {
-        if (mTempFile.exists()) {
-            Assert.assertTrue("Failed to delete \"" + mTempFile + "\"", mTempFile.delete());
-        }
-    }
-
-    @Test
-    @MediumTest
-    // Test that the service saves metrics records to file
-    public void testSaveToFile() throws Throwable {
-        HistogramRecord recordBooleanProto = HistogramRecord.newBuilder()
-                                                     .setRecordType(RecordType.HISTOGRAM_BOOLEAN)
-                                                     .setHistogramName("testSaveToFile.boolean")
-                                                     .setSample(1)
-                                                     .build();
-        HistogramRecord recordLinearProto = HistogramRecord.newBuilder()
-                                                    .setRecordType(RecordType.HISTOGRAM_LINEAR)
-                                                    .setHistogramName("testSaveToFile.linear")
-                                                    .setSample(123)
-                                                    .setMin(1)
-                                                    .setMax(1000)
-                                                    .setNumBuckets(50)
-                                                    .build();
-        ByteArrayOutputStream out = new ByteArrayOutputStream();
-        writeRecordsToStream(out, recordBooleanProto, recordLinearProto, recordBooleanProto);
-        byte[] expectedData = out.toByteArray();
-
-        // Cannot bind to service using real connection since we need to inject test file name.
-        MetricsBridgeService service = new MetricsBridgeService(mTempFile);
-        // Simulate starting the service by calling onCreate()
-        service.onCreate();
-
-        IBinder binder = service.onBind(null);
-        IMetricsBridgeService stub = IMetricsBridgeService.Stub.asInterface(binder);
-        stub.recordMetrics(recordBooleanProto.toByteArray());
-        stub.recordMetrics(recordLinearProto.toByteArray());
-        stub.recordMetrics(recordBooleanProto.toByteArray());
-
-        // Block until all tasks are finished to make sure all records are written to file.
-        FutureTask<Object> blockTask = service.addTaskToBlock();
-        AwActivityTestRule.waitForFuture(blockTask);
-
-        byte[] resultData = FileUtils.readStream(new FileInputStream(mTempFile));
-        Assert.assertArrayEquals(
-                "byte data from file is different from the expected proto byte data", expectedData,
-                resultData);
-    }
-
-    @Test
-    @MediumTest
-    // Test that service recovers saved data from file, appends new records to it and
-    // clears the file after a retrieve call.
-    public void testRetrieveFromFile() throws Throwable {
-        HistogramRecord recordBooleanProto =
-                HistogramRecord.newBuilder()
-                        .setRecordType(RecordType.HISTOGRAM_BOOLEAN)
-                        .setHistogramName("testRecoverFromFile.boolean")
-                        .setSample(1)
-                        .build();
-        HistogramRecord recordLinearProto = HistogramRecord.newBuilder()
-                                                    .setRecordType(RecordType.HISTOGRAM_LINEAR)
-                                                    .setHistogramName("testRecoverFromFile.linear")
-                                                    .setSample(123)
-                                                    .setMin(1)
-                                                    .setMax(1000)
-                                                    .setNumBuckets(50)
-                                                    .build();
-        // write Initial proto data To File
-        writeRecordsToStream(new FileOutputStream(mTempFile), recordBooleanProto, recordLinearProto,
-                recordBooleanProto);
-
-        // Cannot bind to service using real connection since we need to inject test file name.
-        MetricsBridgeService service = new MetricsBridgeService(mTempFile);
-        // Simulate starting the service by calling onCreate()
-        service.onCreate();
-
-        IBinder binder = service.onBind(null);
-        IMetricsBridgeService stub = IMetricsBridgeService.Stub.asInterface(binder);
-        stub.recordMetrics(recordBooleanProto.toByteArray());
-        List<byte[]> retrievedDataList = stub.retrieveNonembeddedMetrics();
-
-        byte[][] expectedData = new byte[][] {recordBooleanProto.toByteArray(),
-                recordLinearProto.toByteArray(), recordBooleanProto.toByteArray(),
-                PARSING_LOG_RESULT_SUCCESS_RECORD, recordBooleanProto.toByteArray(),
-                RETRIEVE_METRICS_TASK_STATUS_SUCCESS_RECORD};
-
-        // Assert file is deleted after the retrieve call
-        Assert.assertFalse(
-                "file should be deleted after retrieve metrics call", mTempFile.exists());
-        Assert.assertNotNull("retrieved byte data from the service is null", retrievedDataList);
-        Assert.assertArrayEquals("retrieved byte data is different from the expected data",
-                expectedData, retrievedDataList.toArray());
-    }
-
     @Test
     @MediumTest
     // Test sending data to the service and retrieving it back.
@@ -229,12 +94,4 @@ public class MetricsBridgeServiceTest {
                     retrievedDataList.toArray());
         }
     }
-
-    private static void writeRecordsToStream(OutputStream os, HistogramRecord... records)
-            throws IOException {
-        for (HistogramRecord record : records) {
-            record.writeDelimitedTo(os);
-        }
-        os.close();
-    }
 }
\ No newline at end of file

android_webview/javatests/src/org/chromium/android_webview/test/services/MetricsBridgeServiceUnitTest.java

+// Copyright 2020 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.android_webview.test.services;
+
+import static org.chromium.android_webview.test.OnlyRunIn.ProcessMode.SINGLE_PROCESS;
+
+import android.os.IBinder;
+
+import androidx.test.filters.MediumTest;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.chromium.android_webview.common.services.IMetricsBridgeService;
+import org.chromium.android_webview.proto.MetricsBridgeRecords.HistogramRecord;
+import org.chromium.android_webview.proto.MetricsBridgeRecords.HistogramRecord.RecordType;
+import org.chromium.android_webview.services.MetricsBridgeService;
+import org.chromium.android_webview.test.AwActivityTestRule;
+import org.chromium.android_webview.test.AwJUnit4ClassRunner;
+import org.chromium.android_webview.test.OnlyRunIn;
+import org.chromium.base.FileUtils;
+import org.chromium.base.test.util.Batch;
+
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.util.List;
+import java.util.concurrent.FutureTask;
+
+/**
+ * Instrumentation tests for MetricsBridgeService. These tests are batched as UNIT_TESTS because
+ * they don't actually launch any services or other components.
+ */
+@RunWith(AwJUnit4ClassRunner.class)
+@OnlyRunIn(SINGLE_PROCESS)
+@Batch(Batch.UNIT_TESTS)
+public class MetricsBridgeServiceUnitTest {
+    public static final byte[] PARSING_LOG_RESULT_SUCCESS_RECORD =
+            HistogramRecord.newBuilder()
+                    .setRecordType(RecordType.HISTOGRAM_LINEAR)
+                    .setHistogramName("Android.WebView.NonEmbeddedMetrics.ParsingLogResult")
+                    .setSample(MetricsBridgeService.ParsingLogResult.SUCCESS)
+                    .setMin(1)
+                    .setMax(MetricsBridgeService.ParsingLogResult.COUNT)
+                    .setNumBuckets(MetricsBridgeService.ParsingLogResult.COUNT + 1)
+                    .build()
+                    .toByteArray();
+
+    public static final byte[] RETRIEVE_METRICS_TASK_STATUS_SUCCESS_RECORD =
+            HistogramRecord.newBuilder()
+                    .setRecordType(RecordType.HISTOGRAM_LINEAR)
+                    .setHistogramName(
+                            "Android.WebView.NonEmbeddedMetrics.RetrieveMetricsTaskStatus")
+                    .setSample(MetricsBridgeService.RetrieveMetricsTaskStatus.SUCCESS)
+                    .setMin(1)
+                    .setMax(MetricsBridgeService.RetrieveMetricsTaskStatus.COUNT)
+                    .setNumBuckets(MetricsBridgeService.RetrieveMetricsTaskStatus.COUNT + 1)
+                    .build()
+                    .toByteArray();
+
+    private File mTempFile;
+
+    @Before
+    public void setUp() throws IOException {
+        mTempFile = File.createTempFile("test_webview_metrics_bridge_logs", null);
+    }
+
+    @After
+    public void tearDown() {
+        if (mTempFile.exists()) {
+            Assert.assertTrue("Failed to delete \"" + mTempFile + "\"", mTempFile.delete());
+        }
+    }
+
+    @Test
+    @MediumTest
+    // Test that the service saves metrics records to file
+    public void testSaveToFile() throws Throwable {
+        HistogramRecord recordBooleanProto = HistogramRecord.newBuilder()
+                                                     .setRecordType(RecordType.HISTOGRAM_BOOLEAN)
+                                                     .setHistogramName("testSaveToFile.boolean")
+                                                     .setSample(1)
+                                                     .build();
+        HistogramRecord recordLinearProto = HistogramRecord.newBuilder()
+                                                    .setRecordType(RecordType.HISTOGRAM_LINEAR)
+                                                    .setHistogramName("testSaveToFile.linear")
+                                                    .setSample(123)
+                                                    .setMin(1)
+                                                    .setMax(1000)
+                                                    .setNumBuckets(50)
+                                                    .build();
+        ByteArrayOutputStream out = new ByteArrayOutputStream();
+        writeRecordsToStream(out, recordBooleanProto, recordLinearProto, recordBooleanProto);
+        byte[] expectedData = out.toByteArray();
+
+        // Cannot bind to service using real connection since we need to inject test file name.
+        MetricsBridgeService service = new MetricsBridgeService(mTempFile);
+        // Simulate starting the service by calling onCreate()
+        service.onCreate();
+
+        IBinder binder = service.onBind(null);
+        IMetricsBridgeService stub = IMetricsBridgeService.Stub.asInterface(binder);
+        stub.recordMetrics(recordBooleanProto.toByteArray());
+        stub.recordMetrics(recordLinearProto.toByteArray());
+        stub.recordMetrics(recordBooleanProto.toByteArray());
+
+        // Block until all tasks are finished to make sure all records are written to file.
+        FutureTask<Object> blockTask = service.addTaskToBlock();
+        AwActivityTestRule.waitForFuture(blockTask);
+
+        byte[] resultData = FileUtils.readStream(new FileInputStream(mTempFile));
+        Assert.assertArrayEquals(
+                "byte data from file is different from the expected proto byte data", expectedData,
+                resultData);
+    }
+
+    @Test
+    @MediumTest
+    // Test that service recovers saved data from file, appends new records to it and
+    // clears the file after a retrieve call.
+    public void testRetrieveFromFile() throws Throwable {
+        HistogramRecord recordBooleanProto =
+                HistogramRecord.newBuilder()
+                        .setRecordType(RecordType.HISTOGRAM_BOOLEAN)
+                        .setHistogramName("testRecoverFromFile.boolean")
+                        .setSample(1)
+                        .build();
+        HistogramRecord recordLinearProto = HistogramRecord.newBuilder()
+                                                    .setRecordType(RecordType.HISTOGRAM_LINEAR)
+                                                    .setHistogramName("testRecoverFromFile.linear")
+                                                    .setSample(123)
+                                                    .setMin(1)
+                                                    .setMax(1000)
+                                                    .setNumBuckets(50)
+                                                    .build();
+        // write Initial proto data To File
+        writeRecordsToStream(new FileOutputStream(mTempFile), recordBooleanProto, recordLinearProto,
+                recordBooleanProto);
+
+        // Cannot bind to service using real connection since we need to inject test file name.
+        MetricsBridgeService service = new MetricsBridgeService(mTempFile);
+        // Simulate starting the service by calling onCreate()
+        service.onCreate();
+
+        IBinder binder = service.onBind(null);
+        IMetricsBridgeService stub = IMetricsBridgeService.Stub.asInterface(binder);
+        stub.recordMetrics(recordBooleanProto.toByteArray());
+        List<byte[]> retrievedDataList = stub.retrieveNonembeddedMetrics();
+
+        byte[][] expectedData = new byte[][] {recordBooleanProto.toByteArray(),
+                recordLinearProto.toByteArray(), recordBooleanProto.toByteArray(),
+                PARSING_LOG_RESULT_SUCCESS_RECORD, recordBooleanProto.toByteArray(),
+                RETRIEVE_METRICS_TASK_STATUS_SUCCESS_RECORD};
+
+        // Assert file is deleted after the retrieve call
+        Assert.assertFalse(
+                "file should be deleted after retrieve metrics call", mTempFile.exists());
+        Assert.assertNotNull("retrieved byte data from the service is null", retrievedDataList);
+        Assert.assertArrayEquals("retrieved byte data is different from the expected data",
+                expectedData, retrievedDataList.toArray());
+    }
+
+    private static void writeRecordsToStream(OutputStream os, HistogramRecord... records)
+            throws IOException {
+        for (HistogramRecord record : records) {
+            record.writeDelimitedTo(os);
+        }
+        os.close();
+    }
+}
\ No newline at end of file

android_webview/javatests/src/org/chromium/android_webview/test/services/MinidumpUploadJobTest.java

@@ -44,6 +44,9 @@ import java.io.IOException;
 /**
  * Instrumentation tests for WebView's implementation of MinidumpUploaderDelegate, and the
  * interoperability of WebView's minidump-copying and minidump-uploading logic.
+ *
+ * These tests loads native library and mark the process as a browser process, it's safer to
+ * leave them unbatched to avoid possible state leaking between tests.
  */
 @RunWith(AwJUnit4ClassRunner.class)
 @OnlyRunIn(SINGLE_PROCESS)

android_webview/javatests/src/org/chromium/android_webview/test/services/VariationsSeedServerTest.java

@@ -37,7 +37,8 @@ import java.io.IOException;
 import java.util.concurrent.TimeoutException;
 
 /**
- * Test VariationsSeedServer.
+ * Test VariationsSeedServer. These tests are not batched to make sure all unbinded services are
+ * properly killed between tests.
  */
 @RunWith(AwJUnit4ClassRunner.class)
 @OnlyRunIn(SINGLE_PROCESS)

android_webview/javatests/src/org/chromium/android_webview/test/services/VisualStateCallbackTest.java

@@ -38,7 +38,8 @@ import org.chromium.base.test.util.Feature;
 import org.chromium.content_public.browser.UiThreadTaskTraits;
 
 /**
- * Test VisualStateCallback when render process is gone.
+ * Test VisualStateCallback when render process is gone. Test is not batched because it tests
+ * behaviour in multiprocesses.
  */
 @RunWith(AwJUnit4ClassRunner.class)
 public class VisualStateCallbackTest {

android_webview/test/BUILD.gn

@@ -330,6 +330,7 @@ instrumentation_test_apk("webview_instrumentation_test_apk") {
     "../javatests/src/org/chromium/android_webview/test/devui/util/WebViewCrashLogParserTest.java",
     "../javatests/src/org/chromium/android_webview/test/services/CrashReceiverServiceTest.java",
     "../javatests/src/org/chromium/android_webview/test/services/MetricsBridgeServiceTest.java",
+    "../javatests/src/org/chromium/android_webview/test/services/MetricsBridgeServiceUnitTest.java",
     "../javatests/src/org/chromium/android_webview/test/services/MinidumpUploadJobTest.java",
     "../javatests/src/org/chromium/android_webview/test/services/MockMetricsBridgeService.java",
     "../javatests/src/org/chromium/android_webview/test/services/MockVariationsSeedServer.java",