[tests] rename functional tests #11047
Conversation
There was a problem hiding this comment.
I like this because there have been times I haven't known how to name a new test based on the existing names. Two suggestions:
- Could more consistently abbreviate transaction as either tx or txn
- Maybe rename to
category/test.pyinstead ofcategory_test.pyto make it easier to distinguish between test utitilities (like combine_logs.py and test_runner.py) and tests, and to make the utilities more discoverable.
👍 |
|
Concept ACK. At this point we have so many test scripts that some sort of naming scheme or grouping really makes sense. |
|
I would have preferred folders but I guess that makes things complicated. |
|
Would you mind adding the naming rules from the OP to the QA developer
notes?
…On Tue, Aug 15, 2017 at 8:41 PM, Jonas Schnelli ***@***.***> wrote:
I would have preferred folder but I guess that makes things complicated.
utACK deda19e
<deda19e>
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#11047 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AGGmv8iURa6gHNIbi3SQpzLkut7fpGjUks5sYeZOgaJpZM4O21Qp>
.
|
|
@jonasschnelli folders is possible. This is just a concept at the moment. @ryanofsky also prefers folders. I'm happy to take feedback on what scheme people prefer. |
I'd dread the resulting python mess when using folders as it conflicts with modules. |
|
I also prefer the prefix style over the folders |
|
For now, I think prefix solves the problem of grouping and provides some extra clarity. |
deda19e to
7c8bea0
Compare
|
Updated with more consistent naming. |
7c8bea0 to
4d6ad55
Compare
4d6ad55 to
a2c0115
Compare
|
reimplemented as a scripted-diff, since this is going to need rebase for every change in |
c682cfd to
e0b1b9e
Compare
23cc14c to
c90f4a6
Compare
|
Can you remove the scripted diff tags? It takes longer to review the script than to look at the git diff. Though, feel free to keep the script in the commit body if that is helpful to you. You might want to ask if anyone objects those changes on Thursday. If not, we can merge on Friday. |
This avoids importing from segwit.py to bumpfee.py
Not implemented as a scripted-diff since the script is more difficult to review than the diff. The script used was: for i in \ disablewallet.py,wallet_disable.py \ wallet-hd.py,wallet_hd.py \ walletbackup.py,wallet_backup.py \ p2p-fullblocktest.py,p2p_block.py \ fundrawtransaction.py,rpc_fundrawtransaction.py \ p2p-compactblocks.py,p2p_compactblocks.py \ segwit.py,p2p_segwit_activation.py \ multiwallet.py,wallet_multiwallet.py \ wallet.py,wallet_basic.py \ wallet-accounts.py,wallet_accounts.py \ p2p-segwit.py,feature_segwit.py \ wallet-dump.py,wallet_dump.py \ listtransactions.py,rpc_listtransactions.py \ sendheaders.py,p2p_sendheaders.py \ zapwallettxes.py,wallet_zapwallettxes.py \ importmulti.py,wallet_importmulti.py \ merkle_blocks.py,rpc_txoutproof.py \ receivedby.py,wallet_listreceivedby.py \ abandonconflict.py,wallet_abandonconflict.py \ bip68-112-113-p2p.py,p2p_csv_activation.py \ reindex.py,feature_reindex.py \ keypool-topup.py,wallet_keypool_topup.py \ zmq_test.py,interface_zmq.py \ bitcoin_cli.py,interface_bitcoin_cli.py \ mempool_resurrect_test.py,mempool_resurrect.py \ txn_clone.py,wallet_txn_clone.py \ getchaintips.py,rpc_getchaintips.py \ rest.py,interface_rest.py \ mempool_spendcoinbase.py,mempool_spend_coinbase.py \ httpbasics.py,interface_http.py \ multi_rpc.py,rpc_users.py \ proxy_test.py,feature_proxy.py \ signrawtransactions.py,rpc_signrawtransaction.py \ rawtransactions.py,rpc_rawtransaction.py \ disconnect_ban.py,p2p_disconnect_ban.py \ decodescript.py,rpc_decodescript.py \ blockchain.py,rpc_blockchain.py \ net.py,rpc_net.py \ keypool.py,wallet_keypool.py \ p2p-mempool.py,p2p_mempool.py \ prioritise_transaction.py,mining_prioritisetransaction.py \ invalidblockrequest.py,p2p_invalid_block.py \ invalidtxrequest.py,p2p_invalid_tx.py \ p2p-versionbits-warning.py,feature_versionbits_warning.py \ preciousblock.py,rpc_preciousblock.py \ importprunedfunds.py,wallet_importprunedfunds.py \ signmessages.py,rpc_signmessage.py \ nulldummy.py,feature_nulldummy.py \ import-rescan.py,wallet_import_rescan.py \ mining.py,mining_basic.py \ bumpfee.py,wallet_bumpfee.py \ rpcnamedargs.py,rpc_named_arguments.py \ listsinceblock.py,wallet_listsinceblock.py \ p2p-leaktests.py,p2p_leak.py \ wallet-encryption.py,wallet_encryption.py \ bipdersig-p2p.py,feature_dersig.py \ bip65-cltv-p2p.py,feature_cltv.py \ uptime.py,rpc_uptime.py \ resendwallettransactions.py,wallet_resendwallettransactions.py \ pruning.py,feature_pruning.py \ smartfees.py,feature_fee_estimation.py \ maxuploadtarget.py,feature_maxuploadtarget.py \ dbcrash.py,feature_dbcrash.py \ bip68-sequence.py,feature_csv.py \ getblocktemplate_longpoll.py,mining_getblocktemplate_longpoll.py \ p2p-timeouts.py,p2p_timeouts.py \ bip9-softforks.py,feature_bip9_softforks.py \ p2p-feefilter.py,p2p_feefilter.py \ rpcbind_test.py,rpc_bind.py \ assumevalid.py,feature_assumevalid.py \ txn_doublespend.py,wallet_txn_doublespend.py \ forknotify.py,feature_forknotify.py \ invalidateblock.py,rpc_invalidateblock.py \ p2p-acceptblock.py,p2p_unrequested_blocks.py \ replace-by-fee.py,feature_rbf.py \ minchainwork.py,feature_minchainwork.py \ deprecated_rpc.py,rpc_deprecated.py; do IFS=","; set -- $i; mv test/functional/$1 test/functional/$2; sed -i -e "s/'$1/'$2/g" test/functional/test_runner.py test/functional/README.md; done
c90f4a6 to
8ad38c6
Compare
|
Rebased with the final commit not implemented as a scripted-diff. |
| - `wallet` for tests for wallet features, eg `wallet_keypool.py` | ||
| - use an underscore to separate words | ||
| - exception: for tests for specific RPCs or command line options which don't include underscores, name the test after the exact RPC or argument name, eg `rpc_decodescript.py`, not `rpc_decode_script.py` | ||
| - Don't use the redundant work `test` in the name, eg `interface_zmq.py`, not `interface_zmq_test.py` |
There was a problem hiding this comment.
Nit: "redundant word" not work
| # vv Tests less than 2m vv | ||
| 'wallet.py', | ||
| 'wallet-accounts.py', | ||
| 'p2p-segwit.py', |
There was a problem hiding this comment.
This seems backwards: segwit.py becomes p2p_segwit_activation, but p2p-segwit.py becomes feature_segwit?
| 'rpc_txoutproof.py', | ||
| 'wallet_listreceivedby.py', | ||
| 'wallet_abandonconflict.py', | ||
| 'p2p_csv_activation.py', |
There was a problem hiding this comment.
This doesn't seem to test p2p protocol aspects, so maybe should be feature_csv_activation? (Or feature_bip68_112_113_activation?)
| 'fundrawtransaction.py', | ||
| 'p2p-compactblocks.py', | ||
| 'segwit.py', | ||
| 'p2p_block.py', |
There was a problem hiding this comment.
p2p_block tests block acceptance and reorg behaviour rather than the p2p protocol so should probably be in the feature_ namespace
|
ACK with a few nits (see other comments). |
|
Will be closing in a couple of weeks, if this sits still. |
This may be a non-starter because of the churn caused, but I thought I'd put it out there and see what people think. Not that git is usually smart enough to figure out filename only changes, so rebasing shouldn't be an issue.
This PR changes the functional tests to have a consistent naming scheme:
rpc_...interface_...p2p_...wallet_...mining_...mempool_...feature_...Rationale: it's sometimes difficult for new contributors to know what's already covered by existing tests and where new tests should be added. Naming in a consistent fashion makes it easier to see what's already covered at a glance.