Refactor logcat collector.

By default collect logcat on every test end and optionally choose
to collect only on test failed.

Provide option to include the logcat duration before the test
started.

Optionally pass only the logcat directory in the run metrics.

Bug: b/171826266

Test: atest CollectorDeviceLibTest:android.device.collectors.LogcatCollectorTest
Change-Id: Ie18437110095da2a6a8ee2f52a0d04c329454884
diff --git a/libraries/device-collectors/src/hostsidetests/AndroidTest.xml b/libraries/device-collectors/src/hostsidetests/AndroidTest.xml
index 4dc015d..c01a1e4 100644
--- a/libraries/device-collectors/src/hostsidetests/AndroidTest.xml
+++ b/libraries/device-collectors/src/hostsidetests/AndroidTest.xml
@@ -19,7 +19,7 @@
         <option name="class" value="com.android.collectors.DeviceCollectorsTest" />
         <option name="class" value="com.android.collectors.ScreenshotOnFailureCollectorHostTest" />
         <option name="class" value="com.android.collectors.BatterystatsCollectorHostTest" />
-        <option name="class" value="com.android.collectors.LogcatOnFailureCollectorHostTest" />
+        <option name="class" value="com.android.collectors.LogcatCollectorHostTest" />
         <option name="class" value="com.android.loggers.DeviceFileLoggerHostTest" />
     </test>
 </configuration>
diff --git a/libraries/device-collectors/src/hostsidetests/src/com/android/collectors/LogcatOnFailureCollectorHostTest.java b/libraries/device-collectors/src/hostsidetests/src/com/android/collectors/LogcatCollectorHostTest.java
similarity index 96%
rename from libraries/device-collectors/src/hostsidetests/src/com/android/collectors/LogcatOnFailureCollectorHostTest.java
rename to libraries/device-collectors/src/hostsidetests/src/com/android/collectors/LogcatCollectorHostTest.java
index df51e73..76ba4be 100644
--- a/libraries/device-collectors/src/hostsidetests/src/com/android/collectors/LogcatOnFailureCollectorHostTest.java
+++ b/libraries/device-collectors/src/hostsidetests/src/com/android/collectors/LogcatCollectorHostTest.java
@@ -50,13 +50,13 @@
  * <p>tradefed.sh run commandAndExit template/local_min --template:map test=CollectorHostsideLibTest
  */
 @RunWith(DeviceJUnit4ClassRunner.class)
