Generalize rebase-errors.py so it works for other files.

net/data/verify_certificate_chain_unittest/rebase-errors.py

Index: net/data/verify_certificate_chain_unittest/rebase-errors.py
diff --git a/net/data/verify_certificate_chain_unittest/rebase-errors.py b/net/data/verify_certificate_chain_unittest/rebase-errors.py
index 0ac03b6600ac7f581ceb0a2f3057e0486ef2f64c..079aa40e746c8ebbda23a45a16451afb364847c5 100755
--- a/net/data/verify_certificate_chain_unittest/rebase-errors.py
+++ b/net/data/verify_certificate_chain_unittest/rebase-errors.py
@@ -14,19 +14,39 @@ To use this run the affected tests, and then pass the input to this script
   $ ./out/Release/net_unittests --gtest_filter="*VerifyCertificateChain*" | \
      net/data/verify_certificate_chain_unittest/rebase-errors.py
 
-The script will search the unit-test (results.txt in above example) and look
-for failure lines and the corresponding actual error string.
+The script will search the unit-test stdout and looking for a failed value of:

[mattm, 2016/09/15 00:42:05]

awkward wording

[eroman, 2016/09/16 01:14:29]

Done.

+   errors.ToDebugString()
 
-It will then go and update the corresponding .pem and .py file.
+It will then go and update the corresponding .pem and/or .py files.
+
+This script works for tests described in .pem files which use a block
+called "ERRORS", and in the C++ side check "errors.ToDebugString()"
 """
 
 import common
-import glob
 import os
 import sys
 import re
 
 
+kFailedTestPattern = r"""

[mattm, 2016/09/15 00:42:05]

