Reformat and fix histogram regerssion test, which used the wrong bucket

since the original code was buggy AFAIKS.

diff --git a/.not-formatted b/.not-formatted
index 9778db3d7cb6515b7c54ce56e34ef43f36b668a1..f03bd9037d562168583e322438b07435671dd642 100644 (file)
--- a/.not-formatted
+++ b/.not-formatted
@@ -175,7 +175,6 @@
 ./pdns/gettime.cc
 ./pdns/gettime.hh
 ./pdns/histog.hh
-./pdns/histogram.hh
 ./pdns/inflighter.cc
 ./pdns/ipcipher.cc
 ./pdns/iputils.cc

diff --git a/pdns/histogram.hh b/pdns/histogram.hh
index 525ccbf45670d0f7be48c5609a70f6c4525428ea..d41d6a9adec60c35b78c7e06ad9f869c21579b82 100644 (file)
--- a/pdns/histogram.hh
+++ b/pdns/histogram.hh
@@ -27,7 +27,8 @@
 #include <string>
 #include <vector>
 
-namespace pdns {
+namespace pdns
+{
 
 // By convention, we are using microsecond units
 struct Bucket
@@ -41,15 +42,17 @@ struct AtomicBucket
 {
   // We need the constructors in this case, since atomics have a disabled copy constructor.
   AtomicBucket() {}
-  AtomicBucket(std::string name, uint64_t boundary, uint64_t val) : d_name(std::move(name)), d_boundary(boundary), d_count(val) {}
-  AtomicBucket(const AtomicBucket& rhs) : d_name(rhs.d_name), d_boundary(rhs.d_boundary), d_count(rhs.d_count.load()) {}
+  AtomicBucket(std::string name, uint64_t boundary, uint64_t val) :
+    d_name(std::move(name)), d_boundary(boundary), d_count(val) {}
+  AtomicBucket(const AtomicBucket& rhs) :
+    d_name(rhs.d_name), d_boundary(rhs.d_boundary), d_count(rhs.d_count.load()) {}
 
   std::string d_name;
   uint64_t d_boundary{0};
   std::atomic<uint64_t> d_count{0};
 };
 
-template<class B>
+template <class B>
 class BaseHistogram
 {
 public:
@@ -66,7 +69,7 @@ public:
     }
     d_buckets.reserve(boundaries.size() + 1);
     uint64_t prev = 0;
-    for (auto b: boundaries) {
+    for (auto b : boundaries) {
       if (prev == b) {
         throw std::invalid_argument("boundary array's elements should be distinct");
       }
@@ -120,7 +123,7 @@ public:
   inline void operator()(uint64_t d)
   {
     auto index = std::upper_bound(d_buckets.begin(), d_buckets.end(), d, lessOrEqual);
-    // out index is always valid
+    // our index is always valid
     ++index->d_count;
   }
 
@@ -128,10 +131,10 @@ private:
   std::vector<B> d_buckets;
 };
 
-template<class T>
+template <class T>
 using Histogram = BaseHistogram<Bucket>;
 
-template<class T>
+template <class T>
 using AtomicHistogram = BaseHistogram<AtomicBucket>;
 
 } // namespace pdns

diff --git a/regression-tests.api/test_Servers.py b/regression-tests.api/test_Servers.py
index 6789796906814490837dfdbce3fc56470c0bf6a7..4547a59de906f1c3c8337757cab557f0d8389239 100644 (file)
--- a/regression-tests.api/test_Servers.py
+++ b/regression-tests.api/test_Servers.py
@@ -56,7 +56,7 @@ class Servers(ApiTestCase):
                 elif elem['type'] == 'MapStatisticItem' and elem['name'] == 'response-by-rcode':
                     rcode_stats = elem['value']
             self.assertIn('A', [e['name'] for e in qtype_stats])
-            self.assertIn('60', [e['name'] for e in respsize_stats])
+            self.assertIn('80', [e['name'] for e in respsize_stats])
             self.assertIn('example.com/A', [e['name'] for e in queries_stats])
             self.assertIn('No Error', [e['name'] for e in rcode_stats])
         else:
@@ -70,8 +70,8 @@ class Servers(ApiTestCase):
                     rcode_stats = elem['value']
             self.assertIn('A', [e['name'] for e in qtype_stats])
             self.assertIn('60', [e['name'] for e in respsize_stats])
+            self.assertIn('80', [e['name'] for e in respsize_stats])
             self.assertIn('No Error', [e['name'] for e in rcode_stats])
-           
 
     def test_read_one_statistic(self):
         r = self.session.get(self.url("/api/v1/servers/localhost/statistics?statistic=uptime"))
@@ -96,7 +96,7 @@ class Servers(ApiTestCase):
                 if line.split(" ")[0] == "pdns_recursor_uptime":
                     found = True
             self.assertTrue(found,"pdns_recursor_uptime is missing")
-            
+
     @unittest.skipIf(is_auth(), "Not applicable")
     def test_read_statistics_using_password(self):
         r = requests.get(self.url("/api/v1/servers/localhost/statistics"), auth=('admin', self.server_web_password))