Added HttpUtils::HasValidators and HttpResponse::HasValidators

net/http/http_util_unittest.cc

 // Copyright (c) 2012 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 <algorithm>
 
 #include "base/basictypes.h"
 #include "base/strings/string_util.h"
 #include "net/http/http_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ skipping 1160 matching lines @@
   std::string data = "name='value";
   HttpUtil::NameValuePairsIterator parser(data.begin(), data.end(), ';');
   EXPECT_TRUE(parser.valid());
 
   ASSERT_NO_FATAL_FAILURE(
       CheckNextNameValuePair(&parser, true, true, "name", "value"));
   ASSERT_NO_FATAL_FAILURE(CheckNextNameValuePair(
       &parser, false, true, std::string(), std::string()));
 }
 
+TEST(HttpUtilTest, HasValidatorsHttp09) {
+  std::string etags[] = {"", "\"strong\"", "W/\"weak\""};
+  std::string last_modified_values[] = {"", "Tue, 15 Nov 1994 12:45:26 GMT",
+                                        "invalid"};
+
+  for (const std::string& etag : etags) {
+    for (const std::string& last_modified : last_modified_values) {
+      EXPECT_FALSE(
+          HttpUtil::HasValidators(HttpVersion(0, 9), etag, last_modified));

[twifkak, 2016/02/22 23:44:44]

add `<< "with etag = " << etag << " and last_modified = "<< last_modified`, so
we know what loop iteration failed. (Ditto below.)

[jamartin, 2016/02/23 14:03:35]

I've applied your second suggestion, which obsoletes this.

+    }
+  }

[twifkak, 2016/02/22 23:44:44]

There are 3*3=9 etag/last-modified pairs.

Suggestion:
1. Make char[] constants for the five values you specify on lines 1182-3 (and
which you duplicate below!).
2. Just hard-code the 9 pairs without a loop, e.g.:
  EXPECT_FALSE(Http::HasValidators(HttpVersion(0, 9), kEmpty,
kLastModifiedInvalid));
  ...

This will:
1. Be easier to read, thanks to not having to use variables or boolean logic in
your expectations.
2. Not be much brittler, thanks to the constants.
3. Only be two lines longer than the current loop-based tests.

[jamartin, 2016/02/23 14:03:35]

Done.

There is one advantage of the loop approach, though: the condition for the
TRUE/FALSE evaluation is made explicit. But I do agree with you that it is now
as simple as it gets.

[twifkak, 2016/02/24 21:07:38]

LGTM. Some rambling thoughts: Agreed, that the logic is harder to read now. On
the plus side, you can read it in the prod code, and the test is much stronger
when prod and test are implemented differently. I see your point, though -- some
future coder might not know what to do with this wall of assertions. I lean
toward simpler still, but I'm not at all steadfast on that.

+}
+
+TEST(HttpUtilTest, HasValidatorsHttp10) {
+  std::string etags[] = {"", "\"strong\"", "W/\"weak\""};
+  std::string last_modified_values[] = {"", "Tue, 15 Nov 1994 12:45:26 GMT",
+                                        "invalid"};
+
+  for (const std::string& etag : etags) {
+    for (const std::string& last_modified : last_modified_values) {
+      EXPECT_EQ(
+          last_modified != "" && last_modified != "invalid",
+          HttpUtil::HasValidators(HttpVersion(1, 0), etag, last_modified));
+    }
+  }
+}
+
+TEST(HttpUtilTest, HasValidatorsHttp11) {
+  std::string etags[] = {"", "\"strong\"", "W/\"weak\""};
+  std::string last_modified_values[] = {"", "Tue, 15 Nov 1994 12:45:26 GMT",
+                                        "invalid"};
+
+  for (const std::string& etag : etags) {
+    for (const std::string& last_modified : last_modified_values) {
+      EXPECT_EQ(
+          etag != "" || (last_modified != "" && last_modified != "invalid"),
+          HttpUtil::HasValidators(HttpVersion(1, 1), etag, last_modified));
+    }
+  }
+}
+
 }  // namespace net