Constrained Network test does not fail fast under fatal conditions.

chrome/test/functional/media/media_constrained_network_perf.py

 #!/usr/bin/env python
 # Copyright (c) 2011 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.
 
 """Records metrics on playing media under constrained network conditions.
 
 Spins up a Constrained Network Server (CNS) and runs through a test matrix of
 bandwidth, latency, and packet loss settings.  Each run records a
 time-to-playback (TTP) and extra-play-percentage (EPP) metric in a format
 consumable by the Chromium perf bots.
 
 Since even a small number of different settings yields a large test matrix, the
 design is threaded... however PyAuto is not, so a global lock is used when calls
 into PyAuto are necessary.  The number of threads can be set by _TEST_THREADS.
 
 The CNS code is located under: <root>/src/media/tools/constrained_network_server
 """
 
 import itertools
+import logging
 import os
 import Queue
 import subprocess
 import sys
 import threading
 
 import pyauto_media
 import pyauto
 import pyauto_paths
 import pyauto_utils
-
+import urllib2

[DaleCurtis, 2012/01/06 23:19:42]

This should go under threading.

[shadi, 2012/01/07 04:56:19]

Done.

 
 # Settings for each network constraint.
 _BANDWIDTH_SETTINGS_KBPS = {'None': 0, 'Low': 256, 'Medium': 2000, 'High': 5000}
 _LATENCY_SETTINGS_MS = {'None': 0, 'Low': 43, 'Medium': 105, 'High': 180}
 _PACKET_LOSS_SETTINGS_PERCENT = {'None': 0, 'Medium': 2, 'High': 5}
 
 # Test constraints are all possible combination of the above settings.  Each
 # tuple must be of the form (Bandwidth, Latency, Packet Loss).
 _TEST_CONSTRAINTS = itertools.product(
     _BANDWIDTH_SETTINGS_KBPS.values(),
@@ skipping 54 matching lines @@
 
   def _FindTabLocked(self, url):
     """Returns the tab index for the tab belonging to this url.
 
     self._automation_lock must be owned by caller.
     """
     for tab in self._pyauto.GetBrowserInfo()['windows'][0]['tabs']:
       if tab['url'] == url:
         return tab['index']
 
-  def _HaveMetrics(self, unique_url):
-    """Returns true if metrics are ready.  Set self.{_epp,_ttp} < 0 pre-run."""
+  def _HaveTTPMetric(self, unique_url):

[DaleCurtis, 2012/01/06 23:19:42]

These methods are very similar, you should create a generic _HaveMetric() method
which takes a value name.

[shadi, 2012/01/07 04:56:19]

Done.

+    """Returns true if ttp metrics is ready. Set self._ttp < 0 pre-run."""
     with self._automation_lock:
       tab = self._FindTabLocked(unique_url)
+      self._ttp = int(self._pyauto.GetDOMValue('time_to_playback',
+                                                 tab_index=tab))

[DaleCurtis, 2012/01/06 23:19:42]

Bad indent. Please run gpylint before publishing.

[shadi, 2012/01/07 04:56:19]

Done.

+    return self._ttp >= 0
 
-      if self._epp < 0:
-        self._epp = int(self._pyauto.GetDOMValue('extra_play_percentage',
+  def _HaveEPPMetric(self, unique_url):
+    """Returns true if epp metric is ready. Set self._epp < 0 pre-run."""
+    with self._automation_lock:
+      tab = self._FindTabLocked(unique_url)
+      self._epp = int(self._pyauto.GetDOMValue('extra_play_percentage',
                                                  tab_index=tab))
-      if self._ttp < 0:
-        self._ttp = int(self._pyauto.GetDOMValue('time_to_playback',
-                                                 tab_index=tab))
-    return self._epp >= 0 and self._ttp >= 0
+    return self._epp >= 0
 
   def run(self):
     """Opens tab, starts HTML test, and records metrics for each queue entry.
 
     No exception handling is done to make sure the main thread exits properly
     during Chrome crashes or other failures.  Doing otherwise has the potential
     to leave the CNS server running in the background.
 
     For a clean shutdown, put the magic exit value (None, None) in the queue.
     """
@@ skipping 20 matching lines @@
       # Start the test!
       with self._automation_lock:
         self._pyauto.AppendTab(pyauto.GURL(unique_url))
         self._pyauto.CallJavascriptFunc(
             'startTest', [video_url], tab_index=self._FindTabLocked(unique_url))
 
       # Wait until the necessary metrics have been collected.  Okay to not lock
       # here since pyauto.WaitUntil doesn't call into Chrome.
       self._epp = self._ttp = -1
       self._pyauto.WaitUntil(
-        self._HaveMetrics, args=[unique_url], retry_sleep=2,
-        timeout=_TEST_VIDEO_DURATION_SEC * 10)
+        self._HaveTTPMetric, args=[unique_url], retry_sleep=1, timeout= 5,

[DaleCurtis, 2012/01/06 23:19:42]

Bad spacing. Again gpylint is your friend here :)

[shadi, 2012/01/07 04:56:19]

Done.

+        debug=False)
+
+      # Do not wait for epp if ttp is not available.
+      if self._ttp >= 0:

[DaleCurtis, 2012/01/06 23:19:42]

Should we log an error? Skip logging of epp, ttp below?

[shadi, 2012/01/07 04:56:19]

Done.

+        self._pyauto.WaitUntil(
+          self._HaveEPPMetric, args=[unique_url], retry_sleep=2,
+          timeout=_TEST_VIDEO_DURATION_SEC * 10, debug=False)
 
       # Record results.
       # TODO(dalecurtis): Support reference builds.
       series_name = ''.join(name)
       pyauto_utils.PrintPerfResult('epp', series_name, self._epp, '%')
       pyauto_utils.PrintPerfResult('ttp', series_name, self._ttp, 'ms')
 
       # Close the tab.
       with self._automation_lock:
         self._pyauto.GetBrowserWindow(0).GetTab(
             self._FindTabLocked(unique_url)).Close(True)
 
       # TODO(dalecurtis): Check results for regressions.
       self._tasks.task_done()
 
 
+class ProcessLogger(threading.Thread):
+  """A thread to log a process's stderr output."""
+  def __init__(self, process):

[DaleCurtis, 2012/01/06 23:19:42]

Need blank line above this. gpylint... :)

