[BattOr] Add real device smoke test

common/battor/battor/battor_wrapper_devicetest.py

Index: common/battor/battor/battor_wrapper_devicetest.py
diff --git a/common/battor/battor/battor_wrapper_devicetest.py b/common/battor/battor/battor_wrapper_devicetest.py
new file mode 100644
index 0000000000000000000000000000000000000000..dfbce16278081fdb971299930572934c0047328a
--- /dev/null
+++ b/common/battor/battor/battor_wrapper_devicetest.py
@@ -0,0 +1,70 @@
+# Copyright 2016 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+import logging
+import platform
+import os
+import sys
+import time
+import unittest
+
+if __name__ == '__main__':
+  sys.path.append(
+      os.path.join(os.path.dirname(__file__), '..'))
+
+from battor import battor_wrapper
+from devil.utils import battor_device_mapping
+from devil.utils import find_usb_devices
+
+
+_SUPPORTED_CQ_PLATFORMS = ['win']

[nednguyen, 2016/04/26 04:02:53]

Let's add a TODO to expand this to mac & android

[charliea (OOO until 10-5), 2016/04/26 13:30:38]

My opinion: just get rid of this list altogether and do a direct check for 'win'
in those places instead. Add a TODO to broaden it later.

It's kind of a weird mix of platform agnosticism and windows specific stuff
right now.

[rnephew (Reviews Here), 2016/04/26 15:15:45]

Android would be linux in this case, and with the next patch it will be enabled.
Adding todo for mac.

+
+class BattorWrapperDeviceTest(unittest.TestCase):
+  def setUp(self):
+    test_platform = platform.system()
+
+    if 'Win' in test_platform:

[charliea (OOO until 10-5), 2016/04/26 13:30:38]

Should we just choke for now if test_platform != 'Win'?

[rnephew (Reviews Here), 2016/04/26 15:15:45]

Not here we shouldn't, this is just the setup portion.

+      self._platform = 'win'
+    elif 'Linux' in test_platform:
+      self._platform = 'linux'
+      device_tree  = find_usb_devices.GetBusNumberToDeviceTreeMap()
+      self._battor_list = battor_device_mapping.GetBattorList(device_tree)
+    elif 'Darwin' in test_platform:
+      self._platform = 'mac'
+
+  def testFullRun(self):
+    if self._platform not in _SUPPORTED_CQ_PLATFORMS:
+      logging.critical('Platform %s is not supported on CQ.' % self._platform)
+      return
+    if self._platform != 'win' and len(self._battor_list) < 1:

[charliea (OOO until 10-5), 2016/04/26 13:30:38]

I think this can just be: if not self._battor_list:

[charliea (OOO until 10-5), 2016/04/26 13:30:38]

Didn't you just check the Windows part of this this above?

[rnephew (Reviews Here), 2016/04/26 15:15:44]

It needs the windows part, because on windows we expect the battor_list to be
None since the usb discovery tools do not work there.

[rnephew (Reviews Here), 2016/04/26 15:15:45]

Thats for test setup that runs before every test, where it sets variables that
will be needed for every test.

+      logging.critical('No BattOrs attached. Cannot run tests.')
+      return # No battors, cannot run test.

[charliea (OOO until 10-5), 2016/04/26 13:30:38]

s/battors/BattOr

[rnephew (Reviews Here), 2016/04/26 15:15:45]

Done.

+    battor = battor_wrapper.BattorWrapper(self._platform)
+    battor.StartShell()
+    battor.StartTracing()
+    time.sleep(1)

[nednguyen, 2016/04/26 15:10:57]

Can you add comment explaining why we need this 1 second sleep?
+Charlie: I think this is could be a battor bug.

[rnephew (Reviews Here), 2016/04/26 15:18:31]

I was finding the clock sync marker would be flaky if there wasn't a sleep
there. I couldn't determine if the flakiness was caused by it being called too
shortly after starting tracing, or if it was being called too close to the end
of tracing.

[nednguyen, 2016/04/27 17:51:58]

Yeah, what I meant was to add code comment.

+    battor.RecordClockSyncMarker('abc')
+    time.sleep(1)
+    battor.StopTracing()
+    if self._platform == 'win':

[charliea (OOO until 10-5), 2016/04/26 13:30:38]

You can already be sure that we're on Windows due to the SUPPORTED_CQ_PLATFORMS
check above.

[rnephew (Reviews Here), 2016/04/26 15:15:45]

Not anymore, since I added linux support.

+      # This is a work around for crbug.com/603971.

[charliea (OOO until 10-5), 2016/04/26 13:30:39]

How is this a workaround for that bug? It looks like that bug has to do w/
autodetection of the right COM port on Windows

[rnephew (Reviews Here), 2016/04/26 15:15:45]

It was the wrong bug, fixed. It was suppose to be the one for it crashing on
windows.

+      time.sleep(5)
+      battor._battor_shell.kill()
+    results = battor.CollectTraceData()
+    self.assertTrue('# BattOr' in results[0])
+    self.assertTrue('# voltage_range' in results[1])
+    self.assertTrue('# current_range' in results[2])
+    self.assertTrue('# sample_rate' in results[3])
+    # First line with results. Should be 3 'words'.
+    self.assertTrue(len(results[4].split()) == 3)
+    clock_sync_found = False
+    for entry in results:
+      if '<abc>' in entry:
+        clock_sync_found = True
+        break
+    self.assertTrue(clock_sync_found, 'BattOr Data:%s\n' % repr(results))

[nednguyen, 2016/04/26 04:02:53]

nits: 2 blank line after this

[rnephew (Reviews Here), 2016/04/26 15:15:44]

Done.

+if __name__ == '__main__':
+  logging.getLogger().setLevel(logging.DEBUG)
+  unittest.main(verbosity=2)
+

[nednguyen, 2016/04/26 04:02:53]

nits: no trailing blank line

[rnephew (Reviews Here), 2016/04/26 15:15:45]

Done.