[tor-bugs] #12955 [Tor]: New tests for routerset.c

[Tor Bug Tracker & Wiki]

#12955: New tests for routerset.c
-----------------------------+--------------------------------
     Reporter:  _x3j11       |      Owner:
         Type:  enhancement  |     Status:  needs_review
     Priority:  normal       |  Milestone:  Tor: 0.2.6.x-final
    Component:  Tor          |    Version:
   Resolution:               |   Keywords:  tor-client
Actual Points:               |  Parent ID:
       Points:               |
-----------------------------+--------------------------------
Changes (by nickm):

 * keywords:   => tor-client
 * type:  defect => enhancement


Comment:

 Wow, thorough.  Awesome.

 Two quick questions, though.

 First, why add the -g to the cflags in src/test/include.am ? It should be
 getting added to CFLAGS by configure.ac, right?

 Second, why the 'return NULL' mocking for strmap_new() and smartlist_new()
 and digestmap_new() ?  They let us make sure that the components of the
 routerset really get initialized, but I think this approach creates a few
 problems:

   * It possibly over-fits the specific implementation for routerset:
 there's nothing written stone saying that a routerset must allocate N
 smartlists, M digestmaps, etc.
   * It makes the functions behave differently from their real versions.
 (The real strmap_new() can never return NULL, for example.)

 I wouldn't mind these issues so much if the tests were clearly marked to
 say which ones were glass-box tests that are expected to break if the
 implementation of routerset changes, and which are black-box tests that
 represent real requirements on the behavior of routerset that ought to be
 met by any implementation of routerset.

 What do you think?

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