Make it possible to stop BatteryStats fully by calling stop_profiling

AndroidRunner/Plugins/batterystats/Batterystats.py

@@ -78,13 +78,15 @@ def get_data(self, device, application):
         # TODO: Check if 'systrace freq idle' is supported by the device
         global sysproc
 
-        sysproc = subprocess.Popen(
-            '{} freq idle -e {} -a {} -t {} -o {}'.format(self.systrace, device.id, application, int(self.duration + 5),
-                                                          systrace_file), shell=True)
-
-    # FIX
+        # Run systrace in another thread.
+        cmd = '{} freq idle -e {} -a {} -o {}'.format(self.systrace, device.id, application,
+                                                          systrace_file) 
+        cmd = cmd.split()
+        sysproc = subprocess.Popen(cmd, stdin=subprocess.PIPE)
 
     def stop_profiling(self, device, **kwargs):
+        # batterystats does not need to be stopped. systrace is stopped in the collect_results function
+        # since it needs to run a little bit longer than the rest (see original implementation where it runs for 5 secs longer)
         self.profile = False
 
     # Pull logcat file from device
@@ -111,8 +113,8 @@ def get_consumed_joules(device):
         return energy_consumed_j
 
     def get_systrace_results(self, device):
-        # Wait for Systrace file finalisation before parsing
-        sysproc.wait()
+        stdout, stderr = sysproc.communicate(input=b"\n")
+
         cores = int(device.shell('cat /proc/cpuinfo | grep processor | wc -l'))
 
         systrace_results = []

tests/unit/test_plugins.py

@@ -477,8 +477,7 @@ def test_get_data(self, popen_mock, time_mock, batterystats_plugin, mock_device,
         batterystats_plugin.start_profiling(mock_device)  # start profiling call get_data
 
         capsys.readouterr()  # Catch print
-        popen_mock.assert_called_once_with('systrace freq idle -e 123 -a com.android.chrome -t 5 -o {}'
-                                           .format(op.join(str(tmpdir), 'systrace_123_strftime.html')), shell=True)
+        popen_mock.assert_called_once_with(["systrace", "freq", "idle", "-e", "123", "-a", "com.android.chrome", "-o", "{}".format(op.join(str(tmpdir), "systrace_123_strftime.html"))], stdin=-1)
 
     def test_stop_profiling(self, batterystats_plugin, mock_device):
         batterystats_plugin.profile = True
@@ -617,7 +616,9 @@ def test_collect_results(self, pull_logcat_mock, get_batterystats_results_mock,
     def test_get_systrace_result(self, popen_mock, time_mock, parse_mock, batterystats_plugin, mock_device, tmpdir,
                                  capsys):
         # set global variables
-        popen_return_value = Mock()
+        proc = Mock()
+        proc.communicate.return_value = [Mock(), Mock()]
+        popen_return_value = proc
         popen_mock.return_value = popen_return_value
         parse_return_value = Mock()
         parse_mock.return_value = parse_return_value
@@ -635,7 +636,7 @@ def test_get_systrace_result(self, popen_mock, time_mock, parse_mock, batterysta
         get_sysrace_result = batterystats_plugin.get_systrace_results(mock_device)
 
         assert get_sysrace_result == parse_return_value
-        popen_return_value.wait.assert_called_once()
+        popen_return_value.communicate.assert_called_once()
         parse_mock.assert_called_once_with('com.android.chrome', op.join(str(tmpdir), 'systrace_123_strftime.html'),
                                            op.join(str(tmpdir), 'logcat_123_strftime.txt'),
                                            op.join(str(tmpdir), 'batterystats_history_123_strftime.txt'),
@@ -647,7 +648,9 @@ def test_get_systrace_result(self, popen_mock, time_mock, parse_mock, batterysta
     def test_get_systrace_result_parsing_disabled(self, popen_mock, time_mock, parse_mock, batterystats_plugin_systrace_parsing_disabled, mock_device, tmpdir,
                                  capsys):
         # set global variables
-        popen_return_value = Mock()
+        proc = Mock()
+        proc.communicate.return_value = [Mock(), Mock()]
+        popen_return_value = proc
         popen_mock.return_value = popen_return_value
         parse_return_value = Mock()
         parse_mock.return_value = parse_return_value
@@ -665,7 +668,7 @@ def test_get_systrace_result_parsing_disabled(self, popen_mock, time_mock, parse
         get_sysrace_result = batterystats_plugin_systrace_parsing_disabled.get_systrace_results(mock_device)
 
         assert get_sysrace_result == []
-        popen_return_value.wait.assert_called_once()
+        popen_return_value.communicate.assert_called_once()
         parse_mock.assert_not_called()