Change ClangCodeCoverageListener to implement BaseDeviceMetricCollector.

Remove caching the profile tool due to BaseDeviceMetricCollector
declaring invocationEnded as final.

Test: Unit tests
Bug: 163063008
Change-Id: I1db451d4388911f22878fbc5a7934c3b5f53c336
Merged-In: I1db451d4388911f22878fbc5a7934c3b5f53c336

4 files changed, 54 insertions, 92 deletions

diff --git a/test_framework/com/android/tradefed/testtype/ClangCodeCoverageListener.java b/test_framework/com/android/tradefed/testtype/ClangCodeCoverageListener.java
index b0fb295fd..1a32b5611 100644
--- a/test_framework/com/android/tradefed/testtype/ClangCodeCoverageListener.java
+++ b/test_framework/com/android/tradefed/testtype/ClangCodeCoverageListener.java
@@ -16,6 +16,7 @@
 
 package com.android.tradefed.testtype;
 
+import static com.android.tradefed.testtype.coverage.CoverageOptions.Toolchain.CLANG;
 import static com.google.common.base.Verify.verify;
 import static com.google.common.base.Verify.verifyNotNull;
 
@@ -25,14 +26,12 @@ import com.android.tradefed.config.IConfiguration;
 import com.android.tradefed.config.IConfigurationReceiver;
 import com.android.tradefed.device.DeviceNotAvailableException;
 import com.android.tradefed.device.ITestDevice;
+import com.android.tradefed.device.metric.BaseDeviceMetricCollector;
+import com.android.tradefed.device.metric.DeviceMetricData;
 import com.android.tradefed.log.LogUtil.CLog;
 import com.android.tradefed.metrics.proto.MetricMeasurement.Metric;
 import com.android.tradefed.result.FileInputStreamSource;
-import com.android.tradefed.result.ITestInvocationListener;
 import com.android.tradefed.result.LogDataType;
-import com.android.tradefed.result.ResultForwarder;
-import com.android.tradefed.testtype.coverage.CoverageOptions;
-import com.android.tradefed.testtype.coverage.CoverageOptions.Toolchain;
 import com.android.tradefed.util.CommandResult;
 import com.android.tradefed.util.CommandStatus;
 import com.android.tradefed.util.FileUtil;
@@ -48,15 +47,15 @@ import java.io.File;
 import java.io.IOException;
 import java.nio.file.Path;
 import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
 /**
- * A {@link ResultForwarder} that will pull Clang coverage measurements off of the device and log
- * them as test artifacts.
+ * A {@link BaseDeviceMetricCollector} that will pull Clang coverage measurements off of the device
+ * and log them as test artifacts.
  */
