NQE Cleanup in preparation for file splitting

net/base/network_quality_estimator_unittest.cc

 // Copyright 2015 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/base/network_quality_estimator.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <limits>
@@ skipping 19 matching lines @@
 #include "net/base/socket_performance_watcher_factory.h"
 #include "net/http/http_status_code.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "net/test/embedded_test_server/http_request.h"
 #include "net/test/embedded_test_server/http_response.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
+namespace net {
+
 namespace {
 
 // Helps in setting the current network type and id.
-class TestNetworkQualityEstimator : public net::NetworkQualityEstimator {
+class TestNetworkQualityEstimator : public NetworkQualityEstimator {
  public:
   TestNetworkQualityEstimator(
       const std::map<std::string, std::string>& variation_params,
-      scoped_ptr<net::ExternalEstimateProvider> external_estimate_provider)
+      scoped_ptr<ExternalEstimateProvider> external_estimate_provider)
       : NetworkQualityEstimator(std::move(external_estimate_provider),
                                 variation_params,
                                 true,
                                 true) {
     // Set up embedded test server.
     embedded_test_server_.ServeFilesFromDirectory(
         base::FilePath(FILE_PATH_LITERAL("net/data/url_request_unittest")));
     EXPECT_TRUE(embedded_test_server_.Start());
     embedded_test_server_.RegisterRequestHandler(base::Bind(
         &TestNetworkQualityEstimator::HandleRequest, base::Unretained(this)));
   }
 
   explicit TestNetworkQualityEstimator(
       const std::map<std::string, std::string>& variation_params)
-      : TestNetworkQualityEstimator(
-            variation_params,
-            scoped_ptr<net::ExternalEstimateProvider>()) {}
+      : TestNetworkQualityEstimator(variation_params,
+                                    scoped_ptr<ExternalEstimateProvider>()) {}
 
   ~TestNetworkQualityEstimator() override {}
 
   // Overrides the current network type and id.
   // Notifies network quality estimator of change in connection.
-  void SimulateNetworkChangeTo(net::NetworkChangeNotifier::ConnectionType type,
+  void SimulateNetworkChangeTo(NetworkChangeNotifier::ConnectionType type,
                                std::string network_id) {
     current_network_type_ = type;
     current_network_id_ = network_id;
     OnConnectionTypeChanged(type);
   }
 
   // Called by embedded server when a HTTP request is received.
-  scoped_ptr<net::test_server::HttpResponse> HandleRequest(
-      const net::test_server::HttpRequest& request) {
-    scoped_ptr<net::test_server::BasicHttpResponse> http_response(
-        new net::test_server::BasicHttpResponse());
-    http_response->set_code(net::HTTP_OK);
+  scoped_ptr<test_server::HttpResponse> HandleRequest(
+      const test_server::HttpRequest& request) {
+    scoped_ptr<test_server::BasicHttpResponse> http_response(
+        new test_server::BasicHttpResponse());
+    http_response->set_code(HTTP_OK);
     http_response->set_content("hello");
     http_response->set_content_type("text/plain");
     return std::move(http_response);
   }
 
   // Returns a GURL hosted at embedded test server.
   const GURL GetEchoURL() const {
     return embedded_test_server_.GetURL("/echo.html");
   }
 
   using NetworkQualityEstimator::ReadCachedNetworkQualityEstimate;
   using NetworkQualityEstimator::OnConnectionTypeChanged;
 
  private:
   // NetworkQualityEstimator implementation that returns the overridden network
   // id (instead of invoking platform APIs).
   NetworkQualityEstimator::NetworkID GetCurrentNetworkID() const override {
     return NetworkQualityEstimator::NetworkID(current_network_type_,
                                               current_network_id_);
   }
 
-  net::NetworkChangeNotifier::ConnectionType current_network_type_;
+  NetworkChangeNotifier::ConnectionType current_network_type_;
   std::string current_network_id_;
 
   // Embedded server used for testing.
-  net::EmbeddedTestServer embedded_test_server_;
+  EmbeddedTestServer embedded_test_server_;
 
   DISALLOW_COPY_AND_ASSIGN(TestNetworkQualityEstimator);
 };
 
-class TestRTTObserver : public net::NetworkQualityEstimator::RTTObserver {
+class TestRTTObserver : public NetworkQualityEstimator::RTTObserver {
  public:
   struct Observation {
     Observation(int32_t ms,
                 const base::TimeTicks& ts,
-                net::NetworkQualityEstimator::ObservationSource src)
+                NetworkQualityEstimator::ObservationSource src)
         : rtt_ms(ms), timestamp(ts), source(src) {}
     int32_t rtt_ms;
     base::TimeTicks timestamp;
-    net::NetworkQualityEstimator::ObservationSource source;
+    NetworkQualityEstimator::ObservationSource source;
   };
 
   std::vector<Observation>& observations() { return observations_; }
 
   // RttObserver implementation:
   void OnRTTObservation(
       int32_t rtt_ms,
       const base::TimeTicks& timestamp,
-      net::NetworkQualityEstimator::ObservationSource source) override {
+      NetworkQualityEstimator::ObservationSource source) override {
     observations_.push_back(Observation(rtt_ms, timestamp, source));
   }
 
  private:
   std::vector<Observation> observations_;
 };
 
 class TestThroughputObserver
-    : public net::NetworkQualityEstimator::ThroughputObserver {
+    : public NetworkQualityEstimator::ThroughputObserver {
  public:
   struct Observation {
     Observation(int32_t kbps,
                 const base::TimeTicks& ts,
-                net::NetworkQualityEstimator::ObservationSource src)
+                NetworkQualityEstimator::ObservationSource src)
         : throughput_kbps(kbps), timestamp(ts), source(src) {}
     int32_t throughput_kbps;
     base::TimeTicks timestamp;
-    net::NetworkQualityEstimator::ObservationSource source;
+    NetworkQualityEstimator::ObservationSource source;
   };
 
   std::vector<Observation>& observations() { return observations_; }
 
   // ThroughputObserver implementation:
   void OnThroughputObservation(
       int32_t throughput_kbps,
       const base::TimeTicks& timestamp,
-      net::NetworkQualityEstimator::ObservationSource source) override {
+      NetworkQualityEstimator::ObservationSource source) override {
     observations_.push_back(Observation(throughput_kbps, timestamp, source));
   }
 
  private:
   std::vector<Observation> observations_;
 };
 
 }  // namespace
 
-namespace net {
-
 TEST(NetworkQualityEstimatorTest, TestKbpsRTTUpdates) {
   base::HistogramTester histogram_tester;
   // Enable requests to local host to be used for network quality estimation.
   std::map<std::string, std::string> variation_params;
   TestNetworkQualityEstimator estimator(variation_params);
 
   EXPECT_EQ(NetworkQualityEstimator::InvalidRTT(),
             estimator.GetURLRequestRTTEstimateInternal(base::TimeTicks(), 100));
   EXPECT_EQ(NetworkQualityEstimator::kInvalidThroughput,
             estimator.GetDownlinkThroughputKbpsEstimateInternal(
@@ skipping 311 matching lines @@
   EXPECT_EQ(1U, estimator.downstream_throughput_kbps_observations_.Size());
   EXPECT_EQ(1U, estimator.rtt_observations_.Size());
 
   base::TimeDelta rtt;
   EXPECT_TRUE(estimator.GetURLRequestRTTEstimate(&rtt));
   int32_t kbps;
   EXPECT_TRUE(estimator.GetDownlinkThroughputKbpsEstimate(&kbps));
 
   EXPECT_EQ(100, kbps);
   EXPECT_EQ(base::TimeDelta::FromMilliseconds(1000), rtt);
-  auto throughput_iterator =
-      estimator.downstream_throughput_kbps_observations_.observations_.begin();
-  EXPECT_EQ(100, (*throughput_iterator).value);

[bengr, 2016/04/21 22:00:54]

Here and elsewhere I see a bunch of EXPECT statements that have been deleted.
Are these going to be moved to some other file in a later CL? Please explain.

[tbansal1, 2016/04/21 22:06:43]

No they are not going to be moved. They were removed because these statements
are looking at internal data structures which is not required. e.g., 
(1) This test verifies that there is exactly one throughput observation by
looking at the size of the throughput observation buffer
(2) This test then verifies that the median throughput estimate is 1000
(3) It looks at the first element in the observation buffer to see if it
actually 1000 or not.

If (1) and (2) are true, then (3) must be true. So, I removed checks
corresponding to (3). This would help in splitting the files later because now
the test does not care about internal details of ObservationBuffer. 

-  auto rtt_iterator = estimator.rtt_observations_.observations_.begin();
-  EXPECT_EQ(base::TimeDelta::FromMilliseconds(1000), (*rtt_iterator).value);
 
   // Simulate network change to Wi-Fi.
   estimator.SimulateNetworkChangeTo(
       NetworkChangeNotifier::ConnectionType::CONNECTION_WIFI, "test-1");
   EXPECT_EQ(1U, estimator.downstream_throughput_kbps_observations_.Size());
   EXPECT_EQ(1U, estimator.rtt_observations_.Size());
 
   EXPECT_TRUE(estimator.GetURLRequestRTTEstimate(&rtt));
   EXPECT_TRUE(estimator.GetDownlinkThroughputKbpsEstimate(&kbps));
   EXPECT_EQ(200, kbps);
   EXPECT_EQ(base::TimeDelta::FromMilliseconds(2000), rtt);
 
-  throughput_iterator =
-      estimator.downstream_throughput_kbps_observations_.observations_.begin();
-  EXPECT_EQ(200, (*throughput_iterator).value);
-  rtt_iterator = estimator.rtt_observations_.observations_.begin();
-  EXPECT_EQ(base::TimeDelta::FromMilliseconds(2000), (*rtt_iterator).value);
-
   // Peak network quality should not be affected by the network quality
   // estimator field trial.
   EXPECT_EQ(NetworkQualityEstimator::InvalidRTT(),
             estimator.peak_network_quality_.rtt());
   EXPECT_EQ(NetworkQualityEstimator::kInvalidThroughput,
             estimator.peak_network_quality_.downstream_throughput_kbps());
 
   // Simulate network change to 2G. Only the Kbps default estimate should be
   // available.
   estimator.SimulateNetworkChangeTo(
       NetworkChangeNotifier::ConnectionType::CONNECTION_2G, "test-2");
   EXPECT_EQ(1U, estimator.downstream_throughput_kbps_observations_.Size());
   EXPECT_EQ(0U, estimator.rtt_observations_.Size());
 
   EXPECT_FALSE(estimator.GetURLRequestRTTEstimate(&rtt));
   EXPECT_TRUE(estimator.GetDownlinkThroughputKbpsEstimate(&kbps));
-
-  throughput_iterator =
-      estimator.downstream_throughput_kbps_observations_.observations_.begin();
-  EXPECT_EQ(300, (*throughput_iterator).value);
+  EXPECT_EQ(300, kbps);
 
   // Simulate network change to 3G. Default estimates should be unavailable.
   estimator.SimulateNetworkChangeTo(
       NetworkChangeNotifier::ConnectionType::CONNECTION_3G, "test-3");
 
   EXPECT_FALSE(estimator.GetURLRequestRTTEstimate(&rtt));
   EXPECT_FALSE(estimator.GetDownlinkThroughputKbpsEstimate(&kbps));
-  EXPECT_EQ(0U, estimator.downstream_throughput_kbps_observations_.Size());
-  EXPECT_EQ(0U, estimator.rtt_observations_.Size());
 }
 
+// Tests if |weight_multiplier_per_second_| is set to correct value for various
+// values of half life parameter.
 TEST(NetworkQualityEstimatorTest, HalfLifeParam) {
-  // Verifies if |weight_multiplier_per_second_| is set to correct value for
-  // various values of half life parameter.
   std::map<std::string, std::string> variation_params;
-  {
-    // Half life parameter is not set. Default value of
-    // |weight_multiplier_per_second_| should be used.
+
+  const struct {
+    std::string description;
+    std::string variation_params_value;
+    double expected_weight_multiplier;
+  } tests[] = {
+      {"Half life parameter is not set, default value should be used",
+       std::string(), 0.988},
+      {"Half life parameter is set to negative, default value should be used",
+       "-100", 0.988},
+      {"Half life parameter is set to zero, default value should be used", "0",
+       0.988},
+      {"Half life parameter is set correctly", "10", 0.933},
+  };
+
+  for (const auto& test : tests) {
+    variation_params["HalfLifeSeconds"] = test.variation_params_value;
     TestNetworkQualityEstimator estimator(variation_params);
-    EXPECT_NEAR(0.988, estimator.downstream_throughput_kbps_observations_
-                           .weight_multiplier_per_second_,
-                0.001);
-  }
-
-  variation_params["HalfLifeSeconds"] = "-100";
-  {
-    // Half life parameter is set to a negative value.  Default value of
-    // |weight_multiplier_per_second_| should be used.
-    TestNetworkQualityEstimator estimator(variation_params);
-    EXPECT_NEAR(0.988, estimator.downstream_throughput_kbps_observations_
-                           .weight_multiplier_per_second_,
-                0.001);
-  }
-
-  variation_params["HalfLifeSeconds"] = "0";
-  {
-    // Half life parameter is set to zero.  Default value of
-    // |weight_multiplier_per_second_| should be used.
-    TestNetworkQualityEstimator estimator(variation_params);
-    EXPECT_NEAR(0.988, estimator.downstream_throughput_kbps_observations_
-                           .weight_multiplier_per_second_,
-                0.001);
-  }
-
-  variation_params["HalfLifeSeconds"] = "10";
-  {
-    // Half life parameter is set to a valid value.
-    TestNetworkQualityEstimator estimator(variation_params);
-    EXPECT_NEAR(0.933, estimator.downstream_throughput_kbps_observations_
-                           .weight_multiplier_per_second_,
-                0.001);
+    EXPECT_NEAR(test.expected_weight_multiplier,
+                estimator.weight_multiplier_per_second_, 0.001)
+        << test.description;
   }
 }
 
 // Test if the network estimates are cached when network change notification
 // is invoked.
 TEST(NetworkQualityEstimatorTest, TestCaching) {
   std::map<std::string, std::string> variation_params;
   TestNetworkQualityEstimator estimator(variation_params);
   size_t expected_cache_size = 0;
   EXPECT_EQ(expected_cache_size, estimator.cached_network_qualities_.size());
@@ skipping 81 matching lines @@
       NetworkChangeNotifier::ConnectionType::CONNECTION_2G, "test-3");
   EXPECT_FALSE(estimator.ReadCachedNetworkQualityEstimate());
 }
 
 // Tests if the cache size remains bounded. Also, ensure that the cache is
 // LRU.
 TEST(NetworkQualityEstimatorTest, TestLRUCacheMaximumSize) {
   std::map<std::string, std::string> variation_params;
   TestNetworkQualityEstimator estimator(variation_params);
   estimator.SimulateNetworkChangeTo(
-      net::NetworkChangeNotifier::ConnectionType::CONNECTION_WIFI,
-      std::string());
+      NetworkChangeNotifier::ConnectionType::CONNECTION_WIFI, std::string());
   EXPECT_EQ(0U, estimator.cached_network_qualities_.size());
 
   // Add 100 more networks than the maximum size of the cache.
   size_t network_count =
       NetworkQualityEstimator::kMaximumNetworkQualityCacheSize + 100;
 
   base::TimeTicks update_time_of_network_100;
   for (size_t i = 0; i < network_count; ++i) {
     estimator.downstream_throughput_kbps_observations_.AddObservation(
         NetworkQualityEstimator::ThroughputObservation(
             2, base::TimeTicks::Now(), NetworkQualityEstimator::URL_REQUEST));
     estimator.rtt_observations_.AddObservation(
         NetworkQualityEstimator::RttObservation(
             base::TimeDelta::FromMilliseconds(500), base::TimeTicks::Now(),
             NetworkQualityEstimator::URL_REQUEST));
 
     if (i == 100)
       update_time_of_network_100 = base::TimeTicks::Now();
 
     estimator.SimulateNetworkChangeTo(
-        net::NetworkChangeNotifier::ConnectionType::CONNECTION_WIFI,
+        NetworkChangeNotifier::ConnectionType::CONNECTION_WIFI,
         base::SizeTToString(i));
     if (i < NetworkQualityEstimator::kMaximumNetworkQualityCacheSize)
       EXPECT_EQ(i, estimator.cached_network_qualities_.size());
     EXPECT_LE(estimator.cached_network_qualities_.size(),
               static_cast<size_t>(
                   NetworkQualityEstimator::kMaximumNetworkQualityCacheSize));
   }
   // One more call so that the last network is also written to cache.
   estimator.downstream_throughput_kbps_observations_.AddObservation(
       NetworkQualityEstimator::ThroughputObservation(
           2, base::TimeTicks::Now(), NetworkQualityEstimator::URL_REQUEST));
   estimator.rtt_observations_.AddObservation(
       NetworkQualityEstimator::RttObservation(
           base::TimeDelta::FromMilliseconds(500), base::TimeTicks::Now(),
           NetworkQualityEstimator::URL_REQUEST));
   estimator.SimulateNetworkChangeTo(
-      net::NetworkChangeNotifier::ConnectionType::CONNECTION_WIFI,
+      NetworkChangeNotifier::ConnectionType::CONNECTION_WIFI,
       base::SizeTToString(network_count - 1));
   EXPECT_EQ(static_cast<size_t>(
                 NetworkQualityEstimator::kMaximumNetworkQualityCacheSize),
             estimator.cached_network_qualities_.size());
 
   // Test that the cache is LRU by examining its contents. Networks in cache
   // must all be newer than the 100th network.
   for (NetworkQualityEstimator::CachedNetworkQualities::iterator it =
            estimator.cached_network_qualities_.begin();
        it != estimator.cached_network_qualities_.end(); ++it) {
@@ skipping 426 matching lines @@
     }
     // At least one notification should be received per socket performance
     // watcher.
     EXPECT_LE(1U, after_count_tcp_rtt_observations -
                       before_count_tcp_rtt_observations)
         << i;
   }
 }
 
 }  // namespace net