qa: Test ZMQ notification after chain reorg #16404

pull promag wants to merge 3 commits into bitcoin:master from promag:2019-07-zmq-reorg changing 2 files +59 −39
  1. promag commented at 1:45 PM on July 17, 2019: member

    No description provided.

  2. fanquake added the label Tests on Jul 17, 2019
  3. promag force-pushed on Jul 17, 2019
  4. promag force-pushed on Jul 17, 2019
  5. promag force-pushed on Jul 17, 2019
  6. promag force-pushed on Jul 17, 2019
  7. in test/functional/interface_zmq.py:60 in e11aa5a6f2 outdated 
      60 |          # that this test fails if the publishing order changes.
  61 |          # Note that the publishing order is not defined in the documentation and
  62 |          # is subject to change.
  63 | -        self.zmq_context = zmq.Context()
  64 | -        socket = self.zmq_context.socket(zmq.SUB)
  65 | +        import zmq
    kallewoof commented at 9:19 AM on August 5, 2019:

    Is it normal to import stuff in the middle of a file in python? Don't see it often. And you could remove the one below in test_reorg if you just moved this to top..

    promag commented at 9:29 AM on August 5, 2019:

    This was already the case. It's like this to not break if zmq module is not available. See skip_if_no_py3_zmq() above.

    kallewoof commented at 9:36 AM on August 5, 2019:

    Ah, that makes sense..

    MarcoFalke commented at 12:53 PM on August 5, 2019:

    Can move the import zmq right after the line skip_if_no_bitcoind_zmq? :)

    promag commented at 12:56 PM on August 5, 2019:

    Really? It's then available to other scopes? I'll try.

    promag commented at 2:24 PM on August 5, 2019:

    If I remove import zmq and keep the one you said I get NameError: name 'zmq' is not defined.

  8. laanwj commented at 12:44 PM on August 5, 2019: member

    ACK e11aa5a6f2683741567f0aaa08d3b198d219913f

  9. in doc/zmq.md:116 in e11aa5a6f2 outdated 
     110 | @@ -111,7 +111,9 @@ using other means such as firewalling.
 111 |  
 112 |  Note that when the block chain tip changes, a reorganisation may occur
 113 |  and just the tip will be notified. It is up to the subscriber to
 114 | -retrieve the chain from the last known block to the new tip.
 115 | +retrieve the chain from the last known block to the new tip. Also note
 116 | +that no notification occurs if the tip was in the active chain - this
 117 | +is the case after calling invalidateblock RPC.
    MarcoFalke commented at 12:52 PM on August 5, 2019:

    invalidateblock is not for production purposes, so I'd rather not mention that here. At most I'd mention this in the invalidateblock rpc itself.

    promag commented at 12:55 PM on August 5, 2019:

    Good point, I'll improve invalidateblock help then.

    promag commented at 2:25 PM on August 5, 2019:

    Done.

  10. in test/functional/interface_zmq.py:14 in be08278a3f outdated 
       9 | @@ -10,12 +10,11 @@
  10 |  from test_framework.messages import CTransaction
  11 |  from test_framework.util import (
  12 |      assert_equal,
  13 | +    connect_nodes,
  14 |      hash256,
  15 |  )
  16 |  from io import BytesIO
  17 |  
  18 | -ADDRESS = "tcp://127.0.0.1:28332"
    MarcoFalke commented at 12:55 PM on August 5, 2019:

    in commit be08278a3f7348a346b60421b2c929a86020a4db

    Would be nice to give a rationale for this "refactor". Otherwise, I'd be NACK on this commit

    laanwj commented at 1:16 PM on August 5, 2019:

    I think this was moved into the specific test because the other test uses a different address, so there's no reason to share this globally

    promag commented at 1:18 PM on August 5, 2019:

    Yes, the refactor is to avoid state and constants across test cases.

    laanwj commented at 1:44 PM on August 5, 2019:

    Avoiding constants across test cases is not a goal in itself though. If you can share, say, constants for magic values that's good. Just in this specific case…

    promag commented at 1:52 PM on August 5, 2019:

    I mean that each test case needs a different address and I thought having a constant for each test is pointless, that's why I moved the value inline.

    But I'll happily revert, the refactor is just to simplify the next commit - I'll add a rationale.

    MarcoFalke commented at 2:11 PM on August 5, 2019:

    Sorry, didn't realize they were different addresses. ACK in that case.

    promag commented at 2:25 PM on August 5, 2019:

    Rationale added.

  11. promag force-pushed on Aug 5, 2019
  12. DrahtBot commented at 2:07 AM on August 14, 2019: member

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    No conflicts as of last run.

  13. DrahtBot added the label Needs rebase on Aug 14, 2019
  14. doc: Add note regarding ZMQ block notification 6bc1ff915d
  15. qa: Refactor ZMQ test aa2622a726
  16. qa: Test ZMQ notification after chain reorg abdfc5e89b
  17. promag force-pushed on Aug 19, 2019
  18. promag commented at 12:26 AM on August 19, 2019: member

    Rebased.

  19. DrahtBot removed the label Needs rebase on Aug 19, 2019
  20. promag commented at 10:21 AM on August 25, 2019: member

    IMO this is ready to merge.

  21. MarcoFalke commented at 1:14 PM on August 26, 2019: member

    Indeed. Thanks for the ping

  22. MarcoFalke referenced this in commit af4100cd6f on Aug 26, 2019
  23. MarcoFalke merged this on Aug 26, 2019
  24. MarcoFalke closed this on Aug 26, 2019
  25. promag deleted the branch on Aug 26, 2019
  26. sidhujag referenced this in commit 0ff97617aa on Aug 27, 2019
  27. MarcoFalke commented at 7:31 PM on August 27, 2019: member

    This test fails on travis

  28. promag commented at 9:46 PM on August 27, 2019: member

    The test should run pretty fast, on my system around 1s:

    2019-08-27T20:59:50.749000Z TestFramework (INFO): Test the getzmqnotifications RPC