do the re.compile for these patterns here rather than where they are used
(mainly so you can see which flags are used with each pattern next to the
pattern itself, but I think it's also more idiomatic / efficient)

[eroman, 2016/09/16 01:14:29]

Done. Agreed that is better! I also added an explanation for the patterns.

+Value of: errors.ToDebugString\(\)
+  Actual: "(.*?)"

[mattm, 2016/09/15 00:42:05]

probably should be greedy (safe since this pattern isn't using DOTALL)

[eroman, 2016/09/16 01:14:29]

Done.

+(?:.|\n)+?
+Test file: (.*?)

[mattm, 2016/09/15 00:42:05]

here too

[eroman, 2016/09/16 01:14:29]

Done.

+"""
+
+
+kErrorsBlockPattern = r"""
+-----END .*-----

[mattm, 2016/09/15 00:42:05]

non-greedy?

[eroman, 2016/09/16 01:14:29]

Done. (Thanks for the help over IM on this).

+
+(.*?
+-----BEGIN ERRORS-----
+.*?
+-----END ERRORS-----
+)"""
+
+
 def read_file_to_string(path):
   """Reads a file entirely to a string"""
   with open(path, 'r') as f:
@@ -40,36 +60,19 @@ def write_string_to_file(data, path):
     f.write(data)
 
 
-def get_file_paths_for_test(test_name):
-  """Returns the file paths (as a tuple) that define a particular unit test.
-  For instance given test name 'IntermediateLacksBasicConstraints' it would
-  return the paths to:
-
-   * intermediate-lacks-basic-constraints.pem,
-   * generate-intermediate-lacks-basic-constraints.py
-  """
-  # The directory that this python script is stored in.
-  base_dir = os.path.dirname(os.path.realpath(__file__))
-
-  # The C++ test name is just a camel case verson of the file name. Rather than
-  # converting directly from camel case to a file name, it is simpler to just
-  # scan the file list and see which matches. (Not efficient but good enough).
-  paths = glob.glob(os.path.join(base_dir, '*.pem'))
-
-  for pem_path in paths:
-    file_name = os.path.basename(pem_path)
-    file_name_no_extension = os.path.splitext(file_name)[0]
+def get_py_path(pem_path):
+  """Returns the .py filepath used to generate the given .pem path if there is
+  one. Otherwise returns None."""
 
-    # Strip the hyphens in file name to bring it closer to the camel case.
-    transformed = file_name_no_extension.replace('-', '')
+  base_dir = os.path.dirname(pem_path)
+  if not base_dir.endswith('/verify_certificate_chain_unittest'):

[mattm, 2016/09/15 00:42:05]

reasoning for hard coding this? (Instead of say, building the .py path and
testing if it exists)

[mattm, 2016/09/15 00:42:05]

os.path.split(base_dir)[1]

[eroman, 2016/09/16 01:14:29]

Done.

+    return None
 
-    # Now all that differs is the case.
-    if transformed.lower() == test_name.lower():
-      py_file_name = 'generate-' + file_name_no_extension + '.py'
-      py_path = os.path.join(base_dir, py_file_name)
-      return (pem_path, py_path)
+  file_name = os.path.basename(pem_path)
+  file_name_no_extension = os.path.splitext(file_name)[0]
 
-  return None
+  py_file_name = 'generate-' + file_name_no_extension + '.py'
+  return os.path.join(base_dir, py_file_name)
 
 
 def replace_string(original, start, end, replacement):
@@ -81,14 +84,15 @@ def fixup_pem_file(path, actual_errors):
   """Updates the ERRORS block in the test .pem file"""
   contents = read_file_to_string(path)
 
-  # This assumes that ERRORS is the last thing in file, and comes after the
-  # VERIFY_RESULT block.
-  kEndVerifyResult = '-----END VERIFY_RESULT-----'
-  contents = contents[0:contents.index(kEndVerifyResult)]
-  contents += kEndVerifyResult
-  contents += '\n'
-  contents += '\n'
-  contents += common.text_data_to_pem('ERRORS', actual_errors)
+  prog = re.compile(kErrorsBlockPattern, re.MULTILINE | re.DOTALL)
+  m = prog.search(contents)
+
+  if not m:
+    print "Couldn't find ERRORS block in %s" % (path)
+    return
+
+  contents = replace_string(contents, m.start(1), m.end(1),
+                            common.text_data_to_pem('ERRORS', actual_errors))
 
   # Update the file.
   write_string_to_file(contents, path)
@@ -110,21 +114,30 @@ def fixup_py_file(path, actual_errors):
   write_string_to_file(contents, path)
 
 
-def fixup_test(test_name, actual_errors):
-  """Updates the test files used by |test_name|, setting the expected error to
-  |actual_errors|"""
+def get_abs_path(rel_path):
+  """Converts |rel_path| (relative to src) to a full path"""
+  # Assume the current script lives in a well known location.
+  kScriptDir = "net/data/verify_certificate_chain_unittest"
+  script_dir = os.path.dirname(os.path.realpath(__file__))
+  if not script_dir.endswith(kScriptDir):
+    print "Script is not in expected location: " % (kScriptDir)

[mattm, 2016/09/15 00:42:05]

format string doesn't contain %s

[eroman, 2016/09/16 01:14:29]

Done

+    exit(1)

[mattm, 2016/09/15 00:42:05]

sys.exit

[eroman, 2016/09/16 01:14:29]

... clearly I didn't test this failure case.

 
-  # Determine the paths for the corresponding *.pem file and generate-*.py
-  pem_path, py_path = get_file_paths_for_test(test_name)
+  src_dir = script_dir[0:-len(kScriptDir)]

[mattm, 2016/09/15 00:42:05]

instead of hardcoding all that, how about something like:

  src_dir = os.path.dirname(os.path.realpath(__file__))
  while True:
    parent_dir, dirname = os.path.split(src_dir)
    if dirname == "src":
      break
    if not parent_dir or parent_dir == src_dir:
      print "couldn't find src dir"
      sys.exit(1)
    src_dir = parent_dir

[eroman, 2016/09/16 01:14:29]

Done.

+  return src_dir + rel_path

[mattm, 2016/09/15 00:42:05]

os.path.join

[eroman, 2016/09/16 01:14:29]

Done.

 
-  fixup_pem_file(pem_path, actual_errors)
-  fixup_py_file(py_path, actual_errors)
 
+def fixup_errors_for_file(actual_errors, pem_path):
+  """Updates the errors in |test_file_path| (.pem file) to match
+  |actual_errors|"""
+
+  fixup_pem_file(pem_path, actual_errors)
 
-kTestNamePattern = (r'^\[ RUN      \] VerifyCertificateChain/'
-                    'VerifyCertificateChainSingleRootTest/0\.(.*)$')
-kValueOfLine = 'Value of: errors.ToDebugString()'
-kActualPattern = '^  Actual: "(.*)"$'
+  # Tests in verify_certificate_chain_unittests additionally have a .py
+  # generator file to update.
+  py_path = get_py_path(pem_path)
+  if py_path:
+    fixup_py_file(py_path, actual_errors)
 
 
 def main():
@@ -140,28 +153,13 @@ def main():
     print 'Reading input from stdin...'
     test_stdout = sys.stdin.read()
 
-  lines = test_stdout.split('\n')
-
-  # Iterate over each line of the unit test stdout.
-  for i in range(len(lines) - 3):
-    # Figure out the name of the test.
-    m = re.search(kTestNamePattern, lines[i])
-    if not m:
-      continue
-    test_name = m.group(1)
-
-    # Confirm that it is a failure having to do with the errors.
-    if lines[i + 2] != kValueOfLine:
-      continue
-
-    # Get the actual error text (which in gtest output is escaped).
-    m = re.search(kActualPattern, lines[i + 3])
-    if not m:
-      continue
-    actual = m.group(1)
-    actual = actual.decode('string-escape')
+  prog = re.compile(kFailedTestPattern, re.MULTILINE)
 
-    fixup_test(test_name, actual)
+  for m in prog.finditer(test_stdout):
+    actual_errors = m.group(1)
+    actual_errors = actual_errors.decode('string-escape')
+    relative_test_path = m.group(2)
+    fixup_errors_for_file(actual_errors, get_abs_path(relative_test_path))
 
 
 if __name__ == "__main__":