Reads bench expectations from skia-autogen svn repo.

slave/skia_slave_scripts/check_for_regressions.py

Index: slave/skia_slave_scripts/check_for_regressions.py
diff --git a/slave/skia_slave_scripts/check_for_regressions.py b/slave/skia_slave_scripts/check_for_regressions.py
index a1c75efc9ad44be1299b703be1228e67aaeeea48..fa828eec8fb32420cb5cfee63ad60ccf0b42e0fa 100644
--- a/slave/skia_slave_scripts/check_for_regressions.py
+++ b/slave/skia_slave_scripts/check_for_regressions.py
@@ -11,6 +11,11 @@ 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):
@@ -21,16 +26,35 @@ class CheckForRegressions(BuildStep):
         **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,