[tor-bugs] #17101 [Core Tor/Tor]: Tests for connection_ap_handshake_rewrite_and_attach

[Tor Bug Tracker & Wiki]

#17101: Tests for connection_ap_handshake_rewrite_and_attach
-------------------------------------------------+-------------------------
 Reporter:  ipazmino                             |          Owner:
     Type:  defect                               |  ipazmino
 Priority:  Medium                               |         Status:
Component:  Core Tor/Tor                         |  needs_revision
 Severity:  Normal                               |      Milestone:  Tor:
 Keywords:  coverage, merge-ready, tor-tests-    |  0.2.9.x-final
  coverage, tor-tests-unit, TorCoreTeam201605,   |        Version:
  TorCoreTeam-postponed-201604                   |     Resolution:
Parent ID:                                       |  Actual Points:
 Reviewer:  asn                                  |         Points:  medium
                                                 |        Sponsor:
                                                 |  SponsorS-can
-------------------------------------------------+-------------------------
Changes (by asn):

 * status:  needs_review => needs_revision


Comment:

 Hello,

 thanks for the code! Here is an initial review:

 - It seems like branch `connection_edge_handshake_tests` has the master
 branch '''merged into it''', instead of it being '''rebased''' on top of
 the master branch. This will not apply cleanly on top of upstream master.
 I'd suggest you start with the master branch and then apply your changes
 on top of it. Or git-rebase master on top of your changes.

 - The
 `conn_edge_ap_handshake/rewrite_and_attach_closes_conn_for_excluded_exit`
 test seems to crash as follows:
 {{{
 May 04 16:33:06.300 [err] tor_assertion_failed_(): Bug:
 src/common/container.c:1410: strmap_set: Assertion val failed; aborting.
 (on Tor 0.2.8.1-alpha-dev 9c6a817c53093831)
 May 04 16:33:06.300 [err] Bug: Assertion val failed in strmap_set at
 src/common/container.c:1410. Stack trace: (on Tor 0.2.8.1-alpha-dev
 9c6a817c53093831)
 May 04 16:33:06.300 [err] Bug:     ./src/test/test(log_backtrace+0x52)
 [0x7fc4dd28a1c2] (on Tor 0.2.8.1-alpha-dev 9c6a817c53093831)
 May 04 16:33:06.300 [err] Bug:
 ./src/test/test(tor_assertion_failed_+0xa4) [0x7fc4dd29eaa4] (on Tor
 0.2.8.1-alpha-dev 9c6a817c53093831)
 May 04 16:33:06.300 [err] Bug:     ./src/test/test(strmap_set+0x197)
 [0x7fc4dd296fd7] (on Tor 0.2.8.1-alpha-dev 9c6a817c53093831)
 May 04 16:33:06.300 [err] Bug:     ./src/test/test(+0x204ab9)
 [0x7fc4dcf25ab9] (on Tor 0.2.8.1-alpha-dev 9c6a817c53093831)
 May 04 16:33:06.300 [err] Bug:     ./src/test/test(+0x3b87fa)
 [0x7fc4dd0d97fa] (on Tor 0.2.8.1-alpha-dev 9c6a817c53093831)
 May 04 16:33:06.300 [err] Bug:     ./src/test/test(testcase_run_one+0x165)
 [0x7fc4dd0d99c5] (on Tor 0.2.8.1-alpha-dev 9c6a817c53093831)
 May 04 16:33:06.300 [err] Bug:     ./src/test/test(tinytest_main+0x11e)
 [0x7fc4dd0da11e] (on Tor 0.2.8.1-alpha-dev 9c6a817c53093831)
 May 04 16:33:06.300 [err] Bug:     ./src/test/test(main+0x347)
 [0x7fc4dcddb8c7] (on Tor 0.2.8.1-alpha-dev 9c6a817c53093831)
 May 04 16:33:06.300 [err] Bug:     /lib/x86_64-linux-
 gnu/libc.so.6(__libc_start_main+0xf5) [0x7fc4db593b45] (on Tor 0.2.8.1
 -alpha-dev 9c6a817c53093831)
 May 04 16:33:06.300 [err] Bug:     ./src/test/test(+0xbc0c2)
 [0x7fc4dcddd0c2] (on Tor 0.2.8.1-alpha-dev 9c6a817c53093831)
 }}}

   The `strmap_set()` call seems to be the offending call here. Also what
 does:
   {{{  routerset_t *excluded_nodes = excluded_nodes = routerset_new()}}}
 do?
   Let me know if you can't reproduce this and you need help debugging this
 further.

 - There seem to be some unnecessary whitespace changes in
 `src/test/include.am`.

 - There seem to be some printf calls left over in the tests.

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