Makes an expensive CTLogVerifier test into a value-parameterized test

net/cert/ct_log_verifier_unittest.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/cert/ct_log_verifier.h"
 
 #include <stdint.h>
 
 #include <memory>
 #include <string>
+#include <vector>
 
 #include "base/strings/string_number_conversions.h"
 #include "base/time/time.h"
 #include "crypto/secure_hash.h"
 #include "net/base/hash_value.h"
 #include "net/cert/ct_log_verifier_util.h"
 #include "net/cert/merkle_consistency_proof.h"
 #include "net/cert/signed_certificate_timestamp.h"
 #include "net/cert/signed_tree_head.h"
 #include "net/test/ct_test_util.h"
@@ skipping 457 matching lines @@
     // doesn't contain the root of snapshot1, so set have_root1 = false.
     subproof = ReferenceSnapshotConsistency(&inputs[split], snapshot2 - split,
                                             snapshot1 - split, false);
     proof.insert(proof.end(), subproof.begin(), subproof.end());
     // Record the hash of the left subtree (equal in both trees).
     proof.push_back(ReferenceMerkleTreeHash(&inputs[0], split));
   }
   return proof;
 }
 
-// Times out on Win7 test bot.  http://crbug.com/598406
-#if defined(OS_WIN)
-#define MAYBE_VerifiesValidConsistencyProofsFromReferenceGenerator \
-  DISABLED_VerifiesValidConsistencyProofsFromReferenceGenerator
-#else
-#define MAYBE_VerifiesValidConsistencyProofsFromReferenceGenerator \
-  VerifiesValidConsistencyProofsFromReferenceGenerator
-#endif
-TEST_F(CTLogVerifierTest,
-       MAYBE_VerifiesValidConsistencyProofsFromReferenceGenerator) {
+class CTLogVerifierTestUsingReferenceGenerator
+    : public CTLogVerifierTest,
+      public ::testing::WithParamInterface<uint64_t> {};
+
+const uint64_t kReferenceTreeSize = 256;
+
+TEST_P(CTLogVerifierTestUsingReferenceGenerator,
+       VerifiesValidConsistencyProof) {
   std::vector<std::string> data;
-  for (int i = 0; i < 256; ++i)
-    data.push_back(std::string(1, i));
+  for (uint64_t i = 0; i < kReferenceTreeSize; ++i)
+    data.push_back(std::string(1, static_cast<char>(i)));
 
-  std::vector<std::string> proof;
-  std::string root1, root2;
-  // More tests with reference proof generator.
-  for (size_t tree_size = 1; tree_size <= data.size() / 2; ++tree_size) {
-    root2 = ReferenceMerkleTreeHash(data.data(), tree_size);
-    // Repeat for each snapshot in range.
-    for (size_t snapshot = 1; snapshot <= tree_size; ++snapshot) {
-      proof =
-          ReferenceSnapshotConsistency(data.data(), tree_size, snapshot, true);
-      root1 = ReferenceMerkleTreeHash(data.data(), snapshot);
-      VerifierConsistencyCheck(snapshot, tree_size, root1, root2, proof);
-    }
+  uint64_t tree_size = GetParam();
+  for (uint64_t snapshot = 1; snapshot <= tree_size; ++snapshot) {
+    SCOPED_TRACE(snapshot);
+    std::string root1 = ReferenceMerkleTreeHash(data.data(), snapshot);
+    std::string root2 = ReferenceMerkleTreeHash(data.data(), tree_size);

[Ryan Sleevi, 2016/07/28 18:03:31]

DESIGN: I believe you can move this to the outer-loop like you used to (see 506
vs 508). That is, you can move this up to be between lines 501/502 and gain
better performance. Might help to make const std::string root2

(or rename the variables based on the order of decl; that is, root1 becomes the
tree_size root, root2 because the snapshot root)

[Rob Percival, 2016/07/28 18:16:43]

Done.

+    std::vector<std::string> proof =
+        ReferenceSnapshotConsistency(data.data(), tree_size, snapshot, true);
+    VerifierConsistencyCheck(snapshot, tree_size, root1, root2, proof);
   }
 }
 
+// Test verification of consistency proofs between all tree sizes from 1 to 128.
+INSTANTIATE_TEST_CASE_P(RangeOfTreeSizesAndSnapshots,
+                        CTLogVerifierTestUsingReferenceGenerator,
+                        testing::Range(1ul, (kReferenceTreeSize / 2) + 1));

[Ryan Sleevi, 2016/07/28 18:03:31]

nit: 1u should be sufficient (C++11 will promote it up to uint64_t if it's
unsigned int)

[Rob Percival, 2016/07/28 18:16:43]

Results in compilation error:
candidate template ignored: deduced conflicting types for parameter 'T'
('unsigned int' vs. 'unsigned long')
internal::ParamGenerator<T> Range(T start, T end) {

+
 }  // namespace
 
 }  // namespace net