[metrics-bugs] #33617 [Core Tor/Tor]: Add a BandwidthStatistics option and consensus parameter

Tor Bug Tracker & Wiki blackhole at torproject.org
Thu May 28 10:22:19 UTC 2020


#33617: Add a BandwidthStatistics option and consensus parameter
-------------------------------------------------+-------------------------
 Reporter:  teor                                 |          Owner:
                                                 |  MrSquanchee
     Type:  enhancement                          |         Status:
                                                 |  needs_revision
 Priority:  Medium                               |      Milestone:  Tor:
                                                 |  0.4.4.x-final
Component:  Core Tor/Tor                         |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  extra-review, prop313, ipv6,         |  Actual Points:
  outreachy-ipv6, network-team-roadmap-2020Q1    |
Parent ID:  #33052                               |         Points:  1
 Reviewer:  asn                                  |        Sponsor:
                                                 |  Sponsor55-can
-------------------------------------------------+-------------------------

Comment (by asn):

 Replying to [comment:38 MrSquanchee]:
 > Hii, asn.

 Hello MrSquanchee,

 thanks for showing interest to write these tests. Let me try to help out.

 > 1. `rep_hist_bw_stats_write()` gets called from
 `write_stats_file_callback` in mainloop.c.
 >  This function handles disk write for all the stats produced.
 >  So, do you want exhaustive unit-tests for `write_stats_file_callback`,
 which would test
 >  for all the stats ?? or should I write unit tests for
 `write_stats_file_callback`
 >  pertaining only to the bandwidth statistics ??

 I would like unittests for `write_stats_file_callback` pertaining only to
 the bandwidth statistics.

 In particular, you could call that function with a special `options`
 argument that only activates the bandwidth statistics. As part of this, I
 would like to see that the separation between `BandwidthStatistics` and
 `ExtraInfoStatistics` is clear. That is, how does each of those two
 options influence the other? Given that this is one of the original
 purposes of this ticket (i.e. not having `ExtraInfoStatistics` control
 everything) I would like the test to be able to test that things work as
 expected.

 > 2. Also, `rep_hist_bw_stats_write()` writes the stats to the disk, so
 does many other stat
 >  functions, but I haven't yet seen a unit test that tests for a
 directory and contents of
 >  a file maybe for some reasons I don't know or maybe I am wrong and you
 can point me to
 >  an appropriate test.
 >  Would you like to explain how I can write such a test  ??

 There are a bunch of ways to achieve this. One way would be to actually do
 the file writes; see how `test_config_write_to_data_subdir()` and
 `test_config_check_or_create_data_subdir()` do this. IMO this would be the
 best and most robust way to approach this (also probably simpler).

 The other way, would be to mock the file-writing functions, so that their
 behavior does not happen when they are getting tested. As an example, of a
 test that does mocking check `test_config_resolve_my_address()`.

 Best of luck in this adventure!

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/33617#comment:39>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online


More information about the metrics-bugs mailing list