[tests] tidy up wallet_importmulti.py #15108

pull amitiuttarwar wants to merge 2 commits into bitcoin:master from amitiuttarwar:14952_rebase changing 3 files +361 −336
  1. amitiuttarwar commented at 2:33 am on January 5, 2019: contributor
    Cherry picks un-merged commits from #14952, which “fixes review comments from @ryanofsky here: #14886#pullrequestreview-183772779”
  2. [tests] tidy up wallet_importmulti.py
    Fixes review comments from PR 14886.
    6be64ef02c
  3. [tests] move wallet util functions to wallet_util.py
    Adds a new wallet_util.py module and moves generic helper functions
    there:
    
    - get_key
    - get_multisig
    - test_address
    2d5f1ea2e3
  4. fanquake added the label Tests on Jan 5, 2019
  5. DrahtBot commented at 4:40 pm on January 5, 2019: member

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

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #15032 (Nit fixes for PR 14565 by MeshCollider)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  6. Sjors commented at 7:42 pm on January 5, 2019: member
    Concept ACK, will review later.
  7. in test/functional/test_framework/wallet_util.py:2 in 2d5f1ea2e3
    0@@ -0,0 +1,99 @@
    1+#!/usr/bin/env python3
    2+# Copyright (c) 2018 The Bitcoin Core developers
    


    promag commented at 8:23 pm on January 6, 2019:
    2019

    MarcoFalke commented at 11:51 am on January 7, 2019:
    This was authored in 2018 (according to the git author date), so I guess it is fine.
  8. promag commented at 9:13 am on January 7, 2019: member
    @amitiuttarwar side note, looks like you haven’t added your email to github?
  9. MarcoFalke commented at 11:51 am on January 7, 2019: member
    utACK 2d5f1ea2e3f9629d313f1d2aff94a15ef8e1128a
  10. sipa commented at 12:01 pm on January 7, 2019: member
    Concept ACK
  11. in test/functional/wallet_importmulti.py:378 in 2d5f1ea2e3
    403+        wrong_privkey = get_key(self.nodes[0]).privkey
    404+        self.test_importmulti({"scriptPubKey": {"address": key.p2pkh_addr},
    405                                "timestamp": "now",
    406                                "keys": [wrong_privkey]},
    407-                               True,
    408+                               success=True,
    


    jnewbery commented at 7:37 pm on January 7, 2019:
    micronit: this line (and the following) is overindented. Remove one leading space (so success is aligned with the opening parens of the function call, not the the opening braces of the object above)
  12. jnewbery commented at 8:02 pm on January 7, 2019: member

    Looks great. Thanks for picking this up @amitiuttarwar!

    There were a couple of review comments from @practicalswift in #14952 which weren’t addressed in the base PR #14565, so you could fix them in this PR if you wanted:

    • wallet_importmulti.py L52 (observed_warnings = result[0]['warnings'] is underindented)
    • the default argument for warnings in test_importmulti() should be None, not [] (see ‘Mutable Default Arguments’ in https://docs.python-guide.org/writing/gotchas/)

    I’ve also added one small nit of my own.

    tested ACK 2d5f1ea2e3f9629d313f1d2aff94a15ef8e1128a

  13. Sjors commented at 6:48 pm on January 9, 2019: member

    tACK 2d5f1ea2e3f9629d313f1d2aff94a15ef8e1128a

    Fixing nits is nice, but adding tests to #14491 will be easier once this is merged (cc @MeshCollider).

  14. MarcoFalke commented at 10:00 pm on January 9, 2019: member
    I am merging this, because it has some review and the nits are only about whitespace in touched lines or additional cleanup in unrelated lines. Please fix them up separately if anyone feels like it.
  15. MarcoFalke merged this on Jan 9, 2019
  16. MarcoFalke closed this on Jan 9, 2019

  17. MarcoFalke referenced this in commit 1cfbb16b5d on Jan 9, 2019
  18. amitiuttarwar deleted the branch on Jan 14, 2019
  19. deadalnix referenced this in commit 97962fdeac on May 21, 2020
  20. deadalnix referenced this in commit 3673616a27 on May 21, 2020
  21. DrahtBot 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-10-30 00:12 UTC

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