This commit adds module-level docstrings for the tests and helper modules in qa. Many of these tests were uncommented previously - this commit ensures that every test case has at least a minimum level of commenting.
It also puts the module-level docstring in front of the imports.
Thanks for adding documentation. utACK
utACK 077b626
Fast review ACK 077b626
Needs rebase
rebased
@marcofalke you have tagged yourself as reviewer, this can be merged as soon as you are ok with it
This does seem like an improvement that should be merged, though it would be nicer if it followed PEP257 to work better with pydoc.
Main reason these strings don't look good in pydoc is they don't follow the guideline to begin with summary lines. A summary line is "a phrase ending in a period. It prescribes the function or method's effect as a command, not as a description." Instead they start with filenames. I think the filenames should be removed even if we don't want to take the time to write summary lines.
Thanks @ryanofsky . I've made the docstrings conform with PEP257. Can you check the pydocs again?
I've also added docstrings for the modules that have been added since opening this PR.
7 | -# 8 | -# Helper script to create the cache 9 | -# (see BitcoinTestFramework.setup_chain) 10 | -# 11 | +Creating a cache of the blockchain speeds up test execution when running 12 | +multiple qa tests. This helper script is executed by rpc-tests when multipl
missing a trailing e?
7 | -# 8 | -# Exercise API with -disablewallet. 9 | -# 10 | +Tests: 11 | + - validateaddress RPC doesn't cause bitcoind to crash when running with -disablewallet 12 | + (https://github.com/bitcoin/bitcoin/issues/6963#issuecomment-154548880)
I think it is not required to be so specific in the doc string. You can leave this link where it was or remove it alltogether.
The test just checks if validateaddress works (even) with the wallet disabled. No need to mention "crash", imo.
utACK mod 2 nits.
Thanks for the comments @MarcoFalke - I appreciate this isn't the most thrilling PR to review!
Nits addressed. I will squash when ready to merge.
@jnewbery Thanks. ACK 82bc966 This is ready for squash.
1 | @@ -2,10 +2,7 @@ 2 | # Copyright (c) 2014-2016 The Bitcoin Core developers 3 | # Distributed under the MIT software license, see the accompanying 4 | # file COPYING or http://www.opensource.org/licenses/mit-license.php. 5 | - 6 | -# 7 | -# Test fee estimation code 8 | -# 9 | +""" Test fee estimation code."""
Trailing whitespace on this line.
13 | import dbm.dumb as dbmd 14 | 15 | class BlockStore(object): 16 | + """ 17 | + BlockStore helper class. 18 | +
Trailing whitespace
1 | @@ -2,16 +2,21 @@ 2 | # Copyright (c) 2015-2016 The Bitcoin Core developers 3 | # Distributed under the MIT software license, see the accompanying 4 | # file COPYING or http://www.opensource.org/licenses/mit-license.php. 5 | -# BlockStore: a helper class that keeps a map of blocks and implements 6 | -# helper functions for responding to getheaders and getdata, 7 | -# and for constructing a getheaders message 8 | -# 9 | +""" Provides BlockStore and TxStore helper classes. """
Unintentended space before close quote?
Again this is already a big improvement as is, so utACK 82bc966aaa9c29555db90f638b3ae65a63d1941d.
But consider running:
sed -i 's/""" \+/"""/' *.py
sed -i 's/\(Test\|Create\)s/\1/g' *.py
to get rid of asymmetric spaces after opening quotes, and to fix verb tenses (commands, not descriptions) for pep257.
This commit fixes the module-level docstrings for the tests and helper
modules in qa. Many of these tests were uncommented previously - this
commit ensures that every test case has at least a minimum level of
commenting.
Thanks @ryanofsky . Nits addressed and commits squashed. I think this is now ready to merge.