meejah / txtorcon

Twisted-based asynchronous Tor control protocol implementation. Includes unit-tests, examples, state-tracking code and configuration abstraction.

http://fjblvrw2jrxnhtg67qpbzi45r7ofojaoo3orzykesly2j3c2m3htapid.onion/

MIT License

250 stars 72 forks source link

245 multiple connect #249

Closed meejah closed 7 years ago

meejah commented 7 years ago

This is https://github.com/meejah/txtorcon/pull/246 with an improved test on top

codecov-io commented 7 years ago

Codecov Report

Merging #249 into master will decrease coverage by <.01%. The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #249      +/-   ##
==========================================
- Coverage   99.94%   99.94%   -0.01%     
==========================================
  Files          20       20              
  Lines        3662     3661       -1     
==========================================
- Hits         3660     3659       -1     
  Misses          2        2

Impacted Files	Coverage Δ
txtorcon/endpoints.py	`100% <100%> (ø)`	:arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1d88253...5163ddd. Read the comment docs.

meejah commented 7 years ago

(Why does codecov think the coverage went down? I can't convince any local tools to tell me the same thing ...)

coveralls commented 7 years ago

Coverage decreased (-1.0e-05%) to 99.945% when pulling 5163ddd0af4077fd05fb32d6e2d44597554ce8c1 on 245-multiple-connect into 1d882534f596280fe18f5fa5982a9d3eab636654 on master.

warner commented 7 years ago

The branch with your added tests works in my local port-forwarder, as expected.

Running 'tox' locally fails, I think I'm missing the GeoIP.h dev headers (I see you commented that out of requirements.txt, but dev-requirements.txt has it, and I'm guessing that tox uses the dev dependencies). But eyeballing the tests, it seems reasonable to me. The only thing I might suggest (but it's super-cosmetic is clearing ports_attempted after the first connect(), so the two assertEqual() statements read the same.