test: add rpc_fundrawtransaction logging #17327

jonatack · 2019-10-31T10:57:37Z

test/functional/rpc_fundrawtransaction.py is fairly slow to run and has no logging, so it can appear to be stalled.

This commit adds info logging at each test to provide feedback on the test run.

jonatack · 2019-10-31T10:59:31Z

Motivation: Watching @achow101 swear repeatedly at this test yesterday :)

Test output with this commit:

$ test/functional/rpc_fundrawtransaction.py 
2019-10-31T10:54:24.851000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_qrx99bxz
2019-10-31T10:54:25.790000Z TestFramework (INFO): Setup, generating 126 nodes and syncing...
2019-10-31T10:54:28.704000Z TestFramework (INFO): Testing fundrawtxn changePosition option
2019-10-31T10:54:29.393000Z TestFramework (INFO): Testing fundrawtxn
2019-10-31T10:54:29.442000Z TestFramework (INFO): Testing fundrawtxn with 2 coins
2019-10-31T10:54:29.559000Z TestFramework (INFO): Testing fundrawtxn with 2 outputs
2019-10-31T10:54:29.670000Z TestFramework (INFO): Testing fundrawtxn with a vin > required amount
2019-10-31T10:54:29.743000Z TestFramework (INFO): Testing fundrawtxn not having a change output
2019-10-31T10:54:29.821000Z TestFramework (INFO): Testing fundrawtxn with an invalid option
2019-10-31T10:54:29.859000Z TestFramework (INFO): Testing fundrawtxn with an invalid change address
2019-10-31T10:54:29.901000Z TestFramework (INFO): Testing fundrawtxn with a provided change address
2019-10-31T10:54:29.971000Z TestFramework (INFO): Testing fundrawtxn with a provided change type
2019-10-31T10:54:30.043000Z TestFramework (INFO): Testing fundrawtxn with a vin < required amount
2019-10-31T10:54:30.124000Z TestFramework (INFO): Testing fundrawtxn with 2 vins
2019-10-31T10:54:30.202000Z TestFramework (INFO): Testing fundrawtxn with 2 vins and 2 vouts
2019-10-31T10:54:30.353000Z TestFramework (INFO): Testing fundrawtxn with an invalid vin
2019-10-31T10:54:30.412000Z TestFramework (INFO): Testing fundrawtxn p2pkh fee
2019-10-31T10:54:30.605000Z TestFramework (INFO): Testing fundrawtxn p2pkh fee with multiple outputs
2019-10-31T10:54:31.091000Z TestFramework (INFO): Testing fundrawtxn fee with 4of5 addresses
2019-10-31T10:54:31.313000Z TestFramework (INFO): Testing fundrawtxn spending 2of2 multisig...
2019-10-31T10:54:41.876000Z TestFramework (INFO): Testing fundrawtxn with locked wallet...
2019-10-31T10:54:45.003000Z TestFramework (INFO): Testing fundrawtxn fee with many inputs...
2019-10-31T10:54:54.834000Z TestFramework (INFO): Testing fundrawtxn sign+send with many inputs...
2019-10-31T10:55:18.605000Z TestFramework (INFO): Testing fundrawtxn with OP_RETURN and no vin
2019-10-31T10:55:18.635000Z TestFramework (INFO): Testing fundrawtxn using only watchonly
2019-10-31T10:55:18.785000Z TestFramework (INFO): Testing fundrawtxn using entirety of watched funds
2019-10-31T10:55:18.941000Z TestFramework (INFO): Testing fundrawtxn feeRate option
2019-10-31T10:55:19.390000Z TestFramework (INFO): Testing fundrawtxn no address reuse occurs
2019-10-31T10:55:19.551000Z TestFramework (INFO): Testing fundrawtxn subtractFeeFromOutputs option
2019-10-31T10:55:20.174000Z TestFramework (INFO): Stopping nodes
2019-10-31T10:55:20.429000Z TestFramework (INFO): Cleaning up /tmp/bitcoin_func_test_qrx99bxz on exit
2019-10-31T10:55:20.429000Z TestFramework (INFO): Tests successful

Slower-running tests are those suffixed with ...

instagibbs · 2019-10-31T15:04:56Z

since you're touching so many lines, maybe just take the ASCII art headers and turn the actual text into the print line? sorry 😬

instagibbs · 2019-10-31T15:05:19Z

test/functional/rpc_fundrawtransaction.py

f.e. just convert this line into a print statement

Jup, can remove this now