-public class LogcatOnFailureCollectorHostTest extends BaseHostJUnit4Test {
+public class LogcatCollectorHostTest extends BaseHostJUnit4Test {
     private static final String TEST_APK = "CollectorDeviceLibTest.apk";
     private static final String PACKAGE_NAME = "android.device.collectors";
     private static final String AJUR_RUNNER = "androidx.test.runner.AndroidJUnitRunner";
 
     private static final String LOGCAT_COLLECTOR =
-            "android.device.collectors.LogcatOnFailureCollector";
+            "android.device.collectors.LogcatCollector";
 
     private IInvocationContext mContext;
     private DeviceTestRunOptions mOptions = null;
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
new file mode 100644
index 0000000..16ba778
--- /dev/null
+++ b/libraries/device-collectors/src/main/java/android/device/collectors/LogcatCollector.java
@@ -0,0 +1,210 @@
+/*
+ * Copyright (C) 2019 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 android.device.collectors;
+
+import android.device.collectors.annotations.OptionClass;
+import android.os.Bundle;
+import android.util.Log;
+
+import androidx.annotation.VisibleForTesting;
+
+import org.junit.runner.Description;
+import org.junit.runner.notification.Failure;
+import org.junit.runner.Result;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Date;
+import java.util.HashMap;
+import java.text.SimpleDateFormat;
+
+/**
+ * A {@link LogcatCollector} that captures logcat after each test.
+ *
+ * This class needs external storage permission. See {@link BaseMetricListener} how to grant
+ * external storage permission, especially at install time.
+ *
+ */
+@OptionClass(alias = "logcat-collector")
+public class LogcatCollector extends BaseMetricListener {
+    @VisibleForTesting
+    static final SimpleDateFormat DATE_FORMATTER = new SimpleDateFormat("MM-dd HH:mm:ss.SSS");
+
+    @VisibleForTesting static final String METRIC_SEP = "-";
+    @VisibleForTesting static final String FILENAME_SUFFIX = "logcat";
+    @VisibleForTesting static final String BEFORE_LOGCAT_DURATION_SECS =
+            "before-logcat-duration-secs";
+    @VisibleForTesting static final String COLLECT_ON_FAILURE_ONLY = "collect-on-failure-only";
+    @VisibleForTesting static final String RETURN_LOGCAT_DIR = "return-logcat-directory";
+    @VisibleForTesting static final String DEFAULT_DIR = "run_listeners/logcats";
+
+    private static final int BUFFER_SIZE = 16 * 1024;
+
+
+    private File mDestDir;
+    private String mStartTime = null;
+    private boolean mTestFailed = false;
+    // Logcat duration to include before the test starts.
+    private long mBeforeLogcatDurationInSecs = 0;
+    // Use this flag to enable logcat collection only when the test fails.
+    private boolean mCollectOnlyTestFailed = false;
+    // Use this flag to return the root directory of the logcat files in run metrics
+    // otherwise individual logcat file will be reported associated with the test.
+    // The final directory which contains all the logcat files will be <DEFAULT_DIR>_all.
+    private boolean mReturnLogcatDir = false;
+
+    // Map to keep track of test iterations for multiple test iterations.
+    private HashMap<Description, Integer> mTestIterations = new HashMap<>();
+
+    public LogcatCollector() {
+        super();
+    }
+
+    /**
+     * Constructor to simulate receiving the instrumentation arguments. Should not be used except
+     * for testing.
+     */
+    @VisibleForTesting
+    LogcatCollector(Bundle args) {
+        super(args);
+    }
+
+    @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.
+        mStartTime = getLogcatStartTime();
+    }
+
+    @Override
+    public void onTestStart(DataRecord testData, Description description) {
+        // Capture the start time for logcat purpose.
+        // Overwrites any start time set prior to the test and adds custom
+        // duration to capture before current start time.
+        mStartTime = getLogcatStartTime();
+        // Keep track of test iterations.
+        mTestIterations.computeIfPresent(description, (desc, iteration) -> iteration + 1);
+        mTestIterations.computeIfAbsent(description, desc -> 1);
+    }
+
+    /**
+     * Mark the test as failed if this is called. The actual collection will be done in {@link
+     * onTestEnd} to ensure that all actions around a test failure end up in the logcat.
+     */
+    @Override
+    public void onTestFail(DataRecord testData, Description description, Failure failure) {
+        mTestFailed = true;
+    }
+
+    /**
+     * Collect the logcat at the end of each test or collect the logcat only on test
+     * failed if the flag is enabled.
+     */
+    @Override
+    public void onTestEnd(DataRecord testData, Description description) {
+        if (!mCollectOnlyTestFailed || (mCollectOnlyTestFailed && mTestFailed)) {
+            // Capture logcat from start time
+            if (mDestDir == null) {
+                return;
+            }
+            try {
+                int iteration = mTestIterations.get(description);
+                final String fileName =
+                        String.format(
+                                "%s.%s%s%s-logcat.txt",
+                                description.getClassName(),
+                                description.getMethodName(),
+                                iteration == 1 ? "" : (METRIC_SEP + String.valueOf(iteration)),
+                                METRIC_SEP + FILENAME_SUFFIX);
+                File logcat = new File(mDestDir, fileName);
+                getLogcatSince(mStartTime, logcat);
+                if (!mReturnLogcatDir) {
+                    // Do not return individual logcat file path if the logcat directory
+                    // option is enabled. Logcat root directory path will be returned in the
+                    // test run status.
+                    testData.addFileMetric(String.format("%s_%s", getTag(), logcat.getName()),
+                            logcat);
+                }
+            } catch (IOException | InterruptedException e) {
+                Log.e(getTag(), "Error trying to retrieve logcat.", e);
+            }
+        }
+        // Reset the flag here, as onTestStart might not have been called if a @BeforeClass method
+        // fails.
+        mTestFailed = false;
+        // Update the start time here in case onTestStart() is not called for the next test. If it
+        // is called, the start time will be overwritten.
+        mStartTime = getLogcatStartTime();
+    }
+
+    @Override
+    public void onTestRunEnd(DataRecord runData, Result result) {
+        if (mReturnLogcatDir) {
+            runData.addStringMetric(getTag(), mDestDir.getAbsolutePath().toString());
+        }
+    }
+
+    /** @hide */
+    @VisibleForTesting
+    protected void getLogcatSince(String startTime, File saveTo)
+            throws IOException, InterruptedException {
+        // ProcessBuilder is used here in favor of UiAutomation.executeShellCommand() because the
+        // logcat command requires the timestamp to be quoted which in Java requires
+        // Runtime.exec(String[]) or ProcessBuilder to work properly, and UiAutomation does not
+        // support this for now.
+        ProcessBuilder pb = new ProcessBuilder(Arrays.asList("logcat", "-t", startTime));
+        pb.redirectOutput(saveTo);
+        Process proc = pb.start();
+        // Make the process blocking to ensure consistent behavior.
+        proc.waitFor();
+    }
+
+    @VisibleForTesting
+    protected String getLogcatStartTime() {
+        Date date = new Date(System.currentTimeMillis());
+        Log.i(getTag(), "Current Date:" + DATE_FORMATTER.format(date));
+        if (mBeforeLogcatDurationInSecs > 0) {
+            date = new Date(System.currentTimeMillis() - (mBeforeLogcatDurationInSecs * 1000));
+            Log.i(getTag(), "Date including the before duration:" + DATE_FORMATTER.format(date));
+        }
+        return DATE_FORMATTER.format(date);
+    }
+
+    /**
+     * Add custom options if available.
+     */
+    private void setupAdditionalArgs() {
+        Bundle args = getArgsBundle();
+
+        if (args.getString(BEFORE_LOGCAT_DURATION_SECS) != null) {
+            mBeforeLogcatDurationInSecs = Long
+                    .parseLong(args.getString(BEFORE_LOGCAT_DURATION_SECS));
+        }
+
+        if (args.getString(COLLECT_ON_FAILURE_ONLY) != null) {
+            mCollectOnlyTestFailed = Boolean.parseBoolean(args.getString(COLLECT_ON_FAILURE_ONLY));
+        }
+
+        if (args.getString(RETURN_LOGCAT_DIR) != null) {
+            mReturnLogcatDir = Boolean
+                    .parseBoolean(args.getString(RETURN_LOGCAT_DIR));
+        }
+
+    }
+}
diff --git a/libraries/device-collectors/src/main/java/android/device/collectors/LogcatOnFailureCollector.java b/libraries/device-collectors/src/main/java/android/device/collectors/LogcatOnFailureCollector.java
deleted file mode 100644
index 1ead31a..0000000
--- a/libraries/device-collectors/src/main/java/android/device/collectors/LogcatOnFailureCollector.java
+++ /dev/null
@@ -1,153 +0,0 @@
-/*
- * Copyright (C) 2019 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 android.device.collectors;
-
-import android.device.collectors.annotations.OptionClass;
-import android.os.Bundle;
-import android.util.Log;
-
-import androidx.annotation.VisibleForTesting;
-
-import org.junit.runner.Description;
-import org.junit.runner.notification.Failure;
-
-import java.io.File;
-import java.io.IOException;
-import java.util.Arrays;
-import java.util.Date;
-import java.util.HashMap;
-import java.text.SimpleDateFormat;
-
-/**
- * A {@link BaseMetricListener} that captures logcat after each test case failure.
- *
- * This class needs external storage permission. See {@link BaseMetricListener} how to grant
- * external storage permission, especially at install time.
- *
- */
-@OptionClass(alias = "logcat-failure-collector")
-public class LogcatOnFailureCollector extends BaseMetricListener {
-    @VisibleForTesting
-    static final SimpleDateFormat DATE_FORMATTER = new SimpleDateFormat("MM-dd HH:mm:ss.SSS");
-
-    @VisibleForTesting static final String METRIC_SEP = "-";
-    @VisibleForTesting static final String FILENAME_SUFFIX = "logcat";
-
-    public static final String DEFAULT_DIR = "run_listeners/logcats";
-    private static final int BUFFER_SIZE = 16 * 1024;
-
-    private File mDestDir;
-    private String mStartTime = null;
-    private boolean mTestFailed = false;
-
-    // Map to keep track of test iterations for multiple test iterations.
-    private HashMap<Description, Integer> mTestIterations = new HashMap<>();
-
-    public LogcatOnFailureCollector() {
-        super();
-    }
-
-    /**
-     * Constructor to simulate receiving the instrumentation arguments. Should not be used except
-     * for testing.
-     */
-    @VisibleForTesting
-    LogcatOnFailureCollector(Bundle args) {
-        super(args);
-    }
-
-    @Override
-    public void onTestRunStart(DataRecord runData, Description description) {
-        mDestDir = createAndEmptyDirectory(DEFAULT_DIR);
-        // Capture the start time in case onTestStart() is never called due to failure during
-        // @BeforeClass.
-        mStartTime = getCurrentDate();
-    }
-
-    @Override
-    public void onTestStart(DataRecord testData, Description description) {
-        // Capture the start time for logcat purpose.
-        // Overwrites any start time set prior to the test.
-        mStartTime = getCurrentDate();
-        // Keep track of test iterations.
-        mTestIterations.computeIfPresent(description, (desc, iteration) -> iteration + 1);
-        mTestIterations.computeIfAbsent(description, desc -> 1);
-    }
-
-    /**
-     * Mark the test as failed if this is called. The actual collection will be done in {@link
-     * onTestEnd} to ensure that all actions around a test failure end up in the logcat.
-     */
-    @Override
-    public void onTestFail(DataRecord testData, Description description, Failure failure) {
-        mTestFailed = true;
-    }
-
-    /** If the test fails, collect logcat since test start time. */
-    @Override
-    public void onTestEnd(DataRecord testData, Description description) {
-        if (mTestFailed) {
-            // Capture logcat from start time
-            if (mDestDir == null) {
-                return;
-            }
-            try {
-                int iteration = mTestIterations.get(description);
-                final String fileName =
-                        String.format(
-                                "%s.%s%s%s-logcat-on-failure.txt",
-                                description.getClassName(),
-                                description.getMethodName(),
-                                iteration == 1 ? "" : (METRIC_SEP + String.valueOf(iteration)),
-                                METRIC_SEP + FILENAME_SUFFIX);
-                File logcat = new File(mDestDir, fileName);
-                getLogcatSince(mStartTime, logcat);
-                testData.addFileMetric(String.format("%s_%s", getTag(), logcat.getName()), logcat);
-            } catch (IOException | InterruptedException e) {
-                Log.e(getTag(), "Error trying to retrieve logcat.", e);
-            }
-        }
-        // Reset the flag here, as onTestStart might not have been called if a @BeforeClass method
-        // fails.
-        mTestFailed = false;
-        // Update the start time here in case onTestStart() is not called for the next test. If it
-        // is called, the start time will be overwritten.
-        mStartTime = getCurrentDate();
-    }
-
-    /** @hide */
-    @VisibleForTesting
-    protected void getLogcatSince(String startTime, File saveTo)
-            throws IOException, InterruptedException {
-        // ProcessBuilder is used here in favor of UiAutomation.executeShellCommand() because the
-        // logcat command requires the timestamp to be quoted which in Java requires
-        // Runtime.exec(String[]) or ProcessBuilder to work properly, and UiAutomation does not
-        // support this for now.
-        ProcessBuilder pb = new ProcessBuilder(Arrays.asList("logcat", "-t", startTime));
-        pb.redirectOutput(saveTo);
-        Process proc = pb.start();
-        // Make the process blocking to ensure consistent behavior.
-        proc.waitFor();
-    }
-
-    /** @hide */
-    @VisibleForTesting
-    protected String getCurrentDate() {
-        // Get time using system (wall clock) time since this is the time that logcat is based on.
-        Date date = new Date(System.currentTimeMillis());
-        return DATE_FORMATTER.format(date);
-    }
-}
diff --git a/libraries/device-collectors/src/test/java/android/device/collectors/LogcatOnFailureCollectorTest.java b/libraries/device-collectors/src/test/java/android/device/collectors/LogcatCollectorTest.java
similarity index 64%
rename from libraries/device-collectors/src/test/java/android/device/collectors/LogcatOnFailureCollectorTest.java
rename to libraries/device-collectors/src/test/java/android/device/collectors/LogcatCollectorTest.java
index 119211c..7beeec3 100644
--- a/libraries/device-collectors/src/test/java/android/device/collectors/LogcatOnFailureCollectorTest.java
+++ b/libraries/device-collectors/src/test/java/android/device/collectors/LogcatCollectorTest.java
@@ -41,20 +41,25 @@
 import java.text.ParsePosition;
 import java.text.SimpleDateFormat;
 import java.util.Calendar;
+import java.util.Date;
 import java.util.List;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
-/** Unit tests for {@link LogcatOnFailureCollector}. */
+/** Unit tests for {@link LogcatCollector}. */
 @RunWith(AndroidJUnit4.class)
-public final class LogcatOnFailureCollectorTest {
+public final class LogcatCollectorTest {
 
     // A {@code Description} to pass when faking a test run start call.
     private static final Description RUN_DESCRIPTION = Description.createSuiteDescription("run");
     private static final Description TEST_DESCRIPTION =
             Description.createTestDescription("run", "test");
+    private static final Description TEST_2_DESCRIPTION =
+            Description.createTestDescription("run", "test-2");
+    private static final Description TEST_FAILURE_DESCRIPTION =
+            Description.createTestDescription("run", "test-failed");
     // A template for a logcat line for the purpose of this test. The three parameters are type (
     // info, warning, etc.), log tag and message.
     private static final String LOGCAT_REGEX_TEMPLATE =
@@ -64,15 +69,15 @@
     // logcat itself does not include year by default, and it is better not to enable it there
     // as it will just end up being visual noise for the user.
     private static final SimpleDateFormat DATE_FORMATTER =
-            new SimpleDateFormat("yyyy " + LogcatOnFailureCollector.DATE_FORMATTER.toPattern());
+            new SimpleDateFormat("yyyy " + LogcatCollector.DATE_FORMATTER.toPattern());
+    private static final String RUN_METRIC_KEY = "android.device.collectors.LogcatCollector";
 
     private File mLogDir;
-    private LogcatOnFailureCollector mCollector;
+    private LogcatCollector mCollector;
     private Instrumentation mMockInstrumentation;
 
     @Before
     public void setUp() throws Exception {
-        mCollector = new LogcatOnFailureCollector();
         mMockInstrumentation = Mockito.mock(Instrumentation.class);
         mLogDir = new File(Environment.getExternalStorageDirectory(), "test_logcat");
         mLogDir.mkdirs();
@@ -83,38 +88,134 @@
         mCollector.recursiveDelete(mLogDir);
     }
 
-    private LogcatOnFailureCollector initListener() {
-        LogcatOnFailureCollector listener = Mockito.spy(mCollector);
+    private LogcatCollector initListener(Bundle bundle) {
+        mCollector = new LogcatCollector(bundle);
+        LogcatCollector listener = Mockito.spy(mCollector);
         listener.setInstrumentation(mMockInstrumentation);
         Mockito.doReturn(mLogDir).when(listener).createAndEmptyDirectory(Mockito.anyString());
         return listener;
     }
 
     @Test
-    public void testLogcatOnFailure_nofailure() throws Exception {
-        LogcatOnFailureCollector listener = initListener();
+    public void testLogcatCollectionOnEveryTestEnd() throws Exception {
+        LogcatCollector listener = initListener(new Bundle());
         Mockito.doNothing()
                 .when(listener)
                 .getLogcatSince(Mockito.any(String.class), Mockito.any(File.class));
         // Test run start behavior
         listener.testRunStarted(RUN_DESCRIPTION);
 
+        // Test 1 start behavior
+        listener.testStarted(TEST_DESCRIPTION);
+        listener.testFinished(TEST_DESCRIPTION);
+
+        // Test 2 test start behavior
+        listener.testStarted(TEST_2_DESCRIPTION);
+        listener.testFinished(TEST_2_DESCRIPTION);
+
+        listener.testRunFinished(new Result());
+        // AJUR runner is then gonna call instrumentationRunFinished
+
+        Bundle resultBundle = new Bundle();
+        listener.instrumentationRunFinished(System.out, resultBundle, new Result());
+        assertEquals(0, resultBundle.size());
+
+        ArgumentCaptor<Bundle> capture = ArgumentCaptor.forClass(Bundle.class);
+        Mockito.verify(mMockInstrumentation, Mockito.times(2))
+                .sendStatus(Mockito.eq(
+                        SendToInstrumentation.INST_STATUS_IN_PROGRESS), capture.capture());
+        List<Bundle> capturedBundle = capture.getAllValues();
+        assertEquals(2, capturedBundle.size());
+
+        Bundle check1 = capturedBundle.get(0);
+        // Ensure we received the file
+        assertEquals(1, check1.size());
+        // The only key is ours
+        for (String key : check1.keySet()) {
+            assertTrue(
+                    key.contains(
+                            String.join(
+                                    "",
+                                    "run.test",
+                                    LogcatCollector.METRIC_SEP
+                                            + LogcatCollector.FILENAME_SUFFIX,
+                                    "-logcat.txt")));
+
+        }
+
+        Bundle check2 = capturedBundle.get(1);
+        // Ensure we received the file
+        assertEquals(1, check2.size());
+        // The only key is ours
+        for (String key : check2.keySet()) {
+            assertTrue(
+                    key.contains(
+                            String.join(
+                                    "",
+                                    "run.test-2",
+                                    LogcatCollector.METRIC_SEP
+                                            + LogcatCollector.FILENAME_SUFFIX,
+                                    "-logcat.txt")));
+
+        }
+    }
+
+    @Test
+    public void testLogcatCollectionWithBeforeDuration() throws Exception {
+
+        Bundle bundle = new Bundle();
+        bundle.putString(LogcatCollector.BEFORE_LOGCAT_DURATION_SECS, "6");
+        LogcatCollector listener = initListener(bundle);
+
+        ArgumentCaptor<String> captureFinalTime = ArgumentCaptor.forClass(String.class);
+        Mockito.doNothing()
+                .when(listener)
+                .getLogcatSince(captureFinalTime.capture(), Mockito.any(File.class));
+
+        String logTag = this.getClass().getSimpleName() + "_testLogcatCollectionWithBeforeDuration";
+        Log.i(logTag, "Sample Message");
+
+        // Log three lines after a short delay.
+        SystemClock.sleep(4000);
+
+        // Test run start behavior
+        listener.testRunStarted(RUN_DESCRIPTION);
+        Date date = new Date(System.currentTimeMillis());
+
         // Test test start behavior
         listener.testStarted(TEST_DESCRIPTION);
         listener.testFinished(TEST_DESCRIPTION);
         listener.testRunFinished(new Result());
         // AJUR runner is then gonna call instrumentationRunFinished
+
         Bundle resultBundle = new Bundle();
         listener.instrumentationRunFinished(System.out, resultBundle, new Result());
+        assertEquals(0, resultBundle.size());
 
-        Mockito.verify(mMockInstrumentation, Mockito.never())
+        ArgumentCaptor<Bundle> capture = ArgumentCaptor.forClass(Bundle.class);
+        Mockito.verify(mMockInstrumentation)
                 .sendStatus(Mockito.eq(
-                        SendToInstrumentation.INST_STATUS_IN_PROGRESS), Mockito.any());
+                        SendToInstrumentation.INST_STATUS_IN_PROGRESS), capture.capture());
+
+        // Verify logcat since time is 60 secs before the test started time.
+        List<String> capturedList = captureFinalTime.getAllValues();
+        assertEquals(1, capturedList.size());
+        int year = Calendar.getInstance().get(Calendar.YEAR);
+        String finalDateWithYear = year + " " + capturedList.get(0);
+        SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy MM-dd HH:mm:ss.SSS");
+        Date finallogcatSinceDate = dateFormat.parse(finalDateWithYear);
+        Log.i("Time Difference in msecs",
+                String.valueOf(date.getTime() - finallogcatSinceDate.getTime()));
+        assertTrue((date.getTime() - finallogcatSinceDate.getTime()) >= 4000);
     }
 
     @Test
-    public void testLogcatOnFailure() throws Exception {
-        LogcatOnFailureCollector listener = initListener();
+    public void testLogcatCollectionWithDirectoryOption() throws Exception {
+
+        Bundle bundle = new Bundle();
+        bundle.putString(LogcatCollector.RETURN_LOGCAT_DIR, "true");
+        LogcatCollector listener = initListener(bundle);
+
         Mockito.doNothing()
                 .when(listener)
                 .getLogcatSince(Mockito.any(String.class), Mockito.any(File.class));
@@ -123,9 +224,52 @@
 
         // Test test start behavior
         listener.testStarted(TEST_DESCRIPTION);
-        Failure f = new Failure(TEST_DESCRIPTION, new RuntimeException("I failed."));
-        listener.testFailure(f);
         listener.testFinished(TEST_DESCRIPTION);
+
+        // Test test start behavior
+        listener.testStarted(TEST_DESCRIPTION);
+        listener.testFinished(TEST_DESCRIPTION);
+        listener.testRunFinished(new Result());
+
+        // Results available in run metrics.
+        Bundle resultBundle = new Bundle();
+        listener.instrumentationRunFinished(System.out, resultBundle, new Result());
+        assertEquals(1, resultBundle.size());
+
+        // The only key is ours
+        for (String key : resultBundle.keySet()) {
+            assertTrue(key.contains(RUN_METRIC_KEY));
+        }
+
+        ArgumentCaptor<Bundle> capture = ArgumentCaptor.forClass(Bundle.class);
+        Mockito.verify(mMockInstrumentation, Mockito.times(0))
+                .sendStatus(Mockito.eq(
+                        SendToInstrumentation.INST_STATUS_IN_PROGRESS), capture.capture());
+    }
+
+    @Test
+    public void testLogcatOnlyOnTestFailureOption() throws Exception {
+
+        Bundle bundle = new Bundle();
+        bundle.putString(LogcatCollector.COLLECT_ON_FAILURE_ONLY, "true");
+        LogcatCollector listener = initListener(bundle);
+
+        Mockito.doNothing()
+                .when(listener)
+                .getLogcatSince(Mockito.any(String.class), Mockito.any(File.class));
+        // Test run start behavior
+        listener.testRunStarted(RUN_DESCRIPTION);
+
+        // Successful test
+        listener.testStarted(TEST_DESCRIPTION);
+        listener.testFinished(TEST_DESCRIPTION);
+
+        // Failed test.
+        listener.testStarted(TEST_FAILURE_DESCRIPTION);
+        Failure f = new Failure(TEST_FAILURE_DESCRIPTION, new RuntimeException("I failed."));
+        listener.testFailure(f);
+        listener.testFinished(TEST_FAILURE_DESCRIPTION);
+
         listener.testRunFinished(new Result());
         // AJUR runner is then gonna call instrumentationRunFinished
         Bundle resultBundle = new Bundle();
@@ -147,17 +291,17 @@
                     key.contains(
                             String.join(
                                     "",
-                                    "run.test",
-                                    LogcatOnFailureCollector.METRIC_SEP
-                                            + LogcatOnFailureCollector.FILENAME_SUFFIX,
-                                    "-logcat-on-failure.txt")));
+                                    "run.test-failed",
+                                    LogcatCollector.METRIC_SEP
+                                            + LogcatCollector.FILENAME_SUFFIX,
+                                    "-logcat.txt")));
         }
     }
 
     /** Test that the collector can actually retrieve logcat. */
     @Test
     public void testRetrievingLogcat() throws Exception {
-        LogcatOnFailureCollector listener = initListener();
+        LogcatCollector listener = initListener(new Bundle());
         // Test run start behavior
         listener.testRunStarted(RUN_DESCRIPTION);
 
@@ -171,8 +315,6 @@
         Log.w(logTag, "Message 2");
         Log.e(logTag, "Message 3");
         SystemClock.sleep(10);
-        Failure f = new Failure(testDescription, new RuntimeException("I failed."));
-        listener.testFailure(f);
         listener.testFinished(testDescription);
         listener.testRunFinished(new Result());
         // AJUR runner is then gonna call instrumentationRunFinished
@@ -227,7 +369,7 @@
     @Ignore
     @Test
     public void testLogcatTimespan() throws Exception {
-        LogcatOnFailureCollector listener = initListener();
+        LogcatCollector listener = initListener(new Bundle());
 
         listener.testRunStarted(RUN_DESCRIPTION);
         // Store the start time of the test. The logcat should begin after this time.
@@ -241,8 +383,6 @@
         Log.i(logTag, "Message");
         Log.i(logTag, "Another message");
         SystemClock.sleep(10);
-        listener.testFailure(new Failure(testDescription, new RuntimeException("I failed.")));
-        listener.testFinished(testDescription);
         // Store the end time of the test. The logcat should end before this time.
         long endTimeMillis = System.currentTimeMillis();
         listener.testRunFinished(new Result());
@@ -311,7 +451,7 @@
     /** Test that the logcat collection supports multiple iterations. */
     @Test
     public void testMultipleIterations() throws Exception {
-        LogcatOnFailureCollector listener = initListener();
+        LogcatCollector listener = initListener(new Bundle());
         Mockito.doNothing()
                 .when(listener)
                 .getLogcatSince(Mockito.any(String.class), Mockito.any(File.class));
@@ -339,13 +479,13 @@
         assertEquals(0, resultBundle.size());
 
         ArgumentCaptor<Bundle> capture = ArgumentCaptor.forClass(Bundle.class);
-        Mockito.verify(mMockInstrumentation, Mockito.times(2))
+        Mockito.verify(mMockInstrumentation, Mockito.times(3))
                 .sendStatus(
                         Mockito.eq(SendToInstrumentation.INST_STATUS_IN_PROGRESS),
                         capture.capture());
         List<Bundle> capturedBundles = capture.getAllValues();
         // 2 bundles as we have two tests that failed (and thus has metrics).
-        assertEquals(2, capturedBundles.size());
+        assertEquals(3, capturedBundles.size());
 
         // The first bundle should have the first logcat file, for the first iteration.
         Bundle check1 = capturedBundles.get(0);
@@ -354,27 +494,42 @@
                 String.join(
                         "",
                         "run.test",
-                        LogcatOnFailureCollector.METRIC_SEP
-                                + LogcatOnFailureCollector.FILENAME_SUFFIX,
-                        "-logcat-on-failure.txt");
+                        LogcatCollector.METRIC_SEP
+                                + LogcatCollector.FILENAME_SUFFIX,
+                        "-logcat.txt");
         for (String key : check1.keySet()) {
             // The first iteration should not have an iteration number.
             assertTrue(key.contains(expectedKey1));
         }
 
-        // The second bundle should have the second logcat file, for the third iteration.
+        // The second bundle should have the second logcat file, for the second iteration.
         Bundle check2 = capturedBundles.get(1);
         assertEquals(1, check2.size());
         String expectedKey2 =
                 String.join(
                         "",
-                        "run.test-3",
-                        LogcatOnFailureCollector.METRIC_SEP
-                                + LogcatOnFailureCollector.FILENAME_SUFFIX,
-                        "-logcat-on-failure.txt");
+                        "run.test-2",
+                        LogcatCollector.METRIC_SEP
+                                + LogcatCollector.FILENAME_SUFFIX,
+                        "-logcat.txt");
         for (String key : check2.keySet()) {
             // The third iteration should have an iteration number, 3.
             assertTrue(key.contains(expectedKey2));
         }
+
+        // The second bundle should have the second logcat file, for the third iteration.
+        Bundle check3 = capturedBundles.get(2);
+        assertEquals(1, check3.size());
+        String expectedKey3 =
+                String.join(
+                        "",
+                        "run.test-3",
+                        LogcatCollector.METRIC_SEP
+                                + LogcatCollector.FILENAME_SUFFIX,
+                        "-logcat.txt");
+        for (String key : check3.keySet()) {
+            // The third iteration should have an iteration number, 3.
+            assertTrue(key.contains(expectedKey3));
+        }
     }
 }