Skip to content

test: accept multiple execve error strings in rpc_signer#34590

Closed
SakshiKasat18 wants to merge 1 commit intobitcoin:masterfrom
SakshiKasat18:test/rpc-signer-execve-errno
Closed

test: accept multiple execve error strings in rpc_signer#34590
SakshiKasat18 wants to merge 1 commit intobitcoin:masterfrom
SakshiKasat18:test/rpc-signer-execve-errno

Conversation

@SakshiKasat18
Copy link

Summary

Relax the rpc_signer.py test to accept multiple POSIX execve failure messages when the signer command is missing.

Motivation

The exact errno text for execve failures can vary across environments (e.g. "No such file or directory" vs "Not a directory", and may include errno numbers). Matching a single exact string makes the test brittle.
Keep the check strict (matching the "execve failed:" prefix) while allowing expected POSIX variants.

Refs #31506.

How tested:
export DYLD_LIBRARY_PATH="..."
python3 build/test/functional/test_runner.py rpc_signer.py

@DrahtBot DrahtBot added the Tests label Feb 14, 2026
@DrahtBot
Copy link
Contributor

DrahtBot commented Feb 14, 2026

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

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first commit cbada90 is empty. Please remove it.

@SakshiKasat18 SakshiKasat18 force-pushed the test/rpc-signer-execve-errno branch from fd38a27 to 6610445 Compare February 14, 2026 11:59
@SakshiKasat18
Copy link
Author

The first commit cbada90 is empty. Please remove it.

Removed the empty cbada90 commit via rebase/force-push. The PR now contains only the test change.

@maflcko
Copy link
Member

maflcko commented Feb 16, 2026

Was this LLM generated? What are the steps to test this? What is the output before and after the changes here?

@SakshiKasat18
Copy link
Author

@maflcko
No, this was not LLM generated. I wrote the change while debugging a failing functional test on my system and investigating why the error string did not match exactly.

How to test:
1. Build Bitcoin Core as usual.
2. Run the functional test:
python3 build/test/functional/test_runner.py rpc_signer.py

Behavior before this change:
The test expected the exact error message:
execve failed: No such file or directory

However, on some systems the actual RPC error message differs slightly. For example, it may include the errno number:
execve failed: No such file or directory (2)

or return a different but valid POSIX message such as:
execve failed: Not a directory

In these cases, the RPC call correctly failed, but the test itself failed with an assertion error because it was matching a single exact string.

Behavior after this change:
The test still verifies that:
• the RPC call fails
• the message starts with "execve failed:"

But instead of requiring one exact string, it now allows the expected POSIX variants such as "No such file or directory" and "Not a directory".

This keeps the check strict while making the test portable across environments.

@maflcko
Copy link
Member

maflcko commented Feb 16, 2026

Well, if this wasn't LLM generated, you could share exact steps to reproduce (copy-paste), ideally starting form a fresh install of the Distro. Also, you could share the exact output before and after.

So far you haven't provided neither, which I guess means that you haven't tested/confimred this at all locally?

@maflcko maflcko closed this Feb 16, 2026
@SakshiKasat18
Copy link
Author

Hi @maflcko , thanks for the feedback.
You’re right that I should have included exact reproduction steps and the full before/after output. I did test this locally, but I didn’t document it properly in the PR.
I’ll prepare a new PR with copy-paste steps starting from a clean upstream master, along with the exact failing output before the change and the passing output after.
Appreciate the review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants