Ignore unknown answers

[diegomrsantos]

No description provided.

[codecov]

Codecov Report

Merging #831 (5b2cc8a) into unstable (676786b) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable     #831      +/-   ##
============================================
- Coverage     83.69%   83.67%   -0.02%     
============================================
  Files            82       82              
  Lines         14907    14908       +1     
============================================
- Hits          12476    12474       -2     
- Misses         2431     2434       +3     

Impacted Files	Coverage Δ
libp2p/services/autonatservice.nim	92.63% <100.00%> (+2.20%)	⬆️
libp2p/protocols/secure/secure.nim	71.15% <0.00%> (-3.85%)	⬇️
libp2p/muxers/mplex/mplex.nim	88.23% <0.00%> (-0.85%)	⬇️
libp2p/transports/wstransport.nim	85.26% <0.00%> (ø)

[alrevuelta]

lgtm! thanks!

We can leave it for another PR, but I think it would be nice to have a couple of simple tests wihtout the autonat stub, so using a real autonat. I would suggest just two tests:

• 3 peers, all of them with autonat. peer1 connects to peer2/3. We can see its reachable with 100% confidence.
• 3 peers, where peer2 supports autonat but peer3 does not. Here we should see that node1 is also labeled as Reachable with 100 confidence, since peer3 responses are dicarded due to not supporting autonat.

imho the stub is only useful for scenarios that we can't reproduce, like NotReachable. For the rest, the closer we get to a real scenario, the better.

[diegomrsantos]

lgtm! thanks!

We can leave it for another PR, but I think it would be nice to have a couple of simple tests wihtout the autonat stub, so using a real autonat. I would suggest just two tests:

• 3 peers, all of them with autonat. peer1 connects to peer2/3. We can see its reachable with 100% confidence.
• 3 peers, where peer2 supports autonat but peer3 does not. Here we should see that node1 is also labeled as Reachable with 100 confidence, since peer3 responses are dicarded due to not supporting autonat.

imho the stub is only useful for scenarios that we can't reproduce, like NotReachable. For the rest, the closer we get to a real scenario, the better.

I agree. That's how I started testing, but it was harder to know when to finish the test. I used sleepAsync which worked on Linux and Mac, but didn't with Nim 1.2 on Windows. Using sleepAsync doesn't seem like a reliable way to do that anyway. It was easier to solve it with the stub, but there might be another solution too.

[Menduist]

Can't you wait for the callback to be called?

[diegomrsantos]

Can't you wait for the callback to be called?

Probably yes, a lot of refactoring happened since then.