Finish deprecating error collection built-in ITestSuite

Instead common AutoLogger collection can be used and are
maintained.

Test: presubmit
Bug: 332337273
Change-Id: I8ea0545be1b8ad97bfcbdb90fbaf96189bf251a3

10 files changed, 28 insertions, 364 deletions

diff --git a/javatests/com/android/tradefed/UnitTests.java b/javatests/com/android/tradefed/UnitTests.java
index 0ea56e08f..d1cf792ec 100644
--- a/javatests/com/android/tradefed/UnitTests.java
+++ b/javatests/com/android/tradefed/UnitTests.java
@@ -375,7 +375,6 @@ import com.android.tradefed.testtype.suite.RemoteTestTimeOutEnforcerTest;
 import com.android.tradefed.testtype.suite.ResolvePartialDownloadTest;
 import com.android.tradefed.testtype.suite.SuiteModuleLoaderTest;
 import com.android.tradefed.testtype.suite.SuiteTestFilterTest;
-import com.android.tradefed.testtype.suite.TestFailureListenerTest;
 import com.android.tradefed.testtype.suite.TestMappingSuiteRunnerTest;
 import com.android.tradefed.testtype.suite.TestSuiteInfoTest;
 import com.android.tradefed.testtype.suite.TfSuiteRunnerTest;
@@ -972,7 +971,6 @@ import org.junit.runners.Suite.SuiteClasses;
     ResolvePartialDownloadTest.class,
     SuiteModuleLoaderTest.class,
     SuiteTestFilterTest.class,
-    TestFailureListenerTest.class,
     TestMappingSuiteRunnerTest.class,
     TestSuiteInfoTest.class,
     TfSuiteRunnerTest.class,
