DO NOT MERGE: Revert "Change ClangCodeCoverageListener to implement BaseDevice..."

Revert submission 12789043-host-test-coverage

Reason for revert: Coverage tests not generating artifacts
Reverted Changes:
I61dde583c:Move initial coverage clearing out of the test typ...
If7479c00a:Add ClangCodeCoverageCollector as an AutoLogCollec...
I1db451d43:Change ClangCodeCoverageListener to implement Base...

Change-Id: Ie794806a16f0315090a1566d61e9261b93b4eed6

4 files changed, 92 insertions, 54 deletions

diff --git a/test_framework/com/android/tradefed/testtype/ClangCodeCoverageListener.java b/test_framework/com/android/tradefed/testtype/ClangCodeCoverageListener.java
index 1a32b5611..b0fb295fd 100644
--- a/test_framework/com/android/tradefed/testtype/ClangCodeCoverageListener.java
+++ b/test_framework/com/android/tradefed/testtype/ClangCodeCoverageListener.java
@@ -16,7 +16,6 @@
 
 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;
 
@@ -26,12 +25,14 @@ 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;
@@ -47,15 +48,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 BaseDeviceMetricCollector} that will pull Clang coverage measurements off of the device
- * and log them as test artifacts.
+ * A {@link ResultForwarder} that will pull Clang coverage measurements off of the device and log
+ * them as test artifacts.
  */
-public final class ClangCodeCoverageListener extends BaseDeviceMetricCollector
+public final class ClangCodeCoverageListener extends ResultForwarder
         implements IConfigurationReceiver {
 
     private static final String NATIVE_COVERAGE_DEVICE_PATH = "/data/misc/trace";
@@ -73,12 +74,23 @@ public final class ClangCodeCoverageListener extends BaseDeviceMetricCollector
     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 = RunUtil.getDefault();
+    private IRunUtil mRunUtil;
 
     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;
@@ -90,30 +102,42 @@ public final class ClangCodeCoverageListener extends BaseDeviceMetricCollector
     }
 
     @Override
