Merge "Move setUpAdditionalArgs() into BaseMetricListener."android-s-v2-preview-2android-s-v2-preview-1android-s-v2-beta-2android-s-v2-preview-1

6 files changed, 65 insertions, 33 deletions

diff --git a/libraries/device-collectors/src/main/java/android/device/collectors/BaseCollectionListener.java b/libraries/device-collectors/src/main/java/android/device/collectors/BaseCollectionListener.java
index 709e87b65..a2837c2d1 100644
--- a/libraries/device-collectors/src/main/java/android/device/collectors/BaseCollectionListener.java
+++ b/libraries/device-collectors/src/main/java/android/device/collectors/BaseCollectionListener.java
@@ -67,9 +67,6 @@ public class BaseCollectionListener<T> extends BaseMetricListener {
         // By default this flag is set to false to collect the metrics on test failure.
         mSkipTestFailureMetrics = "true".equals(args.getString(SKIP_TEST_FAILURE_METRICS));
 
-        // Setup additional args before starting the collection.
-        setupAdditionalArgs();
-
         if (mIsCollectPerRun) {
             mHelper.startCollecting();
         }
@@ -113,14 +110,6 @@ public class BaseCollectionListener<T> extends BaseMetricListener {
         }
     }
 
-    /**
-     * To add listener specific extra args implement this method in the sub class and add the
-     * listener specific args.
-     */
-    public void setupAdditionalArgs() {
-        // NO-OP by default
-    }
-
     protected void createHelperInstance(ICollectorHelper helper) {
         mHelper = helper;
     }
