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

BUG=634443

Review-Url: https://codereview.chromium.org/2346663002
Cr-Commit-Position: refs/heads/master@{#419295}

net/cert/internal/path_builder_unittest.cc

@@ -497,18 +497,19 @@ class PathBuilderKeyRolloverTest : public ::testing::Test {
     bool unused_result;
     std::string unused_errors;
 
-    ReadVerifyCertChainTestFromFile("key-rollover-oldchain.pem", &path,
-                                    &oldroot_, &time_, &unused_result,
-                                    &unused_errors);
+    ReadVerifyCertChainTestFromFile(
+        "net/data/verify_certificate_chain_unittest/key-rollover-oldchain.pem",
+        &path, &oldroot_, &time_, &unused_result, &unused_errors);
     ASSERT_EQ(2U, path.size());
     target_ = path[0];
     oldintermediate_ = path[1];
     ASSERT_TRUE(target_);
     ASSERT_TRUE(oldintermediate_);
 
-    ReadVerifyCertChainTestFromFile("key-rollover-longrolloverchain.pem", &path,
-                                    &oldroot_, &time_, &unused_result,
-                                    &unused_errors);
+    ReadVerifyCertChainTestFromFile(
+        "net/data/verify_certificate_chain_unittest/"
+        "key-rollover-longrolloverchain.pem",
+        &path, &oldroot_, &time_, &unused_result, &unused_errors);
     ASSERT_EQ(4U, path.size());
     newintermediate_ = path[1];
     newroot_ = path[2];

net/cert/internal/path_builder_verify_certificate_chain_unittest.cc

@@ -19,7 +19,8 @@ class PathBuilderDelegate {
                      const scoped_refptr<TrustAnchor>& trust_anchor,
                      const der::GeneralizedTime& time,
                      bool expected_result,
-                     const std::string& expected_errors) {
+                     const std::string& expected_errors,
+                     const std::string& test_file_path) {
     SimpleSignaturePolicy signature_policy(1024);
     ASSERT_FALSE(chain.empty());
 

net/cert/internal/test_helpers.cc

@@ -101,7 +101,7 @@ der::Input SequenceValueFromString(const std::string* s) {
   return ::testing::AssertionSuccess();
 }
 
-void ReadVerifyCertChainTestFromFile(const std::string& file_name,
+void ReadVerifyCertChainTestFromFile(const std::string& file_path_ascii,
                                      ParsedCertificateList* chain,
                                      scoped_refptr<TrustAnchor>* trust_anchor,
                                      der::GeneralizedTime* time,
@@ -111,8 +111,7 @@ void ReadVerifyCertChainTestFromFile(const std::string& file_name,
   *trust_anchor = nullptr;
   expected_errors->clear();
 
-  std::string file_data = ReadTestFileToString(
-      std::string("net/data/verify_certificate_chain_unittest/") + file_name);
+  std::string file_data = ReadTestFileToString(file_path_ascii);
 
   std::vector<std::string> pem_headers;
 
@@ -180,11 +179,11 @@ void ReadVerifyCertChainTestFromFile(const std::string& file_name,
   ASSERT_TRUE(*trust_anchor);
 }
 
-std::string ReadTestFileToString(const std::string& file_name) {
+std::string ReadTestFileToString(const std::string& file_path_ascii) {
   // Compute the full path, relative to the src/ directory.
   base::FilePath src_root;
   PathService::Get(base::DIR_SOURCE_ROOT, &src_root);
-  base::FilePath filepath = src_root.AppendASCII(file_name);
+  base::FilePath filepath = src_root.AppendASCII(file_path_ascii);
 
   // Read the full contents of the file.
   std::string file_data;

net/cert/internal/test_helpers.h

@@ -76,10 +76,12 @@ template <size_t N>
   return ReadTestDataFromPemFile(file_path_ascii, mappings, N);
 }
 
-// Reads a test case from |file_name|. Test cases are comprised of a
-// certificate chain, trust anchor, a timestamp to validate at, and the
-// expected result of verification.
-void ReadVerifyCertChainTestFromFile(const std::string& file_name,
+// Reads a test case from |file_path_ascii| (which is relative to //src). Test
+// cases are comprised of a certificate chain, trust anchor, a timestamp to
+// validate at, and the expected result of verification.
+// Generally |file_path_ascii| will start with:
+//   net/data/verify_certificate_chain_unittest/
+void ReadVerifyCertChainTestFromFile(const std::string& file_path_ascii,
                                      ParsedCertificateList* chain,
                                      scoped_refptr<TrustAnchor>* trust_anchor,
                                      der::GeneralizedTime* time,
@@ -87,7 +89,7 @@ void ReadVerifyCertChainTestFromFile(const std::string& file_name,
                                      std::string* expected_errors);
 
 // Reads a data file relative to the src root directory.
-std::string ReadTestFileToString(const std::string& file_name);
+std::string ReadTestFileToString(const std::string& file_path_ascii);
 
 }  // namespace net
 

net/cert/internal/trust_store_collection_unittest.cc

@@ -39,9 +39,9 @@ class TrustStoreCollectionTest : public testing::Test {
     der::GeneralizedTime unused_time;
     std::string unused_errors;
 
-    ReadVerifyCertChainTestFromFile("key-rollover-oldchain.pem", &chain,
-                                    &oldroot_, &unused_time,
-                                    &unused_verify_result, &unused_errors);
+    ReadVerifyCertChainTestFromFile(
+        "net/data/verify_certificate_chain_unittest/key-rollover-oldchain.pem",
+        &chain, &oldroot_, &unused_time, &unused_verify_result, &unused_errors);
     ASSERT_EQ(2U, chain.size());
     target_ = chain[0];
     oldintermediate_ = chain[1];
@@ -50,9 +50,11 @@ class TrustStoreCollectionTest : public testing::Test {
     ASSERT_TRUE(oldroot_);
 
     scoped_refptr<TrustAnchor> unused_root;
-    ReadVerifyCertChainTestFromFile("key-rollover-longrolloverchain.pem",
-                                    &chain, &unused_root, &unused_time,
-                                    &unused_verify_result, &unused_errors);
+    ReadVerifyCertChainTestFromFile(
+        "net/data/verify_certificate_chain_unittest/"
+        "key-rollover-longrolloverchain.pem",
+        &chain, &unused_root, &unused_time, &unused_verify_result,
+        &unused_errors);
     ASSERT_EQ(4U, chain.size());
     newintermediate_ = chain[1];
     newroot_ = TrustAnchor::CreateFromCertificateNoConstraints(chain[2]);

net/cert/internal/trust_store_nss_unittest.cc

@@ -37,9 +37,9 @@ class TrustStoreNSSTest : public testing::Test {
     der::GeneralizedTime unused_time;
     std::string unused_errors;
 
-    ReadVerifyCertChainTestFromFile("key-rollover-oldchain.pem", &chain,
-                                    &oldroot_, &unused_time,
-                                    &unused_verify_result, &unused_errors);
+    ReadVerifyCertChainTestFromFile(
+        "net/data/verify_certificate_chain_unittest/key-rollover-oldchain.pem",
+        &chain, &oldroot_, &unused_time, &unused_verify_result, &unused_errors);
     ASSERT_EQ(2U, chain.size());
     target_ = chain[0];
     oldintermediate_ = chain[1];
@@ -48,9 +48,11 @@ class TrustStoreNSSTest : public testing::Test {
     ASSERT_TRUE(oldroot_);
 
     scoped_refptr<TrustAnchor> unused_root;
-    ReadVerifyCertChainTestFromFile("key-rollover-longrolloverchain.pem",
-                                    &chain, &unused_root, &unused_time,
-                                    &unused_verify_result, &unused_errors);
+    ReadVerifyCertChainTestFromFile(
+        "net/data/verify_certificate_chain_unittest/"
+        "key-rollover-longrolloverchain.pem",
+        &chain, &unused_root, &unused_time, &unused_verify_result,
+        &unused_errors);
     ASSERT_EQ(4U, chain.size());
     newintermediate_ = chain[1];
     newroot_ = TrustAnchor::CreateFromCertificateNoConstraints(chain[2]);

net/cert/internal/verify_certificate_chain_typed_unittest.h

@@ -5,9 +5,6 @@
 #ifndef NET_CERT_INTERNAL_VERIFY_CERTIFICATE_CHAIN_TYPED_UNITTEST_H_
 #define NET_CERT_INTERNAL_VERIFY_CERTIFICATE_CHAIN_TYPED_UNITTEST_H_
 
-#include "base/base_paths.h"
-#include "base/files/file_util.h"
-#include "base/path_service.h"
 #include "net/cert/internal/parsed_certificate.h"
 #include "net/cert/internal/test_helpers.h"
 #include "net/cert/internal/trust_store.h"
@@ -27,11 +24,14 @@ class VerifyCertificateChainTest : public ::testing::Test {
     bool expected_result;
     std::string expected_errors;
 
-    ReadVerifyCertChainTestFromFile(file_name, &chain, &trust_anchor, &time,
+    std::string path =
+        std::string("net/data/verify_certificate_chain_unittest/") + file_name;
+
+    ReadVerifyCertChainTestFromFile(path, &chain, &trust_anchor, &time,
                                     &expected_result, &expected_errors);
 
     TestDelegate::Verify(chain, trust_anchor, time, expected_result,
-                         expected_errors);
+                         expected_errors, path);
   }
 };
 

net/cert/internal/verify_certificate_chain_unittest.cc

@@ -18,7 +18,8 @@ class VerifyCertificateChainDelegate {
                      const scoped_refptr<TrustAnchor>& trust_anchor,
                      const der::GeneralizedTime& time,
                      bool expected_result,
-                     const std::string& expected_errors) {
+                     const std::string& expected_errors,
+                     const std::string& test_file_path) {
     ASSERT_TRUE(trust_anchor);
 
     SimpleSignaturePolicy signature_policy(1024);
@@ -27,7 +28,8 @@ class VerifyCertificateChainDelegate {
     bool result = VerifyCertificateChain(chain, trust_anchor.get(),
                                          &signature_policy, time, &errors);
     EXPECT_EQ(expected_result, result);
-    EXPECT_EQ(expected_errors, errors.ToDebugString());
+    EXPECT_EQ(expected_errors, errors.ToDebugString()) << "Test file: "
+                                                       << test_file_path;
   }
 };
 

net/data/verify_certificate_chain_unittest/rebase-errors.py

@@ -14,19 +14,47 @@ 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 works by scanning the stdout looking for gtest failures when
+comparing "errors.ToDebugString()". The C++ test side should have been
+instrumented to dump out the test file's path on mismatch.
 
-It will then go and update the corresponding .pem and .py file.
+This script will then update the corresponding file(s) -- a .pem file, and
+possibly an accompanying .py file.
 """
 
 import common
-import glob
 import os
 import sys
 import re
 
 
+# Regular expression to find the failed errors in test stdout.
+#  * Group 1 of the match is the actual error text (backslash-escaped)
+#  * Group 2 of the match is file path (relative to //src) where the expected
+#    errors were read from.
+failed_test_regex = re.compile(r"""
+Value of: errors.ToDebugString\(\)
+  Actual: "(.*)"
+(?:.|\n)+?
+Test file: (.*)
+""", re.MULTILINE)
+
+
+# Regular expression to find the ERRORS block (and any text above it) in a PEM
+# file. The assumption is that ERRORS is not the very first block in the file
+# (since it looks for an -----END to precede it).
+#  * Group 1 of the match is the ERRORS block content and any comments
+#    immediately above it.
+errors_block_regex = re.compile(r""".*
+-----END .*?-----
+
+(.*?
+-----BEGIN ERRORS-----
+.*?
+-----END ERRORS-----
+)""", re.MULTILINE | re.DOTALL)
+
+
 def read_file_to_string(path):
   """Reads a file entirely to a string"""
   with open(path, 'r') as f:
@@ -40,36 +68,17 @@ 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__))
+def get_py_path(pem_path):
+  """Returns the .py filepath used to generate the given .pem path, which may
+  or may not exist.
 
-  # 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]
-
-    # Strip the hyphens in file name to bring it closer to the camel case.
-    transformed = file_name_no_extension.replace('-', '')
-
-    # 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)
-
-  return None
+  Some test files (notably those in verify_certificate_chain_unittest/ have a
+  "generate-XXX.py" script that builds the "XXX.pem" file. Build the path to
+  the corresponding "generate-XXX.py" (which may or may not exist)."""
+  file_name = os.path.basename(pem_path)
+  file_name_no_extension = os.path.splitext(file_name)[0]
+  py_file_name = 'generate-' + file_name_no_extension + '.py'
+  return os.path.join(os.path.dirname(pem_path), py_file_name)
 
 
 def replace_string(original, start, end, replacement):
@@ -81,14 +90,14 @@ 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)
+  m = errors_block_regex.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)
@@ -99,7 +108,7 @@ def fixup_py_file(path, actual_errors):
   contents = read_file_to_string(path)
 
   # This assumes that the errors variable uses triple quotes.
-  prog = re.compile(r'^errors = """(.*)"""', re.MULTILINE | re.DOTALL)
+  prog = re.compile(r'^errors = """(.*?)"""', re.MULTILINE | re.DOTALL)
   result = prog.search(contents)
 
   # Replace the stuff in between the triple quotes with the actual errors.
@@ -110,21 +119,39 @@ 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_src_root():
+  """Returns the path to the enclosing //src directory. This assumes the
+  current script is inside the source tree."""
+  cur_dir = os.path.dirname(os.path.realpath(__file__))
 
-  # Determine the paths for the corresponding *.pem file and generate-*.py
-  pem_path, py_path = get_file_paths_for_test(test_name)
+  while True:
+    parent_dir, dirname = os.path.split(cur_dir)
+    # Check if it looks like the src/ root.
+    if dirname == "src" and os.path.isdir(os.path.join(cur_dir, "net")):
+      return cur_dir
+    if not parent_dir or parent_dir == cur_dir:
+      break
+    cur_dir = parent_dir
 
-  fixup_pem_file(pem_path, actual_errors)
-  fixup_py_file(py_path, actual_errors)
+  print "Couldn't find src dir"
+  sys.exit(1)
+
+
+def get_abs_path(rel_path):
+  """Converts |rel_path| (relative to src) to a full path"""
+  return os.path.join(get_src_root(), rel_path)
+
+
+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: "(.*)"$'
+  # If the test has a generator script update it too.
+  py_path = get_py_path(pem_path)
+  if os.path.isfile(py_path):
+    fixup_py_file(py_path, actual_errors)
 
 
 def main():
@@ -140,28 +167,11 @@ 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')
-
-    fixup_test(test_name, actual)
+  for m in failed_test_regex.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__":