-    public void onTestRunEnd(
-            DeviceMetricData runData, final Map<String, Metric> currentRunMetrics) {
-        if (!mConfiguration.getCoverageOptions().isCoverageEnabled()
-                || !mConfiguration.getCoverageOptions().getCoverageToolchains().contains(CLANG)) {
-            return;
-        }
+    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();
         try {
-            ITestDevice device = getRealDevices().get(0);
+            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.");
 
-            // Enable abd root on the device, otherwise the following commands will fail.
-            verify(device.enableAdbRoot(), "Failed to enable adb root.");
+                if (options.isCoverageFlushEnabled()) {
+                    getCoverageFlusher().forceCoverageFlush();
+                }
+                logCoverageMeasurement(mCurrentRunName);
 
-            if (mConfiguration.getCoverageOptions().isCoverageFlushEnabled()) {
-                getCoverageFlusher(device).forceCoverageFlush();
+                // Delete coverage files on the device.
+                mDevice.executeShellCommand(DELETE_COVERAGE_FILES_COMMAND);
             }
-            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.
      *
@@ -121,7 +145,7 @@ public final class ClangCodeCoverageListener extends BaseDeviceMetricCollector
      * @throws DeviceNotAvailableException
      * @throws IOException
      */
-    private void logCoverageMeasurement(ITestDevice device, String runName)
+    private void logCoverageMeasurement(String runName)
             throws DeviceNotAvailableException, IOException {
         File coverageTarGz = null;
         File untarDir = null;
@@ -129,13 +153,13 @@ public final class ClangCodeCoverageListener extends BaseDeviceMetricCollector
         File indexedProfileFile = null;
         try {
             // Compress coverage measurements on the device before pulling.
-            device.executeShellCommand(ZIP_CLANG_FILES_COMMAND);
-            coverageTarGz = device.pullFile(COVERAGE_TAR_PATH);
+            mDevice.executeShellCommand(ZIP_CLANG_FILES_COMMAND);
+            coverageTarGz = mDevice.pullFile(COVERAGE_TAR_PATH);
             verifyNotNull(
                     coverageTarGz,
                     "Failed to pull the Clang code coverage file %s",
                     COVERAGE_TAR_PATH);
-            device.deleteFile(COVERAGE_TAR_PATH);
+            mDevice.deleteFile(COVERAGE_TAR_PATH);
 
             untarDir = FileUtil.createTempDir("clang_coverage");
             TarUtil.unTar(coverageTarGz, untarDir);
@@ -184,7 +208,6 @@ public final class ClangCodeCoverageListener extends BaseDeviceMetricCollector
         } finally {
             FileUtil.deleteFile(coverageTarGz);
             FileUtil.recursiveDelete(untarDir);
-            FileUtil.recursiveDelete(profileTool);
             FileUtil.deleteFile(indexedProfileFile);
         }
     }
@@ -194,13 +217,13 @@ public final class ClangCodeCoverageListener extends BaseDeviceMetricCollector
      *
      * @return a NativeCodeCoverageFlusher
      */
-    private NativeCodeCoverageFlusher getCoverageFlusher(ITestDevice device) {
+    private NativeCodeCoverageFlusher getCoverageFlusher() {
         if (mFlusher == null) {
             verifyNotNull(mConfiguration);
-            verifyNotNull(device);
+            verifyNotNull(mDevice);
             mFlusher =
                     new NativeCodeCoverageFlusher(
-                            device, mConfiguration.getCoverageOptions().getCoverageProcesses());
+                            mDevice, mConfiguration.getCoverageOptions().getCoverageProcesses());
         }
         return mFlusher;
     }
@@ -211,6 +234,11 @@ public final class ClangCodeCoverageListener extends BaseDeviceMetricCollector
      * @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();
@@ -226,7 +254,8 @@ public final class ClangCodeCoverageListener extends BaseDeviceMetricCollector
                     verifyNotNull(
                             buildInfo.getFile("llvm-profdata.zip"),
                             "Could not get llvm-profdata.zip from the build.");
-            return ZipUtil.extractZipToTemp(profileToolZip, "llvm-profdata");
+            mLlvmProfdataTool = ZipUtil.extractZipToTemp(profileToolZip, "llvm-profdata");
+            return mLlvmProfdataTool;
         } 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 197821e71..70beba964 100644
--- a/test_framework/com/android/tradefed/testtype/GTest.java
+++ b/test_framework/com/android/tradefed/testtype/GTest.java
@@ -431,7 +431,7 @@ public class GTest extends GTestBase implements IDeviceTest {
         }
         // Insert the coverage listener if code coverage collection is enabled.
         listener = addGcovCoverageListenerIfEnabled(testInfo.getContext(), listener);
-        listener = addClangCoverageListenerIfEnabled(testInfo.getContext(), listener);
+        listener = addClangCoverageListenerIfEnabled(listener);
         listener = getGTestListener(listener);
         NativeCodeCoverageFlusher flusher =
                 new NativeCodeCoverageFlusher(
@@ -491,13 +491,14 @@ public class GTest extends GTestBase implements IDeviceTest {
      * @return a native coverage listener if coverage is enabled, otherwise the original listener
      */
     private ITestInvocationListener addClangCoverageListenerIfEnabled(
-            IInvocationContext context, ITestInvocationListener listener) {
+            ITestInvocationListener listener) {
         CoverageOptions options = getConfiguration().getCoverageOptions();
 
         if (options.isCoverageEnabled() && options.getCoverageToolchains().contains(CLANG)) {
-            ClangCodeCoverageListener clangListener = new ClangCodeCoverageListener();
+            ClangCodeCoverageListener clangListener =
+                    new ClangCodeCoverageListener(mDevice, listener);
             clangListener.setConfiguration(getConfiguration());
-            listener = clangListener.init(context, listener);
+            return clangListener;
         }
         return listener;
     }
diff --git a/test_framework/com/android/tradefed/testtype/InstrumentationTest.java b/test_framework/com/android/tradefed/testtype/InstrumentationTest.java
index b08d988e7..bba5fa39b 100644
--- a/test_framework/com/android/tradefed/testtype/InstrumentationTest.java
+++ b/test_framework/com/android/tradefed/testtype/InstrumentationTest.java
@@ -915,7 +915,7 @@ public class InstrumentationTest
             listener = addBugreportListenerIfEnabled(listener);
             listener = addJavaCoverageListenerIfEnabled(testInfo, listener);
             listener = addGcovCoverageListenerIfEnabled(testInfo, listener);
-            listener = addClangCoverageListenerIfEnabled(testInfo, listener);
+            listener = addClangCoverageListenerIfEnabled(listener);
 
             // Clear coverage measurements on the device before running.
             if (mConfiguration != null
@@ -1035,16 +1035,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(
-            TestInformation testInfo, ITestInvocationListener listener) {
+    ITestInvocationListener addClangCoverageListenerIfEnabled(ITestInvocationListener listener) {
         if (mConfiguration == null) {
             return listener;
         }
         if (mConfiguration.getCoverageOptions().isCoverageEnabled()
                 && mConfiguration.getCoverageOptions().getCoverageToolchains().contains(CLANG)) {
-            ClangCodeCoverageListener clangListener = new ClangCodeCoverageListener();
+            ClangCodeCoverageListener clangListener =
+                    new ClangCodeCoverageListener(getDevice(), listener);
             clangListener.setConfiguration(mConfiguration);
-            listener = clangListener.init(testInfo.getContext(), listener);
+            return clangListener;
         }
         return listener;
     }
diff --git a/tests/src/com/android/tradefed/testtype/ClangCodeCoverageListenerTest.java b/tests/src/com/android/tradefed/testtype/ClangCodeCoverageListenerTest.java
index 67f614410..69f60fd52 100644
--- a/tests/src/com/android/tradefed/testtype/ClangCodeCoverageListenerTest.java
+++ b/tests/src/com/android/tradefed/testtype/ClangCodeCoverageListenerTest.java
@@ -28,7 +28,6 @@ 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;
@@ -39,6 +38,7 @@ 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,7 +86,6 @@ public class ClangCodeCoverageListenerTest {
     @Mock IConfiguration mMockConfiguration;
     @Mock IBuildProvider mMockBuildProvider;
     @Mock ITestDevice mMockDevice;
-    @Mock IInvocationContext mMockContext;
     @Spy CommandArgumentCaptor mCommandArgumentCaptor;
     LogFileReader mFakeListener = new LogFileReader();
 
@@ -112,12 +111,9 @@ public class ClangCodeCoverageListenerTest {
         doReturn(mMockBuildProvider).when(mMockConfiguration).getBuildProvider();
         doReturn(mMockBuildInfo).when(mMockBuildProvider).getBuild();
 
-        doReturn(ImmutableList.of(mMockDevice)).when(mMockContext).getDevices();
-
-        mListener = new ClangCodeCoverageListener();
+        mListener = new ClangCodeCoverageListener(mMockDevice, mFakeListener);
         mListener.setConfiguration(mMockConfiguration);
         mListener.setRunUtil(mCommandArgumentCaptor);
-        mListener.init(mMockContext, mFakeListener);
     }
 
     @Test
@@ -281,7 +277,7 @@ public class ClangCodeCoverageListenerTest {
     }
 
     @Test
-    public void testProfileToolNotFound_noLog() throws Exception {
+    public void testProfileToolNotFound_throwsException() throws Exception {
         mCoverageOptionsSetter.setOptionValue("coverage", "true");
         mCoverageOptionsSetter.setOptionValue("coverage-toolchain", "CLANG");
 
@@ -297,16 +293,22 @@ public class ClangCodeCoverageListenerTest {
         doReturn(tarGz).when(mMockDevice).pullFile(anyString());
 
         // Simulate a test run.
-        mListener.testRunStarted(RUN_NAME, TEST_COUNT);
-        mListener.testRunEnded(ELAPSED_TIME, mMetrics);
-        mListener.invocationEnded(ELAPSED_TIME);
+        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");
+        }
 
         // Verify testLog(..) was never called.
         assertThat(mFakeListener.getLogs()).isEmpty();
     }
 
     @Test
-    public void testProfileToolFailed_noLog() throws Exception {
+    public void testProfileToolFailed_throwsException() throws Exception {
         mCoverageOptionsSetter.setOptionValue("coverage", "true");
         mCoverageOptionsSetter.setOptionValue("coverage-toolchain", "CLANG");
 
@@ -325,8 +327,14 @@ public class ClangCodeCoverageListenerTest {
         mCommandArgumentCaptor.setResult(CommandStatus.FAILED);
 
         // Simulate a test run.
-        mListener.testRunStarted(RUN_NAME, TEST_COUNT);
-        mListener.testRunEnded(ELAPSED_TIME, mMetrics);
+        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.invocationEnded(ELAPSED_TIME);
 
         // Verify testLog(..) was never called.