diff --git a/javatests/com/android/tradefed/testtype/suite/GranularRetriableTestWrapperTest.java b/javatests/com/android/tradefed/testtype/suite/GranularRetriableTestWrapperTest.java
index dcc06c49b..05cd6a489 100644
--- a/javatests/com/android/tradefed/testtype/suite/GranularRetriableTestWrapperTest.java
+++ b/javatests/com/android/tradefed/testtype/suite/GranularRetriableTestWrapperTest.java
@@ -312,7 +312,7 @@ public class GranularRetriableTestWrapperTest {
             ModuleDefinition module)
             throws Exception {
         GranularRetriableTestWrapper granularTestWrapper =
-                new GranularRetriableTestWrapper(test, module, null, null, null, maxRunCount);
+                new GranularRetriableTestWrapper(test, module, null, null, maxRunCount);
         granularTestWrapper.setModuleId("test module");
         granularTestWrapper.setMarkTestsSkipped(false);
         granularTestWrapper.setMetricCollectors(collectors);
diff --git a/javatests/com/android/tradefed/testtype/suite/ModuleDefinitionMultiTest.java b/javatests/com/android/tradefed/testtype/suite/ModuleDefinitionMultiTest.java
index dee050f0b..280c0ebcd 100644
--- a/javatests/com/android/tradefed/testtype/suite/ModuleDefinitionMultiTest.java
+++ b/javatests/com/android/tradefed/testtype/suite/ModuleDefinitionMultiTest.java
@@ -126,7 +126,7 @@ public class ModuleDefinitionMultiTest {
         when(mMockTargetPrep.isDisabled()).thenReturn(false);
 
         when(mMockTargetPrep.isTearDownDisabled()).thenReturn(true);
-        mModule.run(moduleInfo, mListener, null, null, 1);
+        mModule.run(moduleInfo, mListener, null, 1);
         verify(mMockTargetPrep, times(2)).isDisabled();
         verify(mListener)
                 .testRunStarted(
@@ -167,7 +167,7 @@ public class ModuleDefinitionMultiTest {
         when(mMockTargetPrep.isDisabled()).thenReturn(false);
 
         when(mMockTargetPrep.isTearDownDisabled()).thenReturn(true);
-        mModule.run(moduleInfo, mListener, null, null, 1);
+        mModule.run(moduleInfo, mListener, null, 1);
         verify(mMockTargetPrep, times(2)).isDisabled();
         verify(mListener)
                 .testRunStarted(
diff --git a/javatests/com/android/tradefed/testtype/suite/ModuleDefinitionTest.java b/javatests/com/android/tradefed/testtype/suite/ModuleDefinitionTest.java
index 2df567f5c..7b6b4ed45 100644
--- a/javatests/com/android/tradefed/testtype/suite/ModuleDefinitionTest.java
+++ b/javatests/com/android/tradefed/testtype/suite/ModuleDefinitionTest.java
@@ -80,8 +80,6 @@ import com.android.tradefed.testtype.IDeviceTest;
 import com.android.tradefed.testtype.IRemoteTest;
 import com.android.tradefed.testtype.ITestFilterReceiver;
 import com.android.tradefed.testtype.suite.module.BaseModuleController;
-import com.android.tradefed.testtype.suite.module.IModuleController;
-import com.android.tradefed.testtype.suite.module.TestFailureModuleController;
 
 import org.junit.Assert;
 import org.junit.Before;
@@ -812,7 +810,7 @@ public class ModuleDefinitionTest {
                         .setInvocationContext(mModule.getModuleInvocationContext())
                         .build();
         mModule.setRetryDecision(mDecision);
-        mModule.run(mModuleInfo, mMockListener, Arrays.asList(mockModuleListener), null);
+        mModule.run(mModuleInfo, mMockListener, Arrays.asList(mockModuleListener));
         verify(mMockListener)
                 .testRunStarted(
                         Mockito.eq(MODULE_NAME), Mockito.eq(1), Mockito.eq(0), Mockito.anyLong());
@@ -980,7 +978,7 @@ public class ModuleDefinitionTest {
                 .shouldRetryPreparation(Mockito.any(), Mockito.anyInt(), Mockito.anyInt());
         when(mMockDecision.getRetryStrategy()).thenReturn(RetryStrategy.ITERATIONS);
         mModule.setRetryDecision(mMockDecision);
-        mModule.run(mModuleInfo, mMockListener, null, null, 3);
+        mModule.run(mModuleInfo, mMockListener, null, 3);
         verify(mMockListener)
                 .testRunStarted(
                         Mockito.eq(MODULE_NAME), Mockito.eq(1), Mockito.eq(0), Mockito.anyLong());
@@ -1252,7 +1250,7 @@ public class ModuleDefinitionTest {
                         .setInvocationContext(mModule.getModuleInvocationContext())
                         .build();
         // module is completely skipped, no tests is recorded.
-        mModule.run(mModuleInfo, mMockListener, null, null);
+        mModule.run(mModuleInfo, mMockListener, null);
     }
 
     /**
@@ -1304,7 +1302,7 @@ public class ModuleDefinitionTest {
                         .build();
 
         // expect the module to run but tests to be ignored
-        mModule.run(mModuleInfo, mMockListener, null, null);
+        mModule.run(mModuleInfo, mMockListener, null);
         verify(mMockListener)
                 .testRunStarted(Mockito.any(), Mockito.anyInt(), Mockito.eq(0), Mockito.anyLong());
         verify(mMockListener).testStarted(Mockito.any(), Mockito.anyLong());
@@ -1423,54 +1421,6 @@ public class ModuleDefinitionTest {
                 .testRunEnded(Mockito.anyLong(), Mockito.<HashMap<String, Metric>>any());
     }
 
-    /**
-     * Test that the {@link IModuleController} object can override the behavior of the capture of
-     * the failure.
-     */
-    @Test
-    public void testOverrideModuleConfig() throws Exception {
-        // failure listener with capture logcat on failure and screenshot on failure.
-        List<ITestDevice> listDevice = new ArrayList<>();
-        listDevice.add(mMockDevice);
-        when(mMockDevice.getSerialNumber()).thenReturn("Serial");
-        TestFailureListener failureListener = new TestFailureListener(listDevice, true, false);
-        failureListener.setLogger(mMockListener);
-        IConfiguration config = new Configuration("", "");
-        TestFailureModuleController moduleConfig = new TestFailureModuleController();
-        OptionSetter setter = new OptionSetter(moduleConfig);
-        // Module option should override the logcat on failure
-        setter.setOptionValue("bugreportz-on-failure", "false");
-        config.setConfigurationObject(ModuleDefinition.MODULE_CONTROLLER, moduleConfig);
-        List<IRemoteTest> testList = new ArrayList<>();
-        testList.add(
-                new IRemoteTest() {
-                    @Override
-                    public void run(TestInformation testInfo, ITestInvocationListener listener)
-                            throws DeviceNotAvailableException {
-                        listener.testFailed(
-                                new TestDescription("failedclass", "failedmethod"),
-                                FailureDescription.create("trace", FailureStatus.TEST_FAILURE));
-                    }
-                });
-        mTargetPrepList.clear();
-        mModule =
-                new ModuleDefinition(
-                        MODULE_NAME,
-                        testList,
-                        mMapDeviceTargetPreparer,
-                        mMultiTargetPrepList,
-                        config);
-        mModule.setRetryDecision(mDecision);
-        mModule.setLogSaver(mMockLogSaver);
-
-        mModule.run(mModuleInfo, mMockListener, null, failureListener);
-        verify(mMockListener)
-                .testRunStarted(
-                        Mockito.eq("fakeName"), Mockito.eq(0), Mockito.eq(0), Mockito.anyLong());
-        verify(mMockListener)
-                .testRunEnded(Mockito.anyLong(), Mockito.<HashMap<String, Metric>>any());
-    }
-
     /** Test when the test yields a DeviceUnresponsive exception. */
     @Test
     public void testRun_partialRun_deviceUnresponsive() throws Exception {
@@ -1572,7 +1522,7 @@ public class ModuleDefinitionTest {
 
         LogSaverResultForwarder forwarder =
                 new LogSaverResultForwarder(mMockLogSaver, Arrays.asList(mMockLogSaverListener));
-        mModule.run(mModuleInfo, forwarder, Arrays.asList(mMockListener), null);
+        mModule.run(mModuleInfo, forwarder, Arrays.asList(mMockListener));
         InOrder inOrder = Mockito.inOrder(mMockLogSaverListener, mMockListener);
         inOrder.verify(mMockLogSaverListener).setLogSaver(mMockLogSaver);
         inOrder.verify(mMockListener)
@@ -1780,7 +1730,7 @@ public class ModuleDefinitionTest {
         when(mMockPrep.isDisabled()).thenReturn(false);
         when(mMockPrep.isTearDownDisabled()).thenReturn(false);
 
-        mModule.run(mModuleInfo, mMockListener, null, null, 3);
+        mModule.run(mModuleInfo, mMockListener, null, 3);
         verify(mMockPrep, times(2)).isDisabled();
         verify(mMockPrep).setUp(Mockito.eq(mModuleInfo));
         verify(mMockPrep).tearDown(Mockito.eq(mModuleInfo), Mockito.isNull());
@@ -1881,7 +1831,7 @@ public class ModuleDefinitionTest {
         when(mMockPrep.isTearDownDisabled()).thenReturn(false);
         when(mMockDevice.getIDevice()).thenReturn(mock(IDevice.class));
 
-        mModule.run(mModuleInfo, mMockListener, null, null, 3);
+        mModule.run(mModuleInfo, mMockListener, null, 3);
         verify(mMockPrep, times(2)).isDisabled();
         verify(mMockDevice, times(3)).getIDevice();
         verify(mMockPrep).setUp(Mockito.eq(mModuleInfo));
diff --git a/javatests/com/android/tradefed/testtype/suite/TestFailureListenerTest.java b/javatests/com/android/tradefed/testtype/suite/TestFailureListenerTest.java
deleted file mode 100644
index 8434af7ba..000000000
--- a/javatests/com/android/tradefed/testtype/suite/TestFailureListenerTest.java
+++ /dev/null
@@ -1,118 +0,0 @@
-/*
- * Copyright (C) 2016 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package com.android.tradefed.testtype.suite;
-
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
-
-import com.android.tradefed.device.ITestDevice;
-import com.android.tradefed.metrics.proto.MetricMeasurement.Metric;
-import com.android.tradefed.result.ITestInvocationListener;
-import com.android.tradefed.result.TestDescription;
-
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
-import org.mockito.InOrder;
-import org.mockito.Mock;
-import org.mockito.Mockito;
-import org.mockito.MockitoAnnotations;
-
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-
-/** Unit tests for {@link com.android.tradefed.testtype.suite.TestFailureListener} */
-@RunWith(JUnit4.class)
-public class TestFailureListenerTest {
-
-    private TestFailureListener mFailureListener;
-    @Mock ITestInvocationListener mMockListener;
-    @Mock ITestDevice mMockDevice;
-    private List<ITestDevice> mListDevice;
-
-    @Before
-    public void setUp() {
-        MockitoAnnotations.initMocks(this);
-
-        mListDevice = new ArrayList<>();
-        mListDevice.add(mMockDevice);
-        when(mMockDevice.getSerialNumber()).thenReturn("SERIAL");
-        // Create base failure listener with all option ON and default logcat size.
-        mFailureListener = new TestFailureListener(mListDevice, true, true);
-        mFailureListener.setLogger(mMockListener);
-    }
-
-    /** Test that on testFailed all the collection are triggered. */
-    @Test
-    public void testTestFailed() throws Exception {
-        TestDescription testId = new TestDescription("com.fake", "methodfake");
-        final String trace = "oups it failed";
-        // Bugreport routine - testLog is internal to it.
-        when(mMockDevice.logBugreport(Mockito.any(), Mockito.any())).thenReturn(true);
-        // Reboot routine
-        when(mMockDevice.getProperty(Mockito.eq("ro.build.type"))).thenReturn("userdebug");
-
-        mFailureListener.testStarted(testId);
-        mFailureListener.testFailed(testId, trace);
-        mFailureListener.testEnded(testId, new HashMap<String, Metric>());
-
-        verify(mMockDevice).reboot();
-    }
-
-    /** Test when a test failure occurs and it is a user build, no reboot is attempted. */
-    @Test
-    public void testTestFailed_userBuild() throws Exception {
-        mFailureListener = new TestFailureListener(mListDevice, false, true);
-        mFailureListener.setLogger(mMockListener);
-        final String trace = "oups it failed";
-        TestDescription testId = new TestDescription("com.fake", "methodfake");
-        when(mMockDevice.getProperty(Mockito.eq("ro.build.type"))).thenReturn("user");
-
-        mFailureListener.testStarted(testId);
-        mFailureListener.testFailed(testId, trace);
-        mFailureListener.testEnded(testId, new HashMap<String, Metric>());
-
-        verify(mMockDevice).getProperty(Mockito.eq("ro.build.type"));
-    }
-
-    /**
-     * Test when a test failure occurs during a multi device run. Each device should capture the
-     * logs.
-     */
-    @Test
-    public void testFailed_multiDevice() throws Exception {
-        ITestDevice device2 = mock(ITestDevice.class);
-        mListDevice.add(device2);
-        mFailureListener = new TestFailureListener(mListDevice, false, true);
-        mFailureListener.setLogger(mMockListener);
-        final String trace = "oups it failed";
-        TestDescription testId = new TestDescription("com.fake", "methodfake");
-        when(mMockDevice.getProperty(Mockito.eq("ro.build.type"))).thenReturn("debug");
-        when(device2.getSerialNumber()).thenReturn("SERIAL2");
-        when(device2.getProperty(Mockito.eq("ro.build.type"))).thenReturn("debug");
-
-        mFailureListener.testStarted(testId);
-        mFailureListener.testFailed(testId, trace);
-        mFailureListener.testEnded(testId, new HashMap<String, Metric>());
-        InOrder inOrder = Mockito.inOrder(mMockDevice, device2);
-        inOrder.verify(mMockDevice).getProperty(Mockito.eq("ro.build.type"));
-        inOrder.verify(mMockDevice).reboot();
-        inOrder.verify(device2).reboot();
-    }
-}
diff --git a/src/com/android/tradefed/result/suite/XmlSuiteResultFormatter.java b/src/com/android/tradefed/result/suite/XmlSuiteResultFormatter.java
index c378d09e8..22af8e191 100644
--- a/src/com/android/tradefed/result/suite/XmlSuiteResultFormatter.java
+++ b/src/com/android/tradefed/result/suite/XmlSuiteResultFormatter.java
@@ -31,7 +31,6 @@ import com.android.tradefed.result.TestStatus;
 import com.android.tradefed.result.error.ErrorIdentifier;
 import com.android.tradefed.testtype.Abi;
 import com.android.tradefed.testtype.IAbi;
-import com.android.tradefed.testtype.suite.TestFailureListener;
 import com.android.tradefed.util.AbiUtils;
 import com.android.tradefed.util.StreamUtil;
 import com.android.tradefed.util.proto.TfMetricProtoUtil;
@@ -443,7 +442,7 @@ public class XmlSuiteResultFormatter implements IFormatterGenerator {
         return fullStackTrace;
     }
 
-    /** Add files captured by {@link TestFailureListener} on test failures. */
+    /** Add files captured on test failures. */
     private static void HandleLoggedFiles(
             XmlSerializer serializer, Entry<String, TestResult> testResult)
             throws IllegalArgumentException, IllegalStateException, IOException {
@@ -731,7 +730,7 @@ public class XmlSuiteResultFormatter implements IFormatterGenerator {
         }
     }
 
-    /** Add files captured by {@link TestFailureListener} on test failures. */
+    /** Add files captured on test failures. */
     private static void parseLoggedFiles(XmlPullParser parser, TestRunResult currentModule)
             throws XmlPullParserException, IOException {
         if (parser.getName().equals(BUGREPORT_TAG)) {
diff --git a/src/com/android/tradefed/testtype/suite/GranularRetriableTestWrapper.java b/src/com/android/tradefed/testtype/suite/GranularRetriableTestWrapper.java
index ec0938174..d73cedf64 100644
--- a/src/com/android/tradefed/testtype/suite/GranularRetriableTestWrapper.java
+++ b/src/com/android/tradefed/testtype/suite/GranularRetriableTestWrapper.java
@@ -89,7 +89,6 @@ public class GranularRetriableTestWrapper implements IRemoteTest, ITestCollector
     private IRemoteTest mTest;
     private ModuleDefinition mModule;
     private List<IMetricCollector> mRunMetricCollectors;
-    private TestFailureListener mFailureListener;
     private IInvocationContext mModuleInvocationContext;
     private IConfiguration mModuleConfiguration;
     private ModuleListener mMainGranularRunListener;
@@ -109,17 +108,15 @@ public class GranularRetriableTestWrapper implements IRemoteTest, ITestCollector
     public GranularRetriableTestWrapper(
             IRemoteTest test,
             ITestInvocationListener mainListener,
-            TestFailureListener failureListener,
             List<ITestInvocationListener> moduleLevelListeners,
             int maxRunLimit) {
-        this(test, null, mainListener, failureListener, moduleLevelListeners, maxRunLimit);
+        this(test, null, mainListener, moduleLevelListeners, maxRunLimit);
     }
 
     public GranularRetriableTestWrapper(
             IRemoteTest test,
             ModuleDefinition module,
             ITestInvocationListener mainListener,
-            TestFailureListener failureListener,
             List<ITestInvocationListener> moduleLevelListeners,
             int maxRunLimit) {
         mTest = test;
@@ -129,7 +126,6 @@ public class GranularRetriableTestWrapper implements IRemoteTest, ITestCollector
             context = module.getModuleInvocationContext();
         }
         initializeGranularRunListener(mainListener, context);
-        mFailureListener = failureListener;
         mModuleLevelListeners = moduleLevelListeners;
         mMaxRunLimit = maxRunLimit;
     }
@@ -248,10 +244,6 @@ public class GranularRetriableTestWrapper implements IRemoteTest, ITestCollector
 
         mRetryAttemptForwarder = new RetryLogSaverResultForwarder(mLogSaver, currentTestListener);
         ITestInvocationListener runListener = mRetryAttemptForwarder;
-        if (mFailureListener != null) {
-            mFailureListener.setLogger(mRetryAttemptForwarder);
-            currentTestListener.add(mFailureListener);
-        }
 
         // The module collectors itself are added: this list will be very limited.
         // We clone them since the configuration object is shared across shards.
diff --git a/src/com/android/tradefed/testtype/suite/ITestSuite.java b/src/com/android/tradefed/testtype/suite/ITestSuite.java
index b29ae63c5..563aa297c 100644
--- a/src/com/android/tradefed/testtype/suite/ITestSuite.java
+++ b/src/com/android/tradefed/testtype/suite/ITestSuite.java
@@ -158,13 +158,12 @@ public abstract class ITestSuite
     private static final Set<String> ALLOWED_PREPARERS_CONFIGS =
             ImmutableSet.of("/suite/allowed-preparers.txt", "/suite/google-allowed-preparers.txt");
 
-    // Options for test failure case
+    @Deprecated
     @Option(
-        name = "bugreport-on-failure",
-        description =
-                "Take a bugreport on every test failure. Warning: This may require a lot"
-                        + "of storage space of the machine running the tests."
-    )
+            name = "bugreport-on-failure",
+            description =
+                    "Take a bugreport on every test failure. Warning: This may require a lot"
+                            + "of storage space of the machine running the tests.")
     private boolean mBugReportOnFailure = false;
 
     @Deprecated
@@ -189,8 +188,8 @@ public abstract class ITestSuite
     )
     private boolean mScreenshotOnFailure = false;
 
-    @Option(name = "reboot-on-failure",
-            description = "Reboot the device after every test failure.")
+    @Deprecated
+    @Option(name = "reboot-on-failure", description = "Reboot the device after every test failure.")
     private boolean mRebootOnFailure = false;
 
     // Options for suite runner behavior
@@ -777,10 +776,6 @@ public abstract class ITestSuite
             }
         }
 
-        /** Setup a special listener to take actions on test failures. */
-        TestFailureListener failureListener =
-                new TestFailureListener(
-                        mContext.getDevices(), mBugReportOnFailure, mRebootOnFailure);
         /** Create the list of listeners applicable at the module level. */
         List<ITestInvocationListener> moduleListeners = createModuleListeners();
 
@@ -889,8 +884,7 @@ public abstract class ITestSuite
                             TestInformation.createModuleTestInfo(
                                     testInfo, module.getModuleInvocationContext());
                     try {
-                        runSingleModule(
-                                module, moduleInfo, listener, moduleListeners, failureListener);
+                        runSingleModule(module, moduleInfo, listener, moduleListeners);
                     } finally {
                         module.getModuleInvocationContext()
                                 .addInvocationAttribute(
@@ -1001,8 +995,7 @@ public abstract class ITestSuite
             ModuleDefinition module,
             TestInformation moduleInfo,
             ITestInvocationListener listener,
-            List<ITestInvocationListener> moduleListeners,
-            TestFailureListener failureListener)
+            List<ITestInvocationListener> moduleListeners)
             throws DeviceNotAvailableException {
         Map<String, String> properties = new LinkedHashMap<>();
         try (CloseableTraceScope ignored = new CloseableTraceScope("module_pre_check")) {
@@ -1057,7 +1050,6 @@ public abstract class ITestSuite
                 moduleInfo,
                 listener,
                 moduleListeners,
-                failureListener,
                 getConfiguration().getRetryDecision().getMaxRetryCount());
 
         if (!mSkipAllSystemStatusCheck && !mSystemStatusCheckers.isEmpty()) {
diff --git a/src/com/android/tradefed/testtype/suite/ModuleDefinition.java b/src/com/android/tradefed/testtype/suite/ModuleDefinition.java
index 04f351d09..505c19e8a 100644
--- a/src/com/android/tradefed/testtype/suite/ModuleDefinition.java
+++ b/src/com/android/tradefed/testtype/suite/ModuleDefinition.java
@@ -393,7 +393,7 @@ public class ModuleDefinition implements Comparable<ModuleDefinition>, ITestColl
      */
     public final void run(TestInformation moduleInfo, ITestInvocationListener listener)
             throws DeviceNotAvailableException {
-        run(moduleInfo, listener, null, null);
+        run(moduleInfo, listener, null);
     }
 
     /**
@@ -402,16 +402,14 @@ public class ModuleDefinition implements Comparable<ModuleDefinition>, ITestColl
      *
      * @param listener the {@link ITestInvocationListener} where to report results.
      * @param moduleLevelListeners The list of listeners at the module level.
-     * @param failureListener a particular listener to collect logs on testFail. Can be null.
      * @throws DeviceNotAvailableException in case of device going offline.
      */
     public final void run(
             TestInformation moduleInfo,
             ITestInvocationListener listener,
-            List<ITestInvocationListener> moduleLevelListeners,
-            TestFailureListener failureListener)
+            List<ITestInvocationListener> moduleLevelListeners)
             throws DeviceNotAvailableException {
-        run(moduleInfo, listener, moduleLevelListeners, failureListener, 1);
+        run(moduleInfo, listener, moduleLevelListeners, 1);
     }
 
     /**
@@ -429,7 +427,6 @@ public class ModuleDefinition implements Comparable<ModuleDefinition>, ITestColl
             TestInformation moduleInfo,
             ITestInvocationListener listener,
             List<ITestInvocationListener> moduleLevelListeners,
-            TestFailureListener failureListener,
             int maxRunLimit)
             throws DeviceNotAvailableException {
         mMaxRetry = maxRunLimit;
@@ -440,7 +437,7 @@ public class ModuleDefinition implements Comparable<ModuleDefinition>, ITestColl
         // Load extra configuration for the module from module_controller
         // TODO: make module_controller a full TF object
         boolean skipTestCases = false;
-        RunStrategy rs = applyConfigurationControl(failureListener);
+        RunStrategy rs = applyConfigurationControl();
         if (RunStrategy.FULL_MODULE_BYPASS.equals(rs)) {
             CLog.d("module_controller applied and module %s should not run.", getId());
             return;
@@ -581,7 +578,6 @@ public class ModuleDefinition implements Comparable<ModuleDefinition>, ITestColl
                         prepareGranularRetriableWrapper(
                                 test,
                                 listener,
-                                failureListener,
                                 moduleLevelListeners,
                                 skipTestCases,
                                 perModuleRetryQuota);
@@ -699,9 +695,6 @@ public class ModuleDefinition implements Comparable<ModuleDefinition>, ITestColl
                 CLog.e(e);
                 tearDownException = e;
             } finally {
-                if (failureListener != null) {
-                    failureListener.join();
-                }
                 InvocationMetricLogger
                         .addInvocationPairMetrics(InvocationMetricKey.MODULE_TEARDOWN_PAIR,
                                 cleanStartTime, getCurrentTime());
@@ -768,13 +761,12 @@ public class ModuleDefinition implements Comparable<ModuleDefinition>, ITestColl
     GranularRetriableTestWrapper prepareGranularRetriableWrapper(
             IRemoteTest test,
             ITestInvocationListener listener,
-            TestFailureListener failureListener,
             List<ITestInvocationListener> moduleLevelListeners,
             boolean skipTestCases,
             int maxRunLimit) {
         GranularRetriableTestWrapper retriableTest =
                 new GranularRetriableTestWrapper(
-                        test, this, listener, failureListener, moduleLevelListeners, maxRunLimit);
+                        test, this, listener, moduleLevelListeners, maxRunLimit);
         retriableTest.setModuleId(getId());
         retriableTest.setMarkTestsSkipped(skipTestCases);
         retriableTest.setMetricCollectors(mRunMetricCollectors);
@@ -1415,8 +1407,7 @@ public class ModuleDefinition implements Comparable<ModuleDefinition>, ITestColl
      * @param failureListener The {@link TestFailureListener} taking actions on tests failures.
      * @return The strategy to use to run the tests.
      */
-    private RunStrategy applyConfigurationControl(TestFailureListener failureListener)
-            throws DeviceNotAvailableException {
+    private RunStrategy applyConfigurationControl() throws DeviceNotAvailableException {
         List<?> ctrlObjectList = mModuleConfiguration.getConfigurationObjectList(MODULE_CONTROLLER);
         if (ctrlObjectList == null) {
             return RunStrategy.RUN;
@@ -1426,10 +1417,6 @@ public class ModuleDefinition implements Comparable<ModuleDefinition>, ITestColl
                 BaseModuleController controller = (BaseModuleController) ctrlObject;
                 // Track usage of the controller
                 TfObjectTracker.countWithParents(controller.getClass());
-                // module_controller can also control the log collection for the one module
-                if (failureListener != null) {
-                    failureListener.applyModuleConfiguration(controller.shouldCaptureBugreport());
-                }
                 if (!controller.shouldCaptureLogcat()) {
                     mRunMetricCollectors.removeIf(c -> (c instanceof LogcatOnFailureCollector));
                 }
diff --git a/src/com/android/tradefed/testtype/suite/TestFailureListener.java b/src/com/android/tradefed/testtype/suite/TestFailureListener.java
deleted file mode 100644
index 2b4a103a5..000000000
--- a/src/com/android/tradefed/testtype/suite/TestFailureListener.java
+++ /dev/null
@@ -1,136 +0,0 @@
-/*
- * Copyright (C) 2016 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package com.android.tradefed.testtype.suite;
-
-import com.android.tradefed.device.DeviceNotAvailableException;
-import com.android.tradefed.device.DeviceProperties;
-import com.android.tradefed.device.ITestDevice;
-import com.android.tradefed.invoker.tracing.CloseableTraceScope;
-import com.android.tradefed.log.ITestLogger;
-import com.android.tradefed.log.LogUtil.CLog;
-import com.android.tradefed.result.ITestInvocationListener;
-import com.android.tradefed.result.InputStreamSource;
-import com.android.tradefed.result.LogDataType;
-import com.android.tradefed.result.TestDescription;
-import com.android.tradefed.util.IRunUtil;
-import com.android.tradefed.util.RunUtil;
-
-import com.google.common.annotations.VisibleForTesting;
-
-import java.util.List;
-
-/**
- * Listener used to take action such as screenshot, bugreport, logcat collection upon a test failure
- * when requested.
- */
-public class TestFailureListener implements ITestInvocationListener {
-
-    private List<ITestDevice> mListDevice;
-    private ITestLogger mLogger;
-    // Settings for the whole invocation
-    private boolean mBugReportOnFailure;
-    private boolean mRebootOnFailure;
-
-    // module specific values
-    private boolean mModuleBugReportOnFailure = true;
-
-    public TestFailureListener(
-            List<ITestDevice> devices, boolean bugReportOnFailure, boolean rebootOnFailure) {
-        mListDevice = devices;
-        mBugReportOnFailure = bugReportOnFailure;
-        mRebootOnFailure = rebootOnFailure;
-    }
-
-    /** {@inheritDoc} */
-    @Override
-    public void testFailed(TestDescription test, String trace) {
-        CLog.i("FailureListener.testFailed %s %b", test.toString(), mBugReportOnFailure);
-        for (ITestDevice device : mListDevice) {
-            captureFailure(device, test);
-        }
-    }
-
-    /** Capture the appropriate logs for one device for one test failure. */
-    private void captureFailure(ITestDevice device, TestDescription test) {
-        String serial = device.getSerialNumber();
-        if (mBugReportOnFailure && mModuleBugReportOnFailure) {
-            try (CloseableTraceScope ignored = new CloseableTraceScope("bugreport_on_failure")) {
-                if (!device.logBugreport(
-                        String.format("%s-%s-bugreport", test.toString(), serial), mLogger)) {
-                    CLog.e("Failed to capture bugreport for %s failure on %s.", test, serial);
-                }
-            }
-        }
-        if (mRebootOnFailure) {
-            try (CloseableTraceScope ignored = new CloseableTraceScope("reboot_on_failure")) {
-                // Rebooting on all failures can hide legitimate issues and platform instabilities,
-                // therefore only allowed on "user-debug" and "eng" builds.
-                if ("user".equals(device.getProperty(DeviceProperties.BUILD_TYPE))) {
-                    CLog.e("Reboot-on-failure should only be used during development," +
-                            " this is a\" user\" build device");
-                } else {
-                    device.reboot();
-                }
-            } catch (DeviceNotAvailableException e) {
-                CLog.e(e);
-                CLog.e("Device %s became unavailable while rebooting", serial);
-            }
-        }
-    }
-
-    /** Join on all the logcat capturing threads to ensure they terminate. */
-    public void join() {
-        // Reset the module config to use the invocation settings by default.
-        mModuleBugReportOnFailure = true;
-    }
-
-    /**
-     * Forward the log to the logger, do not do it from whitin the #testLog callback as if
-     * TestFailureListener is part of the chain, it will results in an infinite loop.
-     */
-    public void testLogForward(
-            String dataName, LogDataType dataType, InputStreamSource dataStream) {
-        mLogger.testLog(dataName, dataType, dataStream);
-    }
-
-    @Override
-    public void testLog(String dataName, LogDataType dataType, InputStreamSource dataStream) {
-        // Explicitly do nothing on testLog
-    }
-
-    /**
-     * Get the default {@link IRunUtil} instance
-     */
-    @VisibleForTesting
-    IRunUtil getRunUtil() {
-        return RunUtil.getDefault();
-    }
-
-    /**
-     * Allows to override the invocation settings of capture on failure by the module specific
-     * configurations.
-     *
-     * @param bugreportOnFailure true to capture a bugreport on test failure. False otherwise.
-     */
-    public void applyModuleConfiguration(boolean bugreportOnFailure) {
-        mModuleBugReportOnFailure = bugreportOnFailure;
-    }
-
-    /** Sets where the logs should be saved. */
-    public void setLogger(ITestLogger logger) {
-        mLogger = logger;
-    }
-}