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

["Tor Bug Tracker & Wiki" <blackhole@xxxxxxxxxxxxxx>]

#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:               |
-----------------------------+--------------------------------

Comment (by _x3j11):

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

 Oh, that -g in cflags might have been a leftover thing when I was
 debugging. Let me doublecheck if -g gets added properly, otherwise, I'll
 remove it.

 > 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's hard to get the right balance between over-fitting as you describe
 and thoroughness. The routerset_new test is probably guilty of that, true.
 Perhaps it's just sufficient to test the routerset's members are non-NULL
 and check that the *_new methods have been called. (The *_new methods just
 return NULL out of "laziness" -- I was trying to avoid creating the real
 objects here, but on further musing, they could just as easily return
 empty blobs of memory. Let me try and rejig that)

 I'll have a scan through and see if there's any other serious instances of
 overfitting like this, and try and weaken them a little.

 > It makes the functions behave differently from their real versions. (The
 real strmap_new() can never return NULL, for example.)

 This is tricky and I'm not sure what is appropriate here: should test-
 writing require cognizance of the behavior of modules not under test? how
 much codification of specific inter-module contracts be done in tests?
 given that there are asserts throughout e.g., on malloc failing, do we
 codify the absence of these with tests? can we assert (positive/negative)
 that an assert (abort) has tripped with the current test infrastructure?
 (I don't think that's true at the moment, but it's something we should do.
 I'll look into that.)

 > 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.

 That seems reasonable. I'll attempt to do that as well.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/12955#comment:3>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs