test: add logging to mock external signers #32928
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32928. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
Sjors
force-pushed
the
2025/07/test-signer
branch
from
4f8075b to
3ff8e66
Compare
Sjors
changed the title
|
Adding mock_signer_log and logging “Started” / “Finished” makes debugging much clearer and great improvement, especially since stdout/stderr can’t be used here. |
naiyoma
left a comment
naiyoma
left a comment
There was a problem hiding this comment.
Concept ACK ->
I attempted to debug -> #32855 (comment) after pulling this branch, and so far I can see some useful logs in combined.log
mock_external_signer” logs from this diff
grep "mock_external_signer" combined.log
node1 2025-07-19T19:35:11.455260Z [init] [common/args.cpp:850] [logArgsPrefix] Command-line arg: signer="/usr/bin/python3 /home/ubuntu/Projects/bitcoin/test/functional/test_framework/../mock_external_signer.py"
test 2025-07-19T19:35:11.823136Z TestFramework (DEBUG): -signer=/usr/bin/python3 /home/ubuntu/Projects/bitcoin/test/functional/test_framework/../mock_external_signer.py
node1 2025-07-19T19:35:11.846121Z [httpworker.1] [common/run_command.cpp:29] [RunCommandParseJSON] RunCommandParseJSON: command='/usr/bin/python3 /home/ubuntu/Projects/bitcoin/test/functional/test_framework/../mock_external_signer.py enumerate', stdin_length=0, stdin_empty=true
test 2025-07-19T19:35:11.963244Z TestFramework.mock_external_signer (DEBUG): Started
test 2025-07-19T19:35:11.969415Z TestFramework.mock_external_signer (DEBUG): Finished
node1 2025-07-19T19:35:11.986061Z [httpworker.1] [common/run_command.cpp:29] [RunCommandParseJSON] RunCommandParseJSON: command='/usr/bin/python3 /home/ubuntu/Projects/bitcoin/test/functional/test_framework/../mock_external_signer.py --fingerprint 00000001 --chain regtest getdescriptors --account 0', stdin_length=0, stdin_empty=true
test 2025-07-19T19:35:12.078454Z TestFramework.mock_external_signer (DEBUG): Started
test 2025-07-19T19:35:12.084684Z TestFramework.mock_external_signer (DEBUG): Naiyoma HANGING: stdin hang for getdescriptors
Sjors
force-pushed
the
2025/07/test-signer
branch
from
3ff8e66 to
b1edd6d
Compare
For reference, the issue was something else, and has been fixed now, in the meantime. |
|
@maflcko updated the PR description. I assume more logging is still useful though. |
Sjors
force-pushed
the
2025/07/test-signer
branch
from
b1edd6d to
038a9eb
Compare
- reuse perform_pre_checks helper - add mock_psbt_path helper
Because stdout and stderr are consumed by the node, we can't use them for logging. Instead have the mocks print directly into test_framework.log, which can then be be retrieved via combine_logs.py.
Sjors
force-pushed
the
2025/07/test-signer
branch
from
038a9eb to
be09b43
Compare
|
🐙 This pull request conflicts with the target branch and needs rebase. |
|
Closing as up for grabs (rather than rebasing again) due to lack of review. |
5 participants
Because
stdoutandstderrare consumed by the node, the mock external signers can't use them for logging.Instead have them print directly into
test_framework.log, which can then be be retrieved viacombine_logs.py.In preparation (and addtion) this PR starts with a few refactors:
mock_signer_pathhelpers toutiland reuse them betweenrpc_signer.pyandwallet_signer.pytest/functional/mockstotest/functional: this is needed to avoid Python import hell in the next commitstest_frameworkby extracting (and reusing)perform_pre_checksandmock_signer_psbt_pathintoutildebug.logstyleFormatterintoutilso we can use it in the final commitOriginally this PR was intended to aid in debugging #32855, which has been solved since.