Chromium Codereview Comparison Script.

tools/compare_codereview.py

Index: tools/compare_codereview.py
diff --git a/tools/compare_codereview.py b/tools/compare_codereview.py
new file mode 100644
index 0000000000000000000000000000000000000000..3abf93841ae1b5d40a57211250cedef093f7e041
--- /dev/null
+++ b/tools/compare_codereview.py
@@ -0,0 +1,254 @@
+#!/usr/bin/python2
+
+# Copyright 2014 Google Inc.
+#
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+"""Skia's Chromium Codereview Comparison Script.
+
+This script takes two Codereview URLs, looks at the trybot results for
+the two codereviews and compares the results.
+
+Usage:
+  compare_codereview.py CONTROL_URL ROLL_URL
+"""
+
+import collections
+import os
+import re
+import sys
+import urllib2
+import HTMLParser
+
+
+class CodeReviewHTMLParser(HTMLParser.HTMLParser):
+    """parses CodeReview web pages.
+    """
+    # pylint: disable=I0011,R0904 
+    @staticmethod
+    def parse(url):
+        """Returns a dictionary of {bot_name:CodeReviewHTMLParser.Status}
+        """

[borenet, 2014/01/21 21:55:43]

I'd prefer that you use the following docstring format:

"""Short description.

Args:
    somearg: type; description.

Returns:
    type; description.
"""


... but this isn't a big deal.

[hal.canary, 2014/01/22 15:25:32]

Done.

+        parser = CodeReviewHTMLParser()
+        try:
+            parser.feed(urllib2.urlopen(url).read())
+        except (urllib2.URLError,):
+            print >> sys.stderr, 'Error getting', url
+            return None
+        parser.close()
+        return parser.statuses
+
+    Status = collections.namedtuple('Status', ['status', 'url'])
+
+    def __init__(self):
+        HTMLParser.HTMLParser.__init__(self)
+        self._id = None
+        self._status = None
+        self._href = None
+        self._anchor_data = None
+        # statuses is a dictionary of CodeReviewHTMLParser.Status
+        self.statuses = {}
+
+    def handle_starttag(self, tag, attrs):

[borenet, 2014/01/21 21:55:43]

I'd appreciate a docstring here and elsewhere, even though these are documented
in the official documentation.

[hal.canary, 2014/01/22 15:25:32]

I'll just note that I'm overriding a method and copy some text from the
specification.

+        attrs = dict(attrs)
+        if tag == 'div':
+            id_attr = attrs.get('id','')
+            if id_attr.startswith('tryjobdiv'):
+                self._id = id_attr
+        if (self._id and tag == 'a'
+            and 'build-result' in attrs.get('class', '').split()):
+            self._status = attrs.get('status')
+            self._href = attrs.get('href')
+            self._anchor_data = ''
+
+    def handle_endtag(self, tag):
+        if tag == 'a' and self._status:
+            bot = self._anchor_data.strip()
+            stat = CodeReviewHTMLParser.Status(status=self._status,
+                                               url=self._href)
+            if bot:
+                self.statuses[bot] = stat
+            self._anchor_data = None
+            self._status = None
+            self._href = None
+
+    def handle_data(self, data):
+        if self._anchor_data is not None:
+            self._anchor_data += data

[borenet, 2014/01/21 21:55:43]

I find this flow hard to follow; I think I'd rather have extra variables called
"_currently_parsing_trybotdiv" or something like that instead of using the
variables you're modifying, or alternatively just add lots of explanation.

For the record, I've found that a simple regex is usually easier than
HTMLParser.

[hal.canary, 2014/01/22 15:25:32]

Done.

