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

pull Empact wants to merge 1 commits into bitcoin:master from Empact:rpc_net-race changing 1 files +2 −2
  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
  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

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-19 00:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me