[shadi, 2012/01/07 04:56:19]

Done.

+    """Starts the process logger thread.
+
+    Args:
+      process: The process to log.
+    """
+    threading.Thread.__init__(self)
+    self._process = process
+    self.start()
+
+  def run(self):
+    """Adds debug statements for the process's stderr output."""
+    line = True
+    while line:
+      line = self._process.stderr.readline()
+      logging.debug(line)
+
+
 class MediaConstrainedNetworkPerfTest(pyauto.PyUITest):
   """PyAuto test container.  See file doc string for more information."""
 
   def setUp(self):
     """Starts the Constrained Network Server (CNS)."""
     cmd = [sys.executable, os.path.join(pyauto_paths.GetSourceDir(), _CNS_PATH),
            '--port', str(_CNS_PORT),
            '--interface', 'lo',
            '--www-root', os.path.join(
-               self.DataDir(), 'pyauto_private', 'media')]
+               self.DataDir(), 'pyauto_private', 'media'),
+           '-v']
 
     process = subprocess.Popen(cmd, stderr=subprocess.PIPE)
 
     # Wait for server to start up.
     line = True
     while line:
       line = process.stderr.readline()
+      logging.debug(line)
       if 'STARTED' in line:
         self._server_pid = process.pid
         pyauto.PyUITest.setUp(self)
+        ProcessLogger(process)
+        self.CheckServerAccess()
         return
     self.fail('Failed to start CNS.')
 
+  def CheckServerAccess(self):
+    """Checks is the CNS server can serve a file with no network constraints."""
+    test_url = ''.join([_CNS_BASE_URL, 'f=' ,_TEST_VIDEO])
+    try:
+      urllib2.urlopen(test_url)
+    except:
+      # Need to call teardown since the server has already started.
+      self.tearDown()

[DaleCurtis, 2012/01/06 23:19:42]

This should be called automatically. Did you find it not being called?

[shadi, 2012/01/07 04:56:19]

If setup() fails, tearDown() doesn't get called automatically. 

http://docs.python.org/library/unittest.html#unittest.TestCase.tearDown

On 2012/01/06 23:19:42, DaleCurtis wrote:

+      self.fail('Cannot connect to CNS server.')

[DaleCurtis, 2012/01/06 23:19:42]

Instead of having a new fail here, why not just return False and in the setUp
phase above do something like:

if self._CanAccessServer():
  return

[shadi, 2012/01/07 04:56:19]

Done.

+
   def tearDown(self):
     """Stops the Constrained Network Server (CNS)."""
     pyauto.PyUITest.tearDown(self)
     self.Kill(self._server_pid)
 
   def testConstrainedNetworkPerf(self):
     """Starts CNS, spins up worker threads to run through _TEST_CONSTRAINTS."""
     # Convert relative test path into an absolute path.
     test_url = self.GetFileURLForDataPath(_TEST_HTML_PATH)
 
@@ skipping 14 matching lines @@
       tasks.put((None, None))
 
     # Wait for threads to exit, gracefully or otherwise.
     for thread in threads:
       thread.join()
 
 
 if __name__ == '__main__':
   # TODO(dalecurtis): Process command line parameters here.
   pyauto_media.Main()