Done, ASCII art blocks removed.

jnewbery

Thanks for doing this. A few personal style nits:

I think the indicative would sound better in these test logs (eg "test x" rather than "testing x")
agree with the other reviewers that the logs can replace the comment blocks
I'd prefer to remove the trailing '...' ellipses. It's not standard that they mean "this takes a long time"

jnewbery · 2019-10-31T16:10:30Z

test/functional/rpc_fundrawtransaction.py

Do you mean 126 blocks? Also, I think there are only 122. Perhaps best to just omit the number?

Thanks @jnewbery! 4 nodes and 122 blocks. Replacing with "Connect nodes, set fees, generate blocks, and sync"

jnewbery · 2019-10-31T16:14:22Z

test/functional/rpc_fundrawtransaction.py

nit: prefer 2-of-2

Done, did the same for "4of5"

jnewbery · 2019-10-31T16:15:49Z

test/functional/rpc_fundrawtransaction.py

this sentence sounds a bit clunky. Perhaps "Test no address reuse occurs with fundrawransaction"

Agree, replacing with "Test fundrawtxn does not reuse addresses" to maintain consistent verb + op noun logging sentences.

test/functional/rpc_fundrawtransaction.py is fairly long to run and has no logging, so it can appear to be stalled. This commit adds info logging at each test to provide feedback on the test run.

laanwj · 2019-11-01T10:56:34Z

Concept ACK, agree with @jnewbery's nits.

Doc changes only to test/functional/rpc_fundrawtransaction.py: - remove ascii art or convert to a docstring when sufficiently different from the logging - touch up other comments while here

jonatack

Thank you @instagibbs, @MarcoFalke, and @jnewbery for reviewing, updated with your suggestions.

jonatack · 2019-11-01T10:14:18Z

test/functional/rpc_fundrawtransaction.py

Thanks @jnewbery! 4 nodes and 122 blocks. Replacing with "Connect nodes, set fees, generate blocks, and sync"

jonatack · 2019-11-01T10:14:47Z

test/functional/rpc_fundrawtransaction.py

Done, ASCII art blocks removed.

jonatack · 2019-11-01T10:15:41Z

test/functional/rpc_fundrawtransaction.py

Done, did the same for "4of5"

jonatack · 2019-11-01T10:17:04Z

test/functional/rpc_fundrawtransaction.py

Agree, replacing with "Test fundrawtxn does not reuse addresses" to maintain consistent verb + op noun logging sentences.

jonatack · 2019-11-01T11:00:31Z

Test output after updates:

$ test/functional/rpc_fundrawtransaction.py 
2019-11-01T10:58:43.324000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_vh0ej7_v
2019-11-01T10:58:44.504000Z TestFramework (INFO): Connect nodes, set fees, generate blocks, and sync
2019-11-01T10:58:46.250000Z TestFramework (INFO): Test fundrawtxn changePosition option
2019-11-01T10:58:47.847000Z TestFramework (INFO): Test fundrawtxn
2019-11-01T10:58:47.950000Z TestFramework (INFO): Test fundrawtxn with 2 coins
2019-11-01T10:58:48.025000Z TestFramework (INFO): Test fundrawtxn with 2 outputs
2019-11-01T10:58:48.093000Z TestFramework (INFO): Test fundrawtxn with a vin > required amount
2019-11-01T10:58:48.150000Z TestFramework (INFO): Test fundrawtxn not having a change output
2019-11-01T10:58:48.209000Z TestFramework (INFO): Test fundrawtxn with an invalid option
2019-11-01T10:58:48.241000Z TestFramework (INFO): Test fundrawtxn with an invalid change address
2019-11-01T10:58:48.274000Z TestFramework (INFO): Test fundrawtxn with a provided change address
2019-11-01T10:58:48.331000Z TestFramework (INFO): Test fundrawtxn with a provided change type
2019-11-01T10:58:48.382000Z TestFramework (INFO): Test fundrawtxn with a vin < required amount
2019-11-01T10:58:48.493000Z TestFramework (INFO): Test fundrawtxn with 2 vins
2019-11-01T10:58:48.645000Z TestFramework (INFO): Test fundrawtxn with 2 vins and 2 vouts
2019-11-01T10:58:48.804000Z TestFramework (INFO): Test fundrawtxn with an invalid vin
2019-11-01T10:58:48.889000Z TestFramework (INFO): Test fundrawtxn p2pkh fee
2019-11-01T10:58:49.005000Z TestFramework (INFO): Test fundrawtxn p2pkh fee with multiple outputs
2019-11-01T10:58:49.540000Z TestFramework (INFO): Test fundrawtxn fee with 4-of-5 addresses
2019-11-01T10:58:49.775000Z TestFramework (INFO): Test fundrawtxn spending 2-of-2 multisig
2019-11-01T10:59:03.159000Z TestFramework (INFO): Test fundrawtxn with locked wallet
2019-11-01T10:59:06.770000Z TestFramework (INFO): Test fundrawtxn fee with many inputs
2019-11-01T10:59:20.289000Z TestFramework (INFO): Test fundrawtxn sign+send with many inputs
2019-11-01T10:59:38.379000Z TestFramework (INFO): Test fundrawtxn with OP_RETURN and no vin
2019-11-01T10:59:38.415000Z TestFramework (INFO): Test fundrawtxn using only watchonly
2019-11-01T10:59:38.614000Z TestFramework (INFO): Test fundrawtxn using entirety of watched funds
2019-11-01T10:59:38.769000Z TestFramework (INFO): Test fundrawtxn feeRate option
2019-11-01T10:59:39.215000Z TestFramework (INFO): Test fundrawtxn does not reuse addresses
2019-11-01T10:59:39.382000Z TestFramework (INFO): Test fundrawtxn subtractFeeFromOutputs option
2019-11-01T10:59:40.115000Z TestFramework (INFO): Stopping nodes
2019-11-01T10:59:40.385000Z TestFramework (INFO): Cleaning up /tmp/bitcoin_func_test_vh0ej7_v on exit
2019-11-01T10:59:40.385000Z TestFramework (INFO): Tests successful