2019-08-27T20:59:51.603000Z TestFramework (INFO): Stopping nodes

    But on one travis failure it takes around 60s to fail:

    2019-08-27T17:18:36.885000Z TestFramework (INFO): Test the getzmqnotifications RPC
2019-08-27T17:19:37.373000Z TestFramework (ERROR): Unexpected exception caught during testing

    and looking at the error:

    zmq.error.Again: Resource temporarily unavailable

    I assume the receive timeouts due to:

            socket.set(zmq.RCVTIMEO, 60000)

    I suspect it happens because by the time the message is published there is no subscriber connected - this is called a "slow joiner" in http://zguide.zeromq.org/py:all#sockets-and-patterns - and I think this happens because we are restart_node after socket.connect. I'll submit a PR to invert this order to see if it stops happening, otherwise we'll have to make sure the subscriber is ready before publishing messages.

  29. jasonbcox referenced this in commit a9f7ee10d4 on Sep 3, 2020
  30. jasonbcox referenced this in commit ce6847916a on Sep 3, 2020
  31. jasonbcox referenced this in commit 14653a00bd on Sep 3, 2020
  32. vijaydasmp referenced this in commit df73c4f5a3 on Dec 6, 2021
  33. vijaydasmp referenced this in commit 4778f9ecf9 on Dec 10, 2021
  34. vijaydasmp referenced this in commit 76edb07716 on Dec 13, 2021
  35. vijaydasmp referenced this in commit 27d2330375 on Dec 13, 2021
  36. vijaydasmp referenced this in commit cf43f40fb4 on Dec 15, 2021
  37. DrahtBot locked this on Dec 16, 2021
Contributors
promagkallewoofMarcoFalkelaanwjDrahtBot
Labels
Tests
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: 2026-04-13 15:14 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me