[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #11507 [Tor]: New tests for status.c

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

 * milestone:   => Tor: 0.2.5.x-final


Comment:

 Interesting!  Yes, I think this token-concatenating stuff is probably a
 good approach.

 Initial comments:
   * For reserved-identifier reasons, I'd like to not be declaring any
 macros or identifiers that start with an underscore.  Instead, I've been
 using ends-with-an-underscore to indicate "don't use this".
   * Most of these newly declared mock functions and test functions should
 be static.
   * I think we should divide testsupport.h into parts that are used for
 the whole codebase (like MOCK_IMPL / MOCK_DECL) and parts that are only
 used in the tests themselves.
   * I like "test_main" as a generic test name slightly more than "main".
   * The TEST_CASE and TEST_CASE_ASPECT macros should expose ways to set
 the 'flags' fields -- or maybe they should all have the TT_FORK flag, to
 prevent stray MOCKs from affecting other tests.

  Also, this patch doesn't pass compilation with our full warning suite
 enabled.  (configure with --enable-gcc-warnings for all the ones your
 compiler supports.) Examples:
 {{{
 src/test/test_status.c:36:16: warning: unused parameter 'arg'
       [-Wunused-parameter]
 NS(main)(void *arg)

 src/test/test_status.c:87:12: warning: assigning to 'char *' from
       'const char [11]' discards qualifiers
       [-Wincompatible-pointer-types-discards-qualifiers]
   expected = "0:00 hours";

 src/test/test_status.c:317:1: warning: no previous prototype for function
       'status_log_heartbeat__not_in_consensus_main' [-Wmissing-prototypes]
 NS(main)(void *arg)

 }}}

 (That's a first pass review -- I haven't read the tests themselves in
 detail yet)

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/11507#comment:2>
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