test: Fix rpc_net.py “pong” race condition #15069

1. 
   Empact commented at 10:52 pm on December 31, 2018: member

   Prior to this change, the test fails with KeyError if pong has a zero value at the time this is called, as getpeerinfo’s bytesrecv_per_msg result excludes zero-values.

   Combined these to a single wait_until as well, which will be a bit more forgiving re the timeout while still enforcing the same 2 seconds overall.

   https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/21310881#L62

2. 
   Empact commented at 10:53 pm on December 31, 2018: member

   /cc #12804
3. fanquake added the label Tests on Dec 31, 2018
4. fanquake requested review from jnewbery on Jan 1, 2019
5. in test/functional/rpc_net.py:68 in 39130dedcd outdated

   61@@ -62,8 +62,11 @@ def _test_getnettotals(self):
   62         # the bytes sent/received should change
   63         # note ping and pong are 32 bytes each
   64         self.nodes[0].ping()
   65-        wait_until(lambda: (self.nodes[0].getnettotals()['totalbytessent'] >= net_totals_after['totalbytessent'] + 32 * 2), timeout=1)
   66-        wait_until(lambda: (self.nodes[0].getnettotals()['totalbytesrecv'] >= net_totals_after['totalbytesrecv'] + 32 * 2), timeout=1)
   67+        wait_until(lambda: (
   68+            self.nodes[0].getnettotals()['totalbytessent'] >= net_totals_after['totalbytessent'] + 32 * 2
   69+            and self.nodes[0].getnettotals()['totalbytesrecv'] >= net_totals_after['totalbytesrecv'] + 32 * 2
   70+            and ('pong' in self.nodes[0].getpeerinfo()[0]['bytesrecv_per_msg'])
   

   ---

   ---

   

   MarcoFalke commented at 12:07 pm on January 1, 2019:

   Instead of and, could split them into separate lines with separate wait_untils to see what condition actually fails (if one does)?

   ---

   

   Empact commented at 8:50 pm on January 1, 2019:

   Ah, right, didn’t consider that. Fixed.
6. 
   MarcoFalke commented at 12:10 pm on January 1, 2019: member

   It seems weird that a key error is thrown unless the value is not 0. With the principle of least surprise, it should probably always return the full result with all values (even if they are 0)
7. Empact force-pushed on Jan 1, 2019
8. 
   Empact commented at 8:56 pm on January 1, 2019: member

   @MarcoFalke makes sense to me to show them, given its an rpc command / programmatic interface as well as a human one. zero-values have been excluded since introduction in ca188c629e. /cc @jonasschnelli
9. Empact force-pushed on Jan 1, 2019
10. test: Fix rpc_net.py "pong" race condition

    Prior to this change, the test fails with KeyError if pong has
    a zero value at the time this is called, as getpeerinfo's
    bytesrecv_per_msg result excludes zero-values.
    
    https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/21310881#L62

    de23739b22
11. Empact force-pushed on Jan 1, 2019
12. 
    Empact commented at 9:14 pm on January 1, 2019: member

    On reflection, seems the before case is the more likely culprit, which calls for using get rather than regular key access, to guard against the 0 case.
13. MarcoFalke deleted a comment on Jan 1, 2019
14. 
    laanwj commented at 9:32 am on January 2, 2019: member

    utACK https://github.com/bitcoin/bitcoin/pull/15069/commits/de23739b22fefd7a409f6cb0a4d6128c176fa229, I prefer this solution to #15077
15. MarcoFalke merged this on Jan 2, 2019
16. MarcoFalke closed this on Jan 2, 2019

    ---

17. MarcoFalke referenced this in commit 3ec4cc0a9e on Jan 2, 2019
18. Empact deleted the branch on Jan 2, 2019
19. 
    jnewbery commented at 8:53 pm on January 7, 2019: member

    utACK de23739b22fefd7a409f6cb0a4d6128c176fa229. Thanks @Empact!
20. codablock referenced this in commit e071daa4a5 on Apr 7, 2020
21. codablock referenced this in commit c88ec23d63 on Apr 8, 2020
22. codablock referenced this in commit 351ddf938d on Apr 8, 2020
23. deadalnix referenced this in commit bffbe939ca on Apr 24, 2020
24. ftrader referenced this in commit 30e7ad4ca0 on Aug 17, 2020
25. ckti referenced this in commit da0ee264a4 on Mar 28, 2021
26. MarcoFalke locked this on Dec 16, 2021

Contributors
Empact MarcoFalke laanwj jnewbery

Review Requested
jnewbery

Labels
Tests