test: convert documentation into type annotations #22092

pull fanquake wants to merge 3 commits into bitcoin:master from fanquake:more_mypy_test_utils changing 3 files +16 −14

fanquake commented at 6:09 AM on May 28, 2021: member

Rather than having function types exist as documentation, make them type annotations, which enables more mypy checking.
fanquake added the label Tests on May 28, 2021
DrahtBot commented at 3:18 PM on May 28, 2021: 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:
- #22089 (test: MiniWallet: fix fee calculation for P2PK and check tx vsize by theStack)
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.
practicalswift commented at 3:28 PM on May 28, 2021: contributor

Concept ACK

Automatic checking is better than human checking. Empirical evidence suggests that humans are slow, inaccurate, and bore easily.
MarcoFalke commented at 6:48 PM on May 31, 2021: member

ci fails
fanquake commented at 3:07 AM on June 1, 2021: member

ci fails

Trying to determine what the failure is here. Looking at https://cirrus-ci.com/task/6475309332561920 it seems the failure happens after printing the uncovered commands, but before it gets to "Cleaning up coverage data".

I can't seem to reproduce when running this branch & test/functional/test_runner.py --coverage locally on a Linux or macOS machine; or when running with what should be the equivalent options from 00_setup_env_native_qt5.sh.

MarcoFalke commented at 6:19 AM on June 1, 2021: member

I can reproduce locally with the ci system

ALL                                                | ✓ Passed  | 4979 s (accumulated) 
Runtime: 719 s

Uncovered RPC commands:
  - addnode
  - analyzepsbt
  - clearbanned
  - combinepsbt
  - combinerawtransaction
  - converttopsbt
  - createmultisig
  - createpsbt
  - createrawtransaction
  - decodepsbt
  - decoderawtransaction
  - decodescript
  - deriveaddresses
  - disconnectnode
  - enumeratesigners
  - estimatesmartfee
  - finalizepsbt
  - generateblock
  - generatetoaddress
  - generatetodescriptor
  - getaddednodeinfo
  - getbestblockhash
  - getblock
  - getblockchaininfo
  - getblockcount
  - getblockfilter
  - getblockhash
  - getblockheader
  - getblockstats
  - getblocktemplate
  - getchaintips
  - getchaintxstats
  - getconnectioncount
  - getdescriptorinfo
  - getdifficulty
  - getindexinfo
  - getmemoryinfo
  - getmempoolancestors
  - getmempooldescendants
  - getmempoolentry
  - getmempoolinfo
  - getmininginfo
  - getnettotals
  - getnetworkhashps
  - getnetworkinfo
  - getnodeaddresses
  - getpeerinfo
  - getrawmempool
  - getrawtransaction
  - getrpcinfo
  - gettxout
  - gettxoutproof
  - gettxoutsetinfo
  - getzmqnotifications
  - help
  - joinpsbts
  - listbanned
  - logging
  - ping
  - preciousblock
  - prioritisetransaction
  - pruneblockchain
  - savemempool
  - scantxoutset
  - sendrawtransaction
  - setban
  - setnetworkactive
  - signmessagewithprivkey
  - signrawtransactionwithkey
  - stop
  - submitblock
  - submitheader
  - testmempoolaccept
  - uptime
  - utxoupdatepsbt
  - validateaddress
  - verifychain
  - verifymessage
  - verifytxoutproof

# echo $?
1

laanwj commented at 12:57 PM on June 2, 2021: member

Concept ACK, I like the mypy-style for documentation better anyhow.

I am unable to reproduce the functional test failure here.
MarcoFalke commented at 7:24 AM on June 3, 2021: member

Maybe you can split up the failing commit into a separate pull?
test: convert docs into type annotations in test_framework/util.py b043ca8e8b
test: convert docs into type annotations in test_framework/wallet.py 8bfcba36db
test: convert docs into type annotations in test_framework/test_node.py 68ace23fa3
fanquake force-pushed on Jun 3, 2021
fanquake commented at 12:45 PM on June 3, 2021: member

Maybe you can split up the failing commit into a separate pull?

Ok. Split into #22139.
kiminuo commented at 4:25 PM on June 4, 2021: contributor

Concept ACK
theStack commented at 5:40 PM on June 5, 2021: member

Concept ACK
instagibbs commented at 3:54 AM on June 7, 2021: member

utACK https://github.com/bitcoin/bitcoin/pull/22092/commits/68ace23fa3bc01baa734ddf2c0963acae1c75ed1

annotations match prior comments
fanquake merged this on Jun 7, 2021
fanquake closed this on Jun 7, 2021
sidhujag referenced this in commit 1d86bbe26b on Jun 9, 2021
fanquake deleted the branch on Jul 27, 2021
MarcoFalke referenced this in commit 24fb69dca4 on Jul 29, 2021
sidhujag referenced this in commit 9ecc128c62 on Jul 29, 2021
Fabcien referenced this in commit e05a29e0ae on Feb 2, 2022
gwillen referenced this in commit f806c13661 on Jun 1, 2022
DrahtBot locked this on Aug 18, 2022

Contributors

Labels

Linked (view graph)

#22139 test: add type annotations to util.get_rpc_proxy

test: convert documentation into type annotations #22092

Conflicts