Add walletnotify functional test #10966

pull promag wants to merge 1 commits into bitcoin:master from promag:2017-07-walletnotify-functional-test changing 2 files +63 −0
  1. promag commented at 9:39 pm on July 31, 2017: member
    Similar to #10941, but for -walletnotify option.
  2. promag force-pushed on Jul 31, 2017
  3. practicalswift commented at 5:03 am on August 1, 2017: contributor
    Concept ACK 👍
  4. in test/functional/test_runner.py:152 in 647920c7cc outdated
    148@@ -149,6 +149,7 @@
    149     'invalidateblock.py',
    150     'p2p-acceptblock.py',
    151     'replace-by-fee.py',
    152+    'walletnotify-py',
    


    jnewbery commented at 5:54 pm on August 2, 2017:
    should be 'walletnotify.py'

    promag commented at 7:17 pm on August 2, 2017:
    Yes.
  5. jnewbery commented at 6:54 pm on August 2, 2017: member

    This test currently fails for me:

    0Traceback (most recent call last):
    1  File "/home/ubuntu/bitcoin/test/functional/test_framework/test_framework.py", line 149, in main
    2    self.run_test()
    3  File "./walletnotify.py", line 61, in run_test
    4    assert_equal(list(map(lambda t: t['txid'], self.nodes[0].listtransactions("*", block_count))), f.read().splitlines())
    5  File "/home/ubuntu/bitcoin/test/functional/test_framework/util.py", line 36, in assert_equal
    6    raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args))
    7AssertionError: not(['f9f2bb01dc512ad6ef8dff8de16995862891be39885f1b922426d83784fc17f3', 'a655a9f15488093e752a870c60578be1ec41e2d013bad183d865c221318bb26f', '07d8beee3fb2e18cd6458ef6d9263bf8a0f58fe81cd799eb4e6319777858ccf4', 'f875332c232f29ab931ff7964ab3ef062584255ce8c446c759091a2073ec4e08', 'ba7f2b8b19428a53f3e3f294bab54617d33232bdfdcfd59cd99a8ba15280575e', '6048b098928225e5a10d778602885009621416f1ce19509e5d39a7ae1bca8257', 'ab5972edc4f7dfe0d0cf8b4b38900e6d97e09bcdcfc43ae15b23ede4071deb32', '906720b1b720e6a4b9893ca2ff1dc11c42fe17ea82f039dfbe4119f616932e8e', 'c1226a7cca4cec6f4d6408a2e1f025267fa10ffd60b7d909a12faacf65fcfa5c', '1caf4ffcd307189f8189d7b76aea65b41eb25680e7207191912c83d54237b71f'] == ['f9f2bb01dc512ad6ef8dff8de16995862891be39885f1b922426d83784fc17f3', 'a655a9f15488093e752a870c60578be1ec41e2d013bad183d865c221318bb26f', '07d8beee3fb2e18cd6458ef6d9263bf8a0f58fe81cd799eb4e6319777858ccf4', 'ba7f2b8b19428a53f3e3f294bab54617d33232bdfdcfd59cd99a8ba15280575e', '6048b098928225e5a10d778602885009621416f1ce19509e5d39a7ae1bca8257', '1caf4ffcd307189f8189d7b76aea65b41eb25680e7207191912c83d54237b71f', 'ab5972edc4f7dfe0d0cf8b4b38900e6d97e09bcdcfc43ae15b23ede4071deb32', '906720b1b720e6a4b9893ca2ff1dc11c42fe17ea82f039dfbe4119f616932e8e', 'c1226a7cca4cec6f4d6408a2e1f025267fa10ffd60b7d909a12faacf65fcfa5c', 'f875332c232f29ab931ff7964ab3ef062584255ce8c446c759091a2073ec4e08'])
    

    ordering is different between the lists.

  6. promag commented at 7:17 pm on August 2, 2017: member
    Makes sense since there are no order guarantees of notification commands. Should that be fixed or for now sort both arrays?
  7. jnewbery commented at 7:35 pm on August 2, 2017: member

    there are no order guarantees of notification commands. Should that be fixed or for now sort both arrays?

    I think it’s fine not to change to sort the arrays in this test (as long as you add a comment to say that notifications are not guaranteed to be in order).

    Feel free to open a separate pull request if you think notifications should be in order.

  8. promag force-pushed on Aug 2, 2017
  9. promag commented at 9:26 pm on August 2, 2017: member
    @jnewbery if the out-of-order is not handled in the test, this will cause unrelated travis failures. If at the moment the order is not guaranteed, then the test must handle that IMO. In the future, if the order is guaranteed, then the test is adjusted.
  10. promag commented at 9:27 pm on August 2, 2017: member
    BTW, the same can happen with -blocknotify.
  11. promag commented at 9:42 pm on August 2, 2017: member
    @jnewbery pushed a new commit with a tiny refactor where I also address your comments from #10941.
  12. jnewbery commented at 9:42 pm on August 2, 2017: member
    I’ve been thinking: Does it make sense to combine this and blocknotify with the existing forknotify.py test?
  13. jonasschnelli added the label Tests on Aug 3, 2017
  14. jonasschnelli commented at 9:43 am on August 3, 2017: contributor
    utACK b982d6db88910aa90ed3a78ef15401326e387f9b (after squash). I agree with @jnewbery about combining this (and rename) with forknotify.py (maybe rename notifyexec.py or notify.py). Could also be done in a later PR.
  15. promag commented at 9:52 am on August 3, 2017: member
    I tend to prefer isolated and simple tests. It’s easier to understand and simpler to maintain.
  16. test: Add walletnotify functional test 7331ea404e
  17. promag force-pushed on Aug 3, 2017
  18. MarcoFalke renamed this:
    Add walletnotify functional test
    qa: Add walletnotify functional test
    on Sep 13, 2017
  19. in test/functional/walletnotify.py:20 in 7331ea404e
    15+from test_framework.util import assert_equal
    16+
    17+class WalletNotifyTest(BitcoinTestFramework):
    18+
    19+    def __init__(self):
    20+        super().__init__()
    


    MarcoFalke commented at 6:56 pm on October 2, 2017:
    Needs rebase due to silent merge conflict, I guess
  20. in test/functional/walletnotify.py:50 in 7331ea404e
    45+    def test_file_content(self):
    46+        txids_rpc = list(map(lambda t: t['txid'], self.nodes[0].listtransactions("*", self.block_count)))
    47+
    48+        # wait at most 10 seconds for expected file size before reading the content
    49+        attempts = 0
    50+        while os.stat(self.path).st_size < (self.block_count * 65) and attempts < 100:
    


    MarcoFalke commented at 6:57 pm on October 2, 2017:
    can use wait_until, no?
  21. promag commented at 7:02 pm on October 2, 2017: member
    I’ll refresh this PR.
  22. promag renamed this:
    qa: Add walletnotify functional test
    Add walletnotify functional test
    on Oct 4, 2017
  23. promag commented at 0:33 am on October 10, 2017: member
    Merged in #10941 as per @jnewbery suggestion.
  24. promag closed this on Oct 10, 2017

  25. promag deleted the branch on Oct 10, 2017
  26. DrahtBot locked this on Sep 8, 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-10-30 03:12 UTC

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