[PATCH] Modify LitTestCommand class to accept optional argument to parse summary lines only

Siva Chandra sivachandra at google.com
Wed Feb 25 13:59:35 PST 2015


LGTM with nits.


================
Comment at: zorg/buildbot/commands/LitTestCommand.py:23
@@ +22,3 @@
+  failingCodes = set(['FAIL', 'XPASS', 'KPASS', 'UNRESOLVED', 'TIMEOUT'])
+  # Regular expressions to indicate start of result line.
+  kTestResultRE = re.compile(r'^Failing Tests \(\d*\)$')
----------------
May be a better comment would be:

Regular expression for start of summary marker.

================
Comment at: zorg/buildbot/commands/LitTestCommand.py:24
@@ -23,1 +23,3 @@
+  # Regular expressions to indicate start of result line.
+  kTestResultRE = re.compile(r'^Failing Tests \(\d*\)$')
 
----------------
s/kTestResultRE/kSummaryStartRE ?

================
Comment at: zorg/buildbot/commands/LitTestCommand.py:26
@@ -23,3 +25,3 @@
 
-  def __init__(self, maxLogs=None):
+  def __init__(self, maxLogs=None, isSummaryOnly=False):
     LogLineObserver.__init__(self)
----------------
s/isSummaryOnly/parseSummaryOnly

================
Comment at: zorg/buildbot/commands/LitTestCommand.py:38
@@ +37,3 @@
+    # If it's true, current line will be parsed as result steps
+    # If false, current line will be skipped
+    self.parserStarted = not isSummaryOnly
----------------
May be a better comment would be:

The current line is parsed only if |parseLine| is True.

================
Comment at: zorg/buildbot/commands/LitTestCommand.py:39
@@ -35,1 +38,3 @@
+    # If false, current line will be skipped
+    self.parserStarted = not isSummaryOnly
 
----------------
s/parserStarted/parseLine

================
Comment at: zorg/buildbot/commands/LitTestCommand.py:105
@@ -99,8 +104,3 @@
 
-    # Check for a new test status line.
-    m = self.kTestLineRE.match(line.strip())
-    if m:
-      # Remember the last test result and update the result counts.
-      self.lastTestResult = (code, name) = m.groups()
-      self.resultCounts[code] = self.resultCounts.get(code, 0) + 1
-      return
+    if self.kTestResultRE.match(line):
+      self.parserStarted = True;
----------------
Add a comment here to explain why and when this check is required.

http://reviews.llvm.org/D7877

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list