Only start the logcatonfailure from the testRunStart
in some cases, runners might never trigger any testRunStart
and End which would leak logcat threads.
Ensure we only initiate capture from testRunStart.
Test: unit tests
Bug: 123704553
Change-Id: I299657b8386119155893f280cdf499d98baed273
diff --git a/src/com/android/tradefed/device/metric/LogcatOnFailureCollector.java b/src/com/android/tradefed/device/metric/LogcatOnFailureCollector.java
index f240536..50ac676 100644
--- a/src/com/android/tradefed/device/metric/LogcatOnFailureCollector.java
+++ b/src/com/android/tradefed/device/metric/LogcatOnFailureCollector.java
@@ -19,9 +19,7 @@
import com.android.tradefed.device.ILogcatReceiver;
import com.android.tradefed.device.ITestDevice;
import com.android.tradefed.device.LogcatReceiver;
-import com.android.tradefed.invoker.IInvocationContext;
import com.android.tradefed.metrics.proto.MetricMeasurement.Metric;
-import com.android.tradefed.result.ITestInvocationListener;
import com.android.tradefed.result.InputStreamSource;
import com.android.tradefed.result.LogDataType;
import com.android.tradefed.result.TestDescription;
@@ -44,20 +42,6 @@
private Map<ITestDevice, Integer> mOffset = new HashMap<>();
@Override
- public ITestInvocationListener init(
- IInvocationContext context, ITestInvocationListener listener) {
- ITestInvocationListener init = super.init(context, listener);
- for (ITestDevice device : getDevices()) {
- initReceiver(device);
- }
- getRunUtil().sleep(100);
- for (ITestDevice device : getDevices()) {
- mLogcatReceivers.get(device).clear();
- }
- return init;
- }
-
- @Override
public void onTestRunStart(DeviceMetricData runData) {
for (ITestDevice device : getDevices()) {
// In case of multiple runs for the same test runner, re-init the receiver.
@@ -98,12 +82,7 @@
@Override
public void onTestRunEnd(DeviceMetricData runData, Map<String, Metric> currentRunMetrics) {
- for (ILogcatReceiver receiver : mLogcatReceivers.values()) {
- receiver.stop();
- receiver.clear();
- }
- mLogcatReceivers.clear();
- mOffset.clear();
+ clearReceivers();
}
@VisibleForTesting
@@ -123,4 +102,13 @@
receiver.start();
}
}
+
+ private void clearReceivers() {
+ for (ILogcatReceiver receiver : mLogcatReceivers.values()) {
+ receiver.stop();
+ receiver.clear();
+ }
+ mLogcatReceivers.clear();
+ mOffset.clear();
+ }
}
diff --git a/tests/src/com/android/tradefed/device/metric/LogcatOnFailureCollectorTest.java b/tests/src/com/android/tradefed/device/metric/LogcatOnFailureCollectorTest.java
index c4b6f4a..7df1b4e 100644
--- a/tests/src/com/android/tradefed/device/metric/LogcatOnFailureCollectorTest.java
+++ b/tests/src/com/android/tradefed/device/metric/LogcatOnFailureCollectorTest.java
@@ -15,6 +15,7 @@
*/
package com.android.tradefed.device.metric;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import com.android.tradefed.config.ConfigurationDef;
@@ -94,7 +95,6 @@
public void testCollect() throws Exception {
mMockReceiver.start();
mMockReceiver.clear();
- EasyMock.expectLastCall().times(2);
mMockReceiver.stop();
mMockListener.testRunStarted("runName", 1);
TestDescription test = new TestDescription("class", "test");
@@ -130,11 +130,21 @@
}
@Test
+ public void testCollect_noRuns() throws Exception {
+ // If there was no runs, nothing should be done.
+ EasyMock.replay(mMockListener, mMockDevice, mMockReceiver);
+ mTestListener = mCollector.init(mContext, mMockListener);
+ EasyMock.verify(mMockListener, mMockDevice, mMockReceiver);
+ assertFalse(mCollector.mOnTestStartCalled);
+ assertFalse(mCollector.mOnTestFailCalled);
+ }
+
+ @Test
public void testCollect_multiRun() throws Exception {
mMockReceiver.start();
EasyMock.expectLastCall().times(2);
mMockReceiver.clear();
- EasyMock.expectLastCall().times(3);
+ EasyMock.expectLastCall().times(2);
mMockReceiver.stop();
EasyMock.expectLastCall().times(2);
mMockListener.testRunStarted("runName", 1);