instagibbs · 2019-11-01T13:48:26Z

utACK ff22751

jnewbery · 2019-11-01T14:20:36Z

tACK ff22751

ff22751 test: rm ascii art in rpc_fundrawtransaction (Jon Atack) 94fcc08 test: add rpc_fundrawtransaction logging (Jon Atack) Pull request description: `test/functional/rpc_fundrawtransaction.py` is fairly slow to run and has no logging, so it can appear to be stalled. This commit adds info logging at each test to provide feedback on the test run. ACKs for top commit: instagibbs: utACK bitcoin@ff22751 jnewbery: tACK ff22751 Tree-SHA512: f4fabad8ef51c29981351bb4e66fb0c0e0517418a4a15892ef804df11d16b2d2ae1a1abc958d2b121819850278de90a2003b0edb8d7098d00360b89fa76e9062

fanquake added the Tests label Oct 31, 2019

instagibbs reviewed Oct 31, 2019

View reviewed changes

jnewbery reviewed Oct 31, 2019

View reviewed changes

test: add rpc_fundrawtransaction logging

94fcc08

test/functional/rpc_fundrawtransaction.py is fairly long to run and has no logging, so it can appear to be stalled. This commit adds info logging at each test to provide feedback on the test run.

test: rm ascii art in rpc_fundrawtransaction

ff22751

Doc changes only to test/functional/rpc_fundrawtransaction.py: - remove ascii art or convert to a docstring when sufficiently different from the logging - touch up other comments while here

jonatack force-pushed the rpc_fundrawtransaction-test-logging branch from 1760e74 to ff22751 Compare November 1, 2019 10:57

jonatack commented Nov 1, 2019

View reviewed changes

jnewbery mentioned this pull request Nov 1, 2019

Tests: speed up fundrawtransaction test #17340

Merged

maflcko merged commit ff22751 into bitcoin:master Nov 1, 2019

jonatack deleted the rpc_fundrawtransaction-test-logging branch November 1, 2019 15:17

jonatack mentioned this pull request Mar 9, 2020

test: Explain why test logging should be used #18305

Merged

MarkLTZ mentioned this pull request Apr 4, 2020

Bitcoin PR tracking litecoinz-core/litecoinz#14

Closed

bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021

test: add rpc_fundrawtransaction logging #17327

test: add rpc_fundrawtransaction logging #17327

Uh oh!

Conversation

jonatack commented Oct 31, 2019

Uh oh!

jonatack commented Oct 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

instagibbs commented Oct 31, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jnewbery left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laanwj commented Nov 1, 2019

Uh oh!

jonatack left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonatack commented Nov 1, 2019

Uh oh!

instagibbs commented Nov 1, 2019

Uh oh!

jnewbery commented Nov 1, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jonatack commented Oct 31, 2019 •

edited

Loading