diff --git a/libraries/device-collectors/src/main/java/android/device/collectors/BaseMetricListener.java b/libraries/device-collectors/src/main/java/android/device/collectors/BaseMetricListener.java
index eb83858c9..ae878aef3 100644
--- a/libraries/device-collectors/src/main/java/android/device/collectors/BaseMetricListener.java
+++ b/libraries/device-collectors/src/main/java/android/device/collectors/BaseMetricListener.java
@@ -119,6 +119,7 @@ public class BaseMetricListener extends InstrumentationRunListener {
     @Override
     public final void testRunStarted(Description description) throws Exception {
         parseArguments();
+        setupAdditionalArgs();
         if (!mLogOnly) {
             try {
                 mRunData = createDataRecord();
@@ -267,6 +268,14 @@ public class BaseMetricListener extends InstrumentationRunListener {
     }
 
     /**
+     * To add listener-specific extra args, implement this method in the sub class and add the
+     * listener specific args.
+     */
+    public void setupAdditionalArgs() {
+        // NO-OP by default
+    }
+
+    /**
      * Turn executeShellCommand into a blocking operation.
      *
      * @param command shell command to be executed.
diff --git a/libraries/device-collectors/src/main/java/android/device/collectors/GcaEventLogCollector.java b/libraries/device-collectors/src/main/java/android/device/collectors/GcaEventLogCollector.java
index 9a196de18..d72d5d663 100644
--- a/libraries/device-collectors/src/main/java/android/device/collectors/GcaEventLogCollector.java
+++ b/libraries/device-collectors/src/main/java/android/device/collectors/GcaEventLogCollector.java
@@ -96,7 +96,6 @@ public class GcaEventLogCollector extends BaseMetricListener {
 
     @Override
     public void onTestRunStart(DataRecord runData, Description description) {
-        setupAdditionalArgs();
         mDestDir = createAndEmptyDirectory(DEST_DIR);
         mEventLogPath = String.format(CAMERA_EVENT_LOG_PATTERN, mGcaPkg);
         cleanUpEventLog();
@@ -148,7 +147,8 @@ public class GcaEventLogCollector extends BaseMetricListener {
         mIsTestFailed = true;
     }
 
-    private void setupAdditionalArgs() {
+    @Override
+    public void setupAdditionalArgs() {
         Bundle args = getArgsBundle();
         if (!Strings.isNullOrEmpty(args.getString(GOOGLE_CAMERA_APP_PACKAGE))) {
             mGcaPkg = args.getString(GOOGLE_CAMERA_APP_PACKAGE);
diff --git a/libraries/device-collectors/src/main/java/android/device/collectors/LogcatCollector.java b/libraries/device-collectors/src/main/java/android/device/collectors/LogcatCollector.java
index 16ba7780e..92ce740e6 100644
--- a/libraries/device-collectors/src/main/java/android/device/collectors/LogcatCollector.java
+++ b/libraries/device-collectors/src/main/java/android/device/collectors/LogcatCollector.java
@@ -85,7 +85,6 @@ public class LogcatCollector extends BaseMetricListener {
 
     @Override
     public void onTestRunStart(DataRecord runData, Description description) {
-        setupAdditionalArgs();
         mDestDir = createAndEmptyDirectory(DEFAULT_DIR);
         // Capture the start time in case onTestStart() is never called due to failure during
         // @BeforeClass.
@@ -186,10 +185,9 @@ public class LogcatCollector extends BaseMetricListener {
         return DATE_FORMATTER.format(date);
     }
 
-    /**
-     * Add custom options if available.
-     */
-    private void setupAdditionalArgs() {
+    /** Add custom options if available. */
+    @Override
+    public void setupAdditionalArgs() {
         Bundle args = getArgsBundle();
 
         if (args.getString(BEFORE_LOGCAT_DURATION_SECS) != null) {
diff --git a/libraries/device-collectors/src/main/java/android/device/collectors/ScheduledRunCollectionListener.java b/libraries/device-collectors/src/main/java/android/device/collectors/ScheduledRunCollectionListener.java
index ce319c326..60ba421ab 100644
--- a/libraries/device-collectors/src/main/java/android/device/collectors/ScheduledRunCollectionListener.java
+++ b/libraries/device-collectors/src/main/java/android/device/collectors/ScheduledRunCollectionListener.java
@@ -169,7 +169,6 @@ public class ScheduledRunCollectionListener<T extends Number> extends ScheduledR
     /** {@inheritDoc} */
     @Override
     void onStart(DataRecord runData, Description description) {
-        setupAdditionalArgs();
         Path path =
                 Paths.get(
                         OUTPUT_ROOT,
@@ -209,14 +208,6 @@ public class ScheduledRunCollectionListener<T extends Number> extends ScheduledR
         mTimeSeriesStatistics.update(dataPoint);
     }
 
-    /**
-     * To add listener specific extra args implement this method in the sub class and add the
-     * listener specific args.
-     */
-    public void setupAdditionalArgs() {
-        // NO-OP by default
-    }
-
     protected void createHelperInstance(ICollectorHelper helper) {
         mHelper = helper;
     }
diff --git a/libraries/device-collectors/src/test/java/android/device/collectors/BaseMetricListenerInstrumentedTest.java b/libraries/device-collectors/src/test/java/android/device/collectors/BaseMetricListenerInstrumentedTest.java
index 093107024..eba850178 100644
--- a/libraries/device-collectors/src/test/java/android/device/collectors/BaseMetricListenerInstrumentedTest.java
+++ b/libraries/device-collectors/src/test/java/android/device/collectors/BaseMetricListenerInstrumentedTest.java
@@ -52,6 +52,7 @@ public class BaseMetricListenerInstrumentedTest {
     private static final String TEST_END_KEY = "test_end_key";
     private static final String TEST_START_VALUE = "test_start_value";
     private static final String TEST_END_VALUE = "test_end_value";
+    private static final String SKIP_METRICS_OPTION = "skip-metrics";
     private BaseMetricListener mListener;
     private Instrumentation mMockInstrumentation;
 
@@ -67,29 +68,47 @@ public class BaseMetricListenerInstrumentedTest {
             args = new Bundle();
         }
         return new BaseMetricListener(args) {
+            private boolean mSuppressMetrics = false;
+
             @Override
             public void onTestStart(DataRecord testData, Description description) {
                 // In this test check that a new DataRecord is passed to testStart each time.
                 assertFalse(testData.hasMetrics());
-                testData.addStringMetric(TEST_START_KEY, TEST_START_VALUE
-                        + description.getMethodName());
+                if (!mSuppressMetrics) {
+                    testData.addStringMetric(
+                            TEST_START_KEY, TEST_START_VALUE + description.getMethodName());
+                }
             }
 
             @Override
             public void onTestEnd(DataRecord testData, Description description) {
-                testData.addStringMetric(TEST_END_KEY, TEST_END_VALUE
-                        + description.getMethodName());
+                if (!mSuppressMetrics) {
+                    testData.addStringMetric(
+                            TEST_END_KEY, TEST_END_VALUE + description.getMethodName());
+                }
             }
 
             @Override
             public void onTestRunStart(DataRecord runData, Description description) {
                 assertFalse(runData.hasMetrics());
-                runData.addStringMetric(RUN_START_KEY, RUN_START_VALUE);
+                if (!mSuppressMetrics) {
+                    runData.addStringMetric(RUN_START_KEY, RUN_START_VALUE);
+                }
             }
 
             @Override
             public void onTestRunEnd(DataRecord runData, Result result) {
-                runData.addStringMetric(RUN_END_KEY, RUN_END_VALUE);
+                if (!mSuppressMetrics) {
+                    runData.addStringMetric(RUN_END_KEY, RUN_END_VALUE);
+                }
+            }
+
+            @Override
+            public void setupAdditionalArgs() {
+                // We test this method by checking if metrics are suppressed when the skip metrics
+                // option is set.
+                mSuppressMetrics =
+                        Boolean.parseBoolean(getArgsBundle().getString(SKIP_METRICS_OPTION));
             }
         };
     }
@@ -306,6 +325,32 @@ public class BaseMetricListenerInstrumentedTest {
         assertEquals(2, check.size());
     }
 
+    /** Test that the parsing of the skip metrics option in setUpAdditionalArgs() is executed. */
+    @MetricOption(group = "testGroup")
+    @Test
+    public void testAdditionalArgs() throws Exception {
+        Bundle args = new Bundle();
+        args.putString(SKIP_METRICS_OPTION, String.valueOf(true));
+        mListener = createWithArgs(args);
+        mListener.setInstrumentation(mMockInstrumentation);
+
+        Description runDescription = Description.createSuiteDescription("run");
+        mListener.testRunStarted(runDescription);
+        Description testDescription = Description.createTestDescription("class", "method");
+        mListener.testStarted(testDescription);
+        mListener.testFinished(testDescription);
+        mListener.testRunFinished(new Result());
+        // AJUR runner is then gonna call instrumentationRunFinished
+        Bundle resultBundle = new Bundle();
+        mListener.instrumentationRunFinished(System.out, resultBundle, new Result());
+
+        Mockito.verify(mMockInstrumentation, Mockito.never())
+                .sendStatus(Mockito.anyInt(), Mockito.any());
+
+        // We have skipped reporting metrics, so no run metrics should be in the bundle either.
+        assertEquals(0, resultBundle.size());
+    }
+
     /**
      * Test annotation that allows to instantiate {@link MetricOption} for testing purpose.
      */