Reads bench expectations from skia-autogen svn repo.

slave/skia_slave_scripts/check_for_regressions.py

 #!/usr/bin/env python
 # Copyright (c) 2012 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.
 
 """ Check for regressions in bench data. """
 
 from build_step import BuildStep
 from utils import shell_utils
 
 import builder_name_schema
 import os
 import sys
+import urllib2
+
+# URL prefix for getting bench expectations from skia-autogen svn repo.
+AUTOGEN_BENCH_EXPECTATIONS_URL_PREFIX = ('http://skia-autogen.googlecode.com/'
+                                         'svn/bench/')
 
 
 class CheckForRegressions(BuildStep):
   def __init__(self, timeout=600, no_output_timeout=600, **kwargs):
     super(CheckForRegressions, self).__init__(
         timeout=timeout,
         no_output_timeout=no_output_timeout,
         **kwargs)
 
   def _RunInternal(self, representation):
+    # Reads expectations from skia-autogen svn repo.
+    http_response = None
+    expectations_filename = ('bench_expectations_' +
+        builder_name_schema.GetWaterfallBot(self.builder_name) + '.txt')
+    url = AUTOGEN_BENCH_EXPECTATIONS_URL_PREFIX + expectations_filename
+    try:
+      http_response = urllib2.urlopen(url)
+    except urllib2.HTTPError, e:
+      if e.code == 404:
+        print 'Skip due to missing expectations: %s' % url

[borenet, 2014/03/13 20:45:37]

You never check again to see whether http_response is None... Can we just return
here?

[benchen, 2014/03/14 01:41:10]

I must've deleted the line by mistake..
On 2014/03/13 20:45:37, borenet wrote:

+      else:
+        raise Exception('HTTPError while reading expectations: %s' % e.code)
+    except urllib2.URLError, e:
+      raise Exception('URLError while reading expectations: %s' % e.reason)

[borenet, 2014/03/13 20:45:37]

I would very strongly prefer to perform a source code checkout (or even just
"svn cat") instead of using HTTP.  The codesite HTTP interface has proven to be
extremely unreliable in the past.

[benchen, 2014/03/14 01:41:10]

I think svn cat is a good idea. Switched to using it.
On 2014/03/13 20:45:37, borenet wrote:

+
     path_to_check_bench_regressions = os.path.join('bench',
         'check_bench_regressions.py')
-    # TODO(borenet): We should move these expectations into expectations/bench.
+
+    # Writes the expectations from svn repo to the local file.
     path_to_bench_expectations = os.path.join(
-        'bench',
-        'bench_expectations_%s.txt' % builder_name_schema.GetWaterfallBot(
-            self._builder_name))
-    if not os.path.isfile(path_to_bench_expectations):
-      print 'Skip due to missing expectations: %s' % path_to_bench_expectations
-      return
+        self._perf_range_input_dir, expectations_filename)
+    if os.path.exists(self._perf_range_input_dir):
+      os.remove(path_to_bench_expectations)
+    else:
+      os.makedirs(self._perf_range_input_dir)

[borenet, 2014/03/13 20:45:37]

Won't the below write replace the file if it already exists?  I think we could
probably change the above four lines to just:

os.makedirs(self._perf_range_input_dir)

[benchen, 2014/03/14 01:41:10]

oh I wasn't sure about it. Done.

+    file_handle = open(path_to_bench_expectations, 'w')
+    file_handle.write(http_response.read())
+    file_handle.close()

[borenet, 2014/03/13 20:45:37]

I'd think the "with" syntax is cleaner:

with open(path_to_bench_expectations, 'w') as file_handle:
  file_handle.write(http_response.read())

[benchen, 2014/03/14 01:41:10]

Done.

+
     cmd = ['python', path_to_check_bench_regressions,
            '-a', representation,
            '-b', self._builder_name,
            '-d', self._perf_data_dir,
            '-e', path_to_bench_expectations,
            '-r', self._got_revision,
            ]
 
     shell_utils.run(cmd)
 
   def _Run(self):
     if self._perf_data_dir:
       self._RunInternal('25th')
 
 
 if '__main__' == __name__:
   sys.exit(BuildStep.RunBuildStep(CheckForRegressions))