+
+
+class BuilderHTMLParser(HTMLParser.HTMLParser):
+    """parses Trybot web pages.
+    """
+    # pylint: disable=I0011,R0904 
+    @staticmethod
+    def parse(url):
+        """Returns an array of BuilderHTMLParser.Results, each a
+        description of failure results, along with an optional url.
+        """
+        parser = BuilderHTMLParser()
+        try:
+            parser.feed(urllib2.urlopen(url).read())
+        except (urllib2.URLError,):
+            print >> sys.stderr, 'Error getting', url
+            return []
+        parser.close()
+        return parser.failure_results
+
+    Result = collections.namedtuple('Result', ['text', 'url'])
+        
+    def __init__(self):
+        HTMLParser.HTMLParser.__init__(self)
+        self.failure_results = []
+        self._current_failure_result = None
+        self._divlevel = None
+        self._li_level = 0
+        self._li_data = ''
+        self._current_failure = False
+        self._failure_results_url = ''
+
+    def handle_starttag(self, tag, attrs):
+        attrs = dict(attrs)
+        if tag == 'li':
+            self._li_level += 1
+            return
+        if tag == 'div' and attrs.get('class') == 'failure result':
+            if self._li_level > 0:
+                self._current_failure = True
+            return
+
+        if tag == 'a' and self._current_failure:
+            href = attrs.get('href')
+            if href.endswith('/logs/stdio'):
+                self._failure_results_url = href
+                
+    def handle_endtag(self, tag):
+        if tag == 'li':
+            self._li_level -= 1
+            if 0 == self._li_level:
+                if self._current_failure:
+                    result = self._li_data.strip()
+                    first = result.split()[0]
+                    if first:
+                        result = re.sub(r'^%s(\s+%s)+' % (first, first),
+                                        first, result)
+                    result = re.sub(r'unexpected flaky.*', '', result)
+                    result = re.sub(r'\bpreamble\b', '', result)
+                    result = re.sub(r'\bstdio\b', '', result)
+                    url = self._failure_results_url
+                    self.failure_results.append(
+                        BuilderHTMLParser.Result(result, url))
+                    self._current_failure_result = None
+                self._current_failure = False
+                self._li_data = ''
+                self._failure_results_url = ''
+
+    def handle_data(self, data):
+        if self._current_failure:
+            self._li_data += data
+
+
+def printer(indent, string):
+    """Print indented, wrapped text.
+    """
+    def wrap_to(line, columns):
+        """Wrap a line to the given number of columns, return a list
+        of strings.
+        """
+        ret = []
+        nextline = ''
+        for word in line.split():
+            if nextline:
+                if len(nextline) + 1 + len(word) > columns:
+                    ret.append(nextline)
+                    nextline = word
+                else:
+                    nextline += (' ' + word)
+            else:
+                nextline = word
+        if nextline:
+            ret.append(nextline)
+        return ret    
+    out = sys.stdout
+    spacer = '  '
+    for line in string.split('\n'):
+        for i, wrapped_line in enumerate(wrap_to(line, 68 - (2 * indent))):
+            out.write(spacer * indent)
+            if i > 0:
+                out.write(spacer)
+            out.write(wrapped_line)
+            out.write('\n')
+    out.flush()
+
+
+def main(control_url, roll_url, verbosity):

[borenet, 2014/01/21 21:55:43]

Optional: you could add a default verbosity level here.

[hal.canary, 2014/01/22 15:25:32]

Done.

+    """Compare two Codereview URLs
+
+    Args:
+        control_url, roll_url: (strings) URL of the format
+            https://codereview.chromium.org/?????????
+
+        verbosity: (int) verbose level.  0, 1, or 2.
+    """
+    # pylint: disable=I0011,R0914,R0912
+    control = CodeReviewHTMLParser.parse(control_url)
+    roll = CodeReviewHTMLParser.parse(roll_url)
+    if not (control and roll):
+        return
+
+    control_name = '[control %s]' % control_url.split('/')[-1]
+    roll_name = '[roll %s]' % roll_url.split('/')[-1]
+    all_bots = set(control) & set(roll)
+
+    if verbosity > 0:
+        print '%11s %11s %4s %s' % ('CONTROL', 'ROLL', 'DIFF', 'BOT')
+        print 
+        for bot in sorted(all_bots):
+            if control[bot].status != roll[bot].status:
+                diff = '****'
+            elif (control[bot].status != 'success' or
+                roll[bot].status != 'success'):
+                diff = '....'
+            else:
+                diff = ''
+            print '%11s %11s %4s %s' % (
+                control[bot].status, roll[bot].status, diff, bot)
+        print
+        sys.stdout.flush()
+
+    for bot in sorted(all_bots):
+        if (roll[bot].status == 'success'):
+            if verbosity > 1:
+                print '\n==%s==' % bot
+                printer(1, 'OK')
+            continue
+        print '\n==%s==' % bot
+
+        for (status, name, url) in (
+            (control[bot].status, control_name, control[bot].url),
+            (roll[bot].status, roll_name, roll[bot].url)):
+
+            if status == 'failure':
+                printer(1, name)
+                results = BuilderHTMLParser.parse(url)
+                for result in results:
+                    formatted_result = re.sub(r'(\S*\.html) ', '\n__\g<1>\n',
+                                              result.text)
+                    printer(2, formatted_result)
+                    if ('compile' in result.text
+                        or '...and more' in result.text):
+                        printer(3, re.sub('/[^/]*$', '/', url) + result.url )
+            else:
+                printer(1, name)
+                printer(2, status)
+
+            
+if __name__ == '__main__':
+    if len(sys.argv) < 3:
+        print >> sys.stderr, __doc__
+        exit(1)
+    main(sys.argv[1], sys.argv[2],
+         int(os.environ.get('COMPARE_CODEREVIEW_VERBOSITY', 1)))
+
+