Make script.py wildcard importable. #9943

1. 
   jnewbery commented at 7:45 pm on March 7, 2017: member

   This PR is part of a larger effort to make the test_framework API more intuitive. See #9876 for background.

   Several testcases use a wildcard import from test_framework.script. That’s not ideal as it pollutes the namespace with all of the names defined in test_framework.script, including anything that script.py has imported itself.

   This PR adds an __all__ module variable to script.py which indexes all of the names that should commonly be required by individual test cases. That makes the wildcard import a well-defined API and test cases can wildcard import without worrying about importing unexpected names. If a test case needs to import something from script.py which isn’t included in __all__, it can explicitly import it.

   I’ve not included hash160() or CScriptOp() in __all__ because hash160() is a helper function which I think should be defined elsewhere (ideally in the same module as sha256(), ripemd160() and hash256()), and CScriptOp should not commonly be required by testcases (the only place it’s currently used is in GetP2PKHScript() in p2p-segwit.py, which arguably should be pulled up into the script.py module).

2. fanquake added the label Tests on Mar 7, 2017
3. 
   fanquake commented at 11:05 pm on March 22, 2017: member

   Needs a rebase.
4. jnewbery force-pushed on Apr 12, 2017
5. in test/functional/p2p-fullblocktest.py:21 in 105483c0b7 outdated

   17@@ -18,6 +18,7 @@
   18 import time
   19 from test_framework.key import CECKey
   20 from test_framework.script import *
   21+from test_framework.script import hash160
   

   ---

   ---

   

   jimmysong commented at 7:23 pm on April 20, 2017:

   It seems a little strange that hash160 is in script.py and hash256/sha256 are in mininode.py can we put all three in one place (probably mininode.py)? You’re changing most of the places where this is being imported (smartfees.py and test_framework/address.py seem to be the only other ones). It might be worth just changing this, too.

   ---

   

   jnewbery commented at 7:41 pm on April 28, 2017:

   Yes, I agree that they should all live in the same place. My preference would be for utils.py to actually be pure utility functions and for the hash functions to live in there.

   I haven’t moved it in this commit because that seemed the minimal change. We should move hash160 to live alongside the other functions in a future PR that cleans up utils.py and mininode.py.

6. 
   jimmysong commented at 7:50 pm on April 20, 2017: contributor

   It looks like smartfees.py, bip9-softforks.py and bip65-cltv-p2p.py could also use the same import in test/functional. Also, address.py and blocktools.py could use the same import cleanup in test/functional/test_framework
7. jnewbery force-pushed on Apr 28, 2017
8. Make script.py wildcard importable. d5cc6ff317
9. jnewbery force-pushed on Apr 28, 2017
10. 
    jnewbery commented at 8:02 pm on April 28, 2017: member

    I didn’t change the imports in smartfees.py, bip9-softforks.py and bip65-cltv-p2p.py because there’s no harm in leaving them as explicit. I only moved segwit.py from exlicit to wildcard because it was importing an absurdly long list of names.

    for imports within the test_framework module itself, I think we should aim for them all to be explicit. I have updated address.py. It was previously importing hash256 and sha256 indirectly via script.py. I’ve now changed it so that it imports them directly from mininode.py.

11. 
    jimmysong commented at 8:46 pm on April 28, 2017: contributor

    ACK d5cc6ff31755877eb899bc45497a240e2432615f
12. jimmysong approved
13. 
    jnewbery commented at 6:27 pm on June 30, 2017: member

    I’d love to tidy up imports in the test framework, but there doesn’t seem to be general enthusiasm for this. Closing for now.
14. jnewbery closed this on Jun 30, 2017

    ---

15. MarcoFalke locked this on Sep 8, 2021

Contributors
jnewbery fanquake jimmysong

Labels
Tests