-public final class ClangCodeCoverageListener extends ResultForwarder
+public final class ClangCodeCoverageListener extends BaseDeviceMetricCollector
         implements IConfigurationReceiver {
 
     private static final String NATIVE_COVERAGE_DEVICE_PATH = "/data/misc/trace";
@@ -74,23 +73,12 @@ public final class ClangCodeCoverageListener extends ResultForwarder
     private static final String DELETE_COVERAGE_FILES_COMMAND =
             String.format("find %s -name '*.profraw' -delete", NATIVE_COVERAGE_DEVICE_PATH);
 
-    private final ITestDevice mDevice;
-
     private IBuildInfo mBuildInfo;
     private IConfiguration mConfiguration;
-    private IRunUtil mRunUtil;
+    private IRunUtil mRunUtil = RunUtil.getDefault();
 
     private NativeCodeCoverageFlusher mFlusher;
 
-    private File mLlvmProfdataTool;
-    private String mCurrentRunName;
-
-    public ClangCodeCoverageListener(ITestDevice device, ITestInvocationListener... listeners) {
-        super(listeners);
-        mDevice = device;
-        mRunUtil = RunUtil.getDefault();
-    }
-
     @Override
     public void setConfiguration(IConfiguration configuration) {
         mConfiguration = configuration;
@@ -102,42 +90,30 @@ public final class ClangCodeCoverageListener extends ResultForwarder
     }
 
     @Override
-    public void testRunStarted(String runName, int testCount) {
-        mCurrentRunName = runName;
-        super.testRunStarted(runName, testCount);
-    }
-
-    @Override
-    public void testRunEnded(long elapsedTime, HashMap<String, Metric> runMetrics) {
-        CoverageOptions options = mConfiguration.getCoverageOptions();
+    public void onTestRunEnd(
+            DeviceMetricData runData, final Map<String, Metric> currentRunMetrics) {
+        if (!mConfiguration.getCoverageOptions().isCoverageEnabled()
+                || !mConfiguration.getCoverageOptions().getCoverageToolchains().contains(CLANG)) {
+            return;
+        }
         try {
-            if (options.isCoverageEnabled()
-                    && options.getCoverageToolchains().contains(Toolchain.CLANG)) {
-                // Enable abd root on the device, otherwise the following commands will fail.
-                verify(mDevice.enableAdbRoot(), "Failed to enable adb root.");
+            ITestDevice device = getRealDevices().get(0);
 
-                if (options.isCoverageFlushEnabled()) {
-                    getCoverageFlusher().forceCoverageFlush();
-                }
-                logCoverageMeasurement(mCurrentRunName);
+            // Enable abd root on the device, otherwise the following commands will fail.
+            verify(device.enableAdbRoot(), "Failed to enable adb root.");
 
-                // Delete coverage files on the device.
-                mDevice.executeShellCommand(DELETE_COVERAGE_FILES_COMMAND);
+            if (mConfiguration.getCoverageOptions().isCoverageFlushEnabled()) {
+                getCoverageFlusher(device).forceCoverageFlush();
             }
+            logCoverageMeasurement(device, getRunName());
+
+            // Delete coverage files on the device.
+            device.executeShellCommand(DELETE_COVERAGE_FILES_COMMAND);
         } catch (DeviceNotAvailableException | IOException e) {
             throw new RuntimeException(e);
-        } finally {
-            super.testRunEnded(elapsedTime, runMetrics);
         }
     }
 
-    @Override
-    public void invocationEnded(long elapsedTime) {
-        // Clean up the llvm-profdata tool.
-        FileUtil.recursiveDelete(mLlvmProfdataTool);
-        super.invocationEnded(elapsedTime);
-    }
-
     /**
      * Logs Clang coverage measurements from the device.
      *
@@ -145,7 +121,7 @@ public final class ClangCodeCoverageListener extends ResultForwarder
      * @throws DeviceNotAvailableException
      * @throws IOException
      */
-    private void logCoverageMeasurement(String runName)
+    private void logCoverageMeasurement(ITestDevice device, String runName)
             throws DeviceNotAvailableException, IOException {
         File coverageTarGz = null;
         File untarDir = null;
@@ -153,13 +129,13 @@ public final class ClangCodeCoverageListener extends ResultForwarder
         File indexedProfileFile = null;
         try {
             // Compress coverage measurements on the device before pulling.
-            mDevice.executeShellCommand(ZIP_CLANG_FILES_COMMAND);
-            coverageTarGz = mDevice.pullFile(COVERAGE_TAR_PATH);
+            device.executeShellCommand(ZIP_CLANG_FILES_COMMAND);
+            coverageTarGz = device.pullFile(COVERAGE_TAR_PATH);
             verifyNotNull(
                     coverageTarGz,
                     "Failed to pull the Clang code coverage file %s",
                     COVERAGE_TAR_PATH);
-            mDevice.deleteFile(COVERAGE_TAR_PATH);
+            device.deleteFile(COVERAGE_TAR_PATH);
 
             untarDir = FileUtil.createTempDir("clang_coverage");
             TarUtil.unTar(coverageTarGz, untarDir);
@@ -208,6 +184,7 @@ public final class ClangCodeCoverageListener extends ResultForwarder
         } finally {
             FileUtil.deleteFile(coverageTarGz);
             FileUtil.recursiveDelete(untarDir);
+            FileUtil.recursiveDelete(profileTool);
             FileUtil.deleteFile(indexedProfileFile);
         }
     }
@@ -217,13 +194,13 @@ public final class ClangCodeCoverageListener extends ResultForwarder
      *
      * @return a NativeCodeCoverageFlusher
      */
-    private NativeCodeCoverageFlusher getCoverageFlusher() {
+    private NativeCodeCoverageFlusher getCoverageFlusher(ITestDevice device) {
         if (mFlusher == null) {
             verifyNotNull(mConfiguration);
-            verifyNotNull(mDevice);
+            verifyNotNull(device);
             mFlusher =
                     new NativeCodeCoverageFlusher(
-                            mDevice, mConfiguration.getCoverageOptions().getCoverageProcesses());
+                            device, mConfiguration.getCoverageOptions().getCoverageProcesses());
         }
         return mFlusher;
     }
@@ -234,11 +211,6 @@ public final class ClangCodeCoverageListener extends ResultForwarder
      * @return the directory containing the profile tool and dependencies
      */
     private File getProfileTool() throws IOException {
-        // If we have a cached version of the profile tool already, use it.
-        if (mLlvmProfdataTool != null) {
-            return mLlvmProfdataTool;
-        }
-
         // If llvm-profdata-path was set in the Configuration, pass it through. Don't save the path
         // locally since the parent process is responsible for cleaning it up.
         File configurationTool = mConfiguration.getCoverageOptions().getLlvmProfdataPath();
@@ -254,8 +226,7 @@ public final class ClangCodeCoverageListener extends ResultForwarder
                     verifyNotNull(
                             buildInfo.getFile("llvm-profdata.zip"),
                             "Could not get llvm-profdata.zip from the build.");
-            mLlvmProfdataTool = ZipUtil.extractZipToTemp(profileToolZip, "llvm-profdata");
-            return mLlvmProfdataTool;
+            return ZipUtil.extractZipToTemp(profileToolZip, "llvm-profdata");
         } catch (BuildRetrievalError e) {
             throw new RuntimeException(e);
         } finally {
diff --git a/test_framework/com/android/tradefed/testtype/GTest.java b/test_framework/com/android/tradefed/testtype/GTest.java
index 0adc0f103..db170fcf2 100644
--- a/test_framework/com/android/tradefed/testtype/GTest.java
+++ b/test_framework/com/android/tradefed/testtype/GTest.java
@@ -430,7 +430,7 @@ public class GTest extends GTestBase implements IDeviceTest {
         }
         // Insert the coverage listener if code coverage collection is enabled.
         listener = addNativeCoverageListenerIfEnabled(testInfo.getContext(), listener);
-        listener = addClangCoverageListenerIfEnabled(listener);
+        listener = addClangCoverageListenerIfEnabled(testInfo.getContext(), listener);
         listener = getGTestListener(listener);
         NativeCodeCoverageFlusher flusher =
                 new NativeCodeCoverageFlusher(
@@ -490,14 +490,13 @@ public class GTest extends GTestBase implements IDeviceTest {
      * @return a native coverage listener if coverage is enabled, otherwise the original listener
      */
     private ITestInvocationListener addClangCoverageListenerIfEnabled(
-            ITestInvocationListener listener) {
+            IInvocationContext context, ITestInvocationListener listener) {
         CoverageOptions options = getConfiguration().getCoverageOptions();
 
         if (options.isCoverageEnabled() && options.getCoverageToolchains().contains(CLANG)) {
-            ClangCodeCoverageListener clangListener =
-                    new ClangCodeCoverageListener(mDevice, listener);
+            ClangCodeCoverageListener clangListener = new ClangCodeCoverageListener();
             clangListener.setConfiguration(getConfiguration());
-            return clangListener;
+            listener = clangListener.init(context, listener);
         }
         return listener;
     }
diff --git a/test_framework/com/android/tradefed/testtype/InstrumentationTest.java b/test_framework/com/android/tradefed/testtype/InstrumentationTest.java
index b08732899..92df21248 100644
--- a/test_framework/com/android/tradefed/testtype/InstrumentationTest.java
+++ b/test_framework/com/android/tradefed/testtype/InstrumentationTest.java
@@ -914,7 +914,7 @@ public class InstrumentationTest
             listener = addBugreportListenerIfEnabled(listener);
             listener = addJavaCoverageListenerIfEnabled(testInfo, listener);
             listener = addGcovCoverageListenerIfEnabled(testInfo, listener);
-            listener = addClangCoverageListenerIfEnabled(listener);
+            listener = addClangCoverageListenerIfEnabled(testInfo, listener);
 
             // Clear coverage measurements on the device before running.
             if (mConfiguration != null
@@ -1031,16 +1031,16 @@ public class InstrumentationTest
      * Returns a listener that will collect Clang coverage measurements, or the original {@code
      * listener} if this feature is disabled.
      */
-    ITestInvocationListener addClangCoverageListenerIfEnabled(ITestInvocationListener listener) {
+    ITestInvocationListener addClangCoverageListenerIfEnabled(
+            TestInformation testInfo, ITestInvocationListener listener) {
         if (mConfiguration == null) {
             return listener;
         }
         if (mConfiguration.getCoverageOptions().isCoverageEnabled()
                 && mConfiguration.getCoverageOptions().getCoverageToolchains().contains(CLANG)) {
-            ClangCodeCoverageListener clangListener =
-                    new ClangCodeCoverageListener(getDevice(), listener);
+            ClangCodeCoverageListener clangListener = new ClangCodeCoverageListener();
             clangListener.setConfiguration(mConfiguration);
-            return clangListener;
+            listener = clangListener.init(testInfo.getContext(), listener);
         }
         return listener;
     }
diff --git a/tests/src/com/android/tradefed/testtype/ClangCodeCoverageListenerTest.java b/tests/src/com/android/tradefed/testtype/ClangCodeCoverageListenerTest.java
index 69f60fd52..67f614410 100644
--- a/tests/src/com/android/tradefed/testtype/ClangCodeCoverageListenerTest.java
+++ b/tests/src/com/android/tradefed/testtype/ClangCodeCoverageListenerTest.java
@@ -28,6 +28,7 @@ import com.android.tradefed.build.IBuildProvider;
 import com.android.tradefed.config.IConfiguration;
 import com.android.tradefed.config.OptionSetter;
 import com.android.tradefed.device.ITestDevice;
+import com.android.tradefed.invoker.IInvocationContext;
 import com.android.tradefed.metrics.proto.MetricMeasurement;
 import com.android.tradefed.result.ITestInvocationListener;
 import com.android.tradefed.result.InputStreamSource;
@@ -38,7 +39,6 @@ import com.android.tradefed.util.CommandStatus;
 import com.android.tradefed.util.IRunUtil;
 import com.android.tradefed.util.proto.TfMetricProtoUtil;
 
-import com.google.common.base.VerifyException;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.protobuf.ByteString;
@@ -86,6 +86,7 @@ public class ClangCodeCoverageListenerTest {
     @Mock IConfiguration mMockConfiguration;
     @Mock IBuildProvider mMockBuildProvider;
     @Mock ITestDevice mMockDevice;
+    @Mock IInvocationContext mMockContext;
     @Spy CommandArgumentCaptor mCommandArgumentCaptor;
     LogFileReader mFakeListener = new LogFileReader();
 
@@ -111,9 +112,12 @@ public class ClangCodeCoverageListenerTest {
         doReturn(mMockBuildProvider).when(mMockConfiguration).getBuildProvider();
         doReturn(mMockBuildInfo).when(mMockBuildProvider).getBuild();
 
-        mListener = new ClangCodeCoverageListener(mMockDevice, mFakeListener);
+        doReturn(ImmutableList.of(mMockDevice)).when(mMockContext).getDevices();
+
+        mListener = new ClangCodeCoverageListener();
         mListener.setConfiguration(mMockConfiguration);
         mListener.setRunUtil(mCommandArgumentCaptor);
+        mListener.init(mMockContext, mFakeListener);
     }
 
     @Test
@@ -277,7 +281,7 @@ public class ClangCodeCoverageListenerTest {
     }
 
     @Test
-    public void testProfileToolNotFound_throwsException() throws Exception {
+    public void testProfileToolNotFound_noLog() throws Exception {
         mCoverageOptionsSetter.setOptionValue("coverage", "true");
         mCoverageOptionsSetter.setOptionValue("coverage-toolchain", "CLANG");
 
@@ -293,22 +297,16 @@ public class ClangCodeCoverageListenerTest {
         doReturn(tarGz).when(mMockDevice).pullFile(anyString());
 
         // Simulate a test run.
-        try {
-            mListener.testRunStarted(RUN_NAME, TEST_COUNT);
-            mListener.testRunEnded(ELAPSED_TIME, mMetrics);
-            mListener.invocationEnded(ELAPSED_TIME);
-            fail("an exception should have been thrown");
-        } catch (VerifyException e) {
-            // Expected.
-            assertThat(e).hasMessageThat().contains("llvm-profdata");
-        }
+        mListener.testRunStarted(RUN_NAME, TEST_COUNT);
+        mListener.testRunEnded(ELAPSED_TIME, mMetrics);
+        mListener.invocationEnded(ELAPSED_TIME);
 
         // Verify testLog(..) was never called.
         assertThat(mFakeListener.getLogs()).isEmpty();
     }
 
     @Test
-    public void testProfileToolFailed_throwsException() throws Exception {
+    public void testProfileToolFailed_noLog() throws Exception {
         mCoverageOptionsSetter.setOptionValue("coverage", "true");
         mCoverageOptionsSetter.setOptionValue("coverage-toolchain", "CLANG");
 
@@ -327,14 +325,8 @@ public class ClangCodeCoverageListenerTest {
         mCommandArgumentCaptor.setResult(CommandStatus.FAILED);
 
         // Simulate a test run.
-        try {
-            mListener.testRunStarted(RUN_NAME, TEST_COUNT);
-            mListener.testRunEnded(ELAPSED_TIME, mMetrics);
-            fail("an exception should have been thrown");
-        } catch (RuntimeException e) {
-            // Expected.
-            assertThat(e).hasMessageThat().contains("merge Clang profile data");
-        }
+        mListener.testRunStarted(RUN_NAME, TEST_COUNT);
+        mListener.testRunEnded(ELAPSED_TIME, mMetrics);
         mListener.invocationEnded(ELAPSED_TIME);
 
         // Verify testLog(..) was never called.