test: Remove `system_tests/run_command` runtime dependencies

hebasto commented at 11:03 pm on November 23, 2025: member

system_tests currently rely on cat, echo, false and sh being available in PATH at runtime.

This PR:

Removes these dependencies.
Reduces the number of platform-specific code paths.

The change is primarily motivated by my work on maintaining the bitcoin-core package in Guix. It enables the removal of the existing bash and coreutils native inputs, which in turn makes it possible to drop the implicit dependency on qtbase@5 (see https://codeberg.org/guix/guix/pulls/4386#issuecomment-8613333).

Based on #34349.

hebasto added the label Tests on Nov 23, 2025

DrahtBot commented at 11:03 pm on November 23, 2025: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33929.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	bensig
Stale ACK	maflcko

If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

Conflicts

No conflicts as of last run.

in src/test/system_tests.cpp:26 in c996515a6d

21 
22 #ifdef ENABLE_EXTERNAL_SIGNER
23 
24 BOOST_AUTO_TEST_CASE(run_command)
25 {
26+    const std::string test_executable = std::string(boost::unit_test::framework::master_test_suite().argv[0]).append(" --log_level=nothing --report_level=no --run_test=mock_process/");

maflcko commented at 8:45 pm on December 4, 2025:

nit: This will fail with spaces in the dir:

0$ ./a\ space/test_bitcoin -t system_tests
1Running 1 test case...
2unknown location(0): fatal error: in "system_tests/run_command": subprocess::CalledProcessError: execve failed: No such file or directory (2)
3test/system_tests.cpp(29): last checkpoint
4
5*** 1 failure is detected in the test module "Bitcoin Core Test Suite"

sedited commented at 8:39 pm on January 19, 2026:

Looks like this is broken by subprocess’s use of split here: https://github.com/bitcoin/bitcoin/blob/master/src/util/subprocess.h#L941 . This makes me wonder if we should get rid of the single command constructor. Seems a bit like a footgun?

maflcko commented at 7:19 am on January 20, 2026:

Fixed in #34349, due to lack of progress here.

hebasto commented at 7:10 pm on January 21, 2026:

Thanks! Rebased on top of #34349.

maflcko approved

maflcko commented at 8:45 pm on December 4, 2025: member

would be nice to fix the space error, but not sure how easy that is.

review ACK c996515a6dcb158d2d3aa88efc499ffff49bea79 🚆

Signature:

0untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
2trusted comment: review ACK c996515a6dcb158d2d3aa88efc499ffff49bea79 🚆
3tIPkSDEjaEB97BQIv8sM9m414fm5sxxOHHqLWNWGBf6fsHuOwabixwP9VIGmJHyf+ljEBjgD2gIKL2G+psZMAA==

bensig commented at 9:05 pm on January 5, 2026: contributor

Concept ACK c996515

Nice approach using the test binary itself as a mock process.

nit: This will fail if the path contains spaces:

0$ "./a space/test_bitcoin" --run_test=system_tests
1  unknown location(0): fatal error: in "system_tests/run_command": subprocess::CalledProcessError: execve failed: No such file or directory (2)

The executable path in test_executable probably needs quoting.

maflcko commented at 7:25 am on January 6, 2026: member

nit: This will fail if the path contains spaces: @bensig I’ve seen you minimally reword a prior review (here and in other pull requests), which makes you look like an LLM bot. Review is needed and welcome, but it does not help to simply repeat prior comments, such as the motivation from the pull request description, or from other reviews. It would be better to only mention new findings or new points of view.

bensig commented at 6:41 pm on January 6, 2026: contributor

nit: This will fail if the path contains spaces:

@bensig I’ve seen you minimally reword a prior review (here and in other pull requests), which makes you look like an LLM bot. Review is needed and welcome, but it does not help to simply repeat prior comments, such as the motivation from the pull request description, or from other reviews. It would be better to only mention new findings or new points of view.

You got it… I am still learning the ropes here. I did 21+ reviews this week so far… I was simply reiterating something that I verified.. I will minimize my ACKs to only include any verifications that I have done or new information. Thanks for the feedback.

l0rinc commented at 7:41 pm on January 6, 2026: contributor

We’re usually more interested to know that you have verified the change yourself (e.g. recreated the change locally, ended up with same changes; ran all tests/benchmarks locally; searches for similar PRs or occurrences where the change applies, ran the fuzzer for 36 hours; did an IBD against this change; tested it on a big-endian/32-bit/raspberry-pi/windows, etc), i.e. that there’s PoW behind the ack.

util: Remove brittle and confusing sp::Popen(std::string) fa95f47509

test: Stricter unit test

Now that the previous commit fixed a unit test bug, make the test
stricter, to prevent this issue from happening again in the future.

fac2b1eea6

test: Remove `system_tests/run_command` runtime dependencies 22399ff8eb

test: Enable `system_tests/run_command` "stdin" test on Windows 683acc9977

hebasto force-pushed on Jan 21, 2026

hebasto commented at 7:09 pm on January 21, 2026: member

Rebased on top of #34349, which resolves #33929 (review).

hebasto marked this as a draft on Jan 21, 2026

test: Remove system_tests/run_command runtime dependencies #33929

Code Coverage & Benchmarks

Reviews

Conflicts

test: Remove `system_tests/run_command` runtime dependencies #33929