Add unit tests for buffer-stats.

Now that formatting the buffer-stats string is separate from writing
it to disk, we can also decouple the logic to extract stats from
circuits and finally write some unit tests for the history code.

src/or/rephist.c

@@ -2364,23 +2364,41 @@ typedef struct circ_buffer_stats_t {
 /** List of circ_buffer_stats_t. */
 static smartlist_t *circuits_for_buffer_stats = NULL;
 
+/** Remember cell statistics <b>mean_num_cells_in_queue</b>,
+ * <b>mean_time_cells_in_queue</b>, and <b>processed_cells</b> of a
+ * circuit. */
+void
+rep_hist_add_buffer_stats(double mean_num_cells_in_queue,
+    double mean_time_cells_in_queue, uint32_t processed_cells)
+{
+  circ_buffer_stats_t *stat;
+  if (!start_of_buffer_stats_interval)
+    return; /* Not initialized. */
+  stat = tor_malloc_zero(sizeof(circ_buffer_stats_t));
+  stat->mean_num_cells_in_queue = mean_num_cells_in_queue;
+  stat->mean_time_cells_in_queue = mean_time_cells_in_queue;
+  stat->processed_cells = processed_cells;
+  if (!circuits_for_buffer_stats)
+    circuits_for_buffer_stats = smartlist_create();
+  smartlist_add(circuits_for_buffer_stats, stat);
+}
+
 /** Remember cell statistics for circuit <b>circ</b> at time
  * <b>end_of_interval</b> and reset cell counters in case the circuit
  * remains open in the next measurement interval. */
 void
 rep_hist_buffer_stats_add_circ(circuit_t *circ, time_t end_of_interval)
 {
-  circ_buffer_stats_t *stat;
   time_t start_of_interval;
   int interval_length;
   or_circuit_t *orcirc;
+  double mean_num_cells_in_queue, mean_time_cells_in_queue;
+  uint32_t processed_cells;
   if (CIRCUIT_IS_ORIGIN(circ))
     return;
   orcirc = TO_OR_CIRCUIT(circ);
   if (!orcirc->processed_cells)
     return;
-  if (!circuits_for_buffer_stats)
-    circuits_for_buffer_stats = smartlist_create();
   start_of_interval = (circ->timestamp_created.tv_sec >
                        start_of_buffer_stats_interval) ?
         circ->timestamp_created.tv_sec :
@@ -2388,17 +2406,18 @@ rep_hist_buffer_stats_add_circ(circuit_t *circ, time_t end_of_interval)
   interval_length = (int) (end_of_interval - start_of_interval);
   if (interval_length <= 0)
     return;
-  stat = tor_malloc_zero(sizeof(circ_buffer_stats_t));
-  stat->processed_cells = orcirc->processed_cells;
+  processed_cells = orcirc->processed_cells;
   /* 1000.0 for s -> ms; 2.0 because of app-ward and exit-ward queues */
-  stat->mean_num_cells_in_queue = (double) orcirc->total_cell_waiting_time /
+  mean_num_cells_in_queue = (double) orcirc->total_cell_waiting_time /
       (double) interval_length / 1000.0 / 2.0;
-  stat->mean_time_cells_in_queue =
+  mean_time_cells_in_queue =
       (double) orcirc->total_cell_waiting_time /
       (double) orcirc->processed_cells;
-  smartlist_add(circuits_for_buffer_stats, stat);
   orcirc->total_cell_waiting_time = 0;
   orcirc->processed_cells = 0;
+  rep_hist_add_buffer_stats(mean_num_cells_in_queue,
+                            mean_time_cells_in_queue,
+                            processed_cells);
 }
 
 /** Sorting helper: return -1, 1, or 0 based on comparison of two

src/or/rephist.h

@@ -77,6 +77,8 @@ void rep_hist_buffer_stats_add_circ(circuit_t *circ,
                                     time_t end_of_interval);
 time_t rep_hist_buffer_stats_write(time_t now);
 void rep_hist_buffer_stats_term(void);
+void rep_hist_add_buffer_stats(double mean_num_cells_in_queue,
+     double mean_time_cells_in_queue, uint32_t processed_cells);
 char *rep_hist_format_buffer_stats(time_t now);
 void rep_hist_reset_buffer_stats(time_t now);
 

src/test/test.c

@@ -1635,6 +1635,61 @@ test_stats(void)
   rep_hist_reset_conn_stats(now);
   s = rep_hist_format_conn_stats(now + 86400);
   test_streq("conn-bi-direct 2010-08-12 13:27:30 (86400 s) 0,0,0,0\n", s);
+  tor_free(s);
+
+  /* Continue with testing buffer statistics; we shouldn't collect buffer
+   * stats without initializing them. */
+  rep_hist_add_buffer_stats(2.0, 2.0, 20);
+  s = rep_hist_format_buffer_stats(now + 86400);
+  test_assert(!s);
+
+  /* Initialize stats, add statistics for a single circuit, and generate
+   * the history string. */
+  rep_hist_buffer_stats_init(now);
+  rep_hist_add_buffer_stats(2.0, 2.0, 20);
+  s = rep_hist_format_buffer_stats(now + 86400);
+  test_streq("cell-stats-end 2010-08-12 13:27:30 (86400 s)\n"
+             "cell-processed-cells 20,0,0,0,0,0,0,0,0,0\n"
+             "cell-queued-cells 2.00,0.00,0.00,0.00,0.00,0.00,0.00,0.00,"
+                               "0.00,0.00\n"
+             "cell-time-in-queue 2,0,0,0,0,0,0,0,0,0\n"
+             "cell-circuits-per-decile 1\n", s);
+  tor_free(s);
+
+  /* Add nineteen more circuit statistics to the one that's already in the
+   * history to see that the math works correctly. */
+  for (i = 21; i < 30; i++)
+    rep_hist_add_buffer_stats(2.0, 2.0, i);
+  for (i = 20; i < 30; i++)
+    rep_hist_add_buffer_stats(3.5, 3.5, i);
+  s = rep_hist_format_buffer_stats(now + 86400);
+  test_streq("cell-stats-end 2010-08-12 13:27:30 (86400 s)\n"
+             "cell-processed-cells 29,28,27,26,25,24,23,22,21,20\n"
+             "cell-queued-cells 2.75,2.75,2.75,2.75,2.75,2.75,2.75,2.75,"
+                               "2.75,2.75\n"
+             "cell-time-in-queue 3,3,3,3,3,3,3,3,3,3\n"
+             "cell-circuits-per-decile 2\n", s);
+  tor_free(s);
+
+  /* Stop collecting stats, add statistics for one circuit, and ensure we
+   * don't generate a history string. */
+  rep_hist_buffer_stats_term();
+  rep_hist_add_buffer_stats(2.0, 2.0, 20);
+  s = rep_hist_format_buffer_stats(now + 86400);
+  test_assert(!s);
+
+  /* Re-start stats, add statistics for one circuit, reset stats, and make
+   * sure that the history has all zeros. */
+  rep_hist_buffer_stats_init(now);
+  rep_hist_add_buffer_stats(2.0, 2.0, 20);
+  rep_hist_reset_buffer_stats(now);
+  s = rep_hist_format_buffer_stats(now + 86400);
+  test_streq("cell-stats-end 2010-08-12 13:27:30 (86400 s)\n"
+             "cell-processed-cells 0,0,0,0,0,0,0,0,0,0\n"
+             "cell-queued-cells 0.00,0.00,0.00,0.00,0.00,0.00,0.00,0.00,"
+                               "0.00,0.00\n"
+             "cell-time-in-queue 0,0,0,0,0,0,0,0,0,0\n"
+             "cell-circuits-per-decile 0\n", s);
 
  done:
   tor_free(s);