magic-wormhole / magic-wormhole-transit-relay

Transit Relay server for Magic-Wormhole
MIT License
169 stars 42 forks source link

iosim.IOPump based tests #20

Closed meejah closed 3 years ago

meejah commented 3 years ago

This is essentially back-story for WebSocket support .. this switches the tests to use iosim so that we use a real client -> server protocol and transport. This also uses a consistent client protocol. For TCP this isn't substantially different from just directly tweaking Twisted APIs. For WebSocket this is necessary (because the client has to do a WebSocket negotiation properly before it can send messages).

Also changes "mock" location to match python3

codecov[bot] commented 3 years ago

Codecov Report

Merging #20 (00086a7) into master (4f818bb) will increase coverage by 0.29%. The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #20      +/-   ##
==========================================
+ Coverage   98.79%   99.09%   +0.29%     
==========================================
  Files           5        5              
  Lines         332      331       -1     
  Branches       48       48              
==========================================
  Hits          328      328              
+ Misses          2        1       -1     
  Partials        2        2              
Impacted Files Coverage Δ
src/wormhole_transit_relay/transit_server.py 99.49% <66.66%> (+0.49%) :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 4f818bb...00086a7. Read the comment docs.

meejah commented 3 years ago

I feel like codecov must be "just wrong" here? How can the patch not affect coverage, but somehow .. overall coverage down?

meejah commented 3 years ago

Previous push, codecov said 100% for the patch .. now it says 66.66% ??!

meejah commented 3 years ago

While pairing with @hacklschorsch we found why the double-flush()-es were needed so that is now gone.

meejah commented 3 years ago

locally I get 100% patch coverage. codecov seems to have two opinions here: either this raises project coverage, or it only has two thirds coverage of patch??

meejah commented 3 years ago

Based on Florian's comment that this is easier to read, I'm merging it ..