Makes an expensive CTLogVerifier test into a value-parameterized test

net/cert/ct_log_verifier_unittest.cc

Index: net/cert/ct_log_verifier_unittest.cc
diff --git a/net/cert/ct_log_verifier_unittest.cc b/net/cert/ct_log_verifier_unittest.cc
index 754d3e39073c50f89c3c83b1d3d623a7ef782659..07f81426f6fc7955726ab95cb2f17401a6d99429 100644
--- a/net/cert/ct_log_verifier_unittest.cc
+++ b/net/cert/ct_log_verifier_unittest.cc
@@ -8,6 +8,7 @@
 
 #include <memory>
 #include <string>
+#include <vector>
 
 #include "base/strings/string_number_conversions.h"
 #include "base/time/time.h"
@@ -485,35 +486,34 @@ std::vector<std::string> ReferenceSnapshotConsistency(std::string* inputs,
   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) {
-  std::vector<std::string> data;
-  for (int i = 0; i < 256; ++i)
-    data.push_back(std::string(1, i));
+class CTLogVerifierTestUsingReferenceGenerator
+    : public CTLogVerifierTest,
+      public ::testing::WithParamInterface<uint64_t> {};
 
-  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);
-    }
+const uint64_t kReferenceTreeSize = 256;
+
+TEST_P(CTLogVerifierTestUsingReferenceGenerator,
+       VerifiesValidConsistencyProof) {
+  std::vector<std::string> data;
+  for (uint64_t i = 0; i < kReferenceTreeSize; ++i)
+    data.push_back(std::string(1, static_cast<char>(i)));
+
+  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