Domain Reliability: More security review.

components/domain_reliability/monitor_unittest.cc

 // Copyright 2014 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 "components/domain_reliability/monitor.h"
 
 #include <map>
 #include <string>
 #include <vector>
 
@@ skipping 18 matching lines @@
 class DomainReliabilityMonitorTest : public testing::Test {
  protected:
   typedef DomainReliabilityMonitor::RequestInfo RequestInfo;
 
   DomainReliabilityMonitorTest()
       : bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP),
         url_request_context_getter_(new net::TestURLRequestContextGetter(
             base::MessageLoopProxy::current())),
         time_(new MockTime()),
         monitor_(url_request_context_getter_->GetURLRequestContext(),
+                 "test-reporter",
                  scoped_ptr<MockableTime>(time_)),
         context_(monitor_.AddContextForTesting(CreateConfig())) {}
 
   static scoped_ptr<const DomainReliabilityConfig> CreateConfig() {
     DomainReliabilityConfig* config = new DomainReliabilityConfig();
 
     DomainReliabilityConfig::Resource* resource;
 
     resource = new DomainReliabilityConfig::Resource();
     resource->name = "always_report";
@@ skipping 30 matching lines @@
   RequestInfo MakeRequestInfo() {
     RequestInfo request;
     request.status = net::URLRequestStatus();
     request.response_code = 200;
     request.was_cached = false;
     request.load_flags = 0;
     request.is_upload = false;
     return request;
   }
 
-  bool CheckNoBeacons(int index) {
+  bool CheckNoBeacons(size_t index) {
     BeaconVector beacons;
-    int successful, failed;
+    unsigned successful, failed;
     context_->GetQueuedDataForTesting(index, &beacons, &successful, &failed);
     return beacons.empty() && successful == 0 && failed == 0;
   }
 
   void OnRequestLegComplete(const RequestInfo& info) {
     monitor_.OnRequestLegComplete(info);
   }
 
   content::TestBrowserThreadBundle bundle_;
   scoped_refptr<net::URLRequestContextGetter> url_request_context_getter_;
@@ skipping 47 matching lines @@
   EXPECT_TRUE(CheckNoBeacons(0));
   EXPECT_TRUE(CheckNoBeacons(1));
 }
 
 TEST_F(DomainReliabilityMonitorTest, AddBakedInConfigs) {
   // AddBakedInConfigs DCHECKs that the baked-in configs parse correctly, so
   // this unittest will fail if someone tries to add an invalid config to the
   // source tree.
   monitor_.AddBakedInConfigs();
 
+  // Count the number of baked-in configs.
   size_t num_baked_in_configs = 0;
   for (const char* const* p = kBakedInJsonConfigs; *p; ++p)
     ++num_baked_in_configs;
 
   // The monitor should have contexts for all of the baked-in configs, plus the
   // test one added in the test constructor.
   EXPECT_EQ(num_baked_in_configs + 1, monitor_.contexts_size_for_testing());
 }
 

[Ryan Sleevi, 2014/05/05 19:16:03]

There's no tests for OnBeforeRedirect here, which is part of monitor's API.

[Deprecated (see juliatuttle), 2014/05/06 18:52:03]

There aren't any for OnComplete, either! I was trying to dodge mocking out an
entire URLRequest, since it is a royal pain.

Can you think of an easier way to do that than doing the URL request filter
thing to mock out the redirects and requests?

[Ryan Sleevi, 2014/05/06 19:06:25]

Not off the top of my head.

However, what you're testing now isn't really a "good test" - that is, you're
not really testing the methods/behaviour of your class in isolation, but instead
testing it in concert with some external thing.

I definitely want to see defined behaviour for how redirects are handled and
what's the expected result. I honestly don't know what to expect, and that's the
best time to add tests (and documentation, but tests are the best kind of
documentation, since they can't lie :D)

I don't think it's essential for this CL.

 }  // namespace domain_reliability