1. 
   marshallshen commented at 7:55 pm on July 20, 2025: none

   Summary

   This PR adds a new unit test test_error_messages to verify that all PSBTError enum values return the correct error messages from the PSBTErrorString() function in messages.cpp.

   Motivation

   • Ensures complete test coverage of the PSBTErrorString() function in src/common/messages.cpp
   • Provides regression protection against accidental changes to user-facing error messages
   • Documents the expected error messages for all PSBT error cases

   Changes

   • Added: test_error_messages() unit test in src/wallet/test/psbt_wallet_tests.cpp
   • Added: Required includes for <common/messages.h> and <common/types.h>
   • Tests: All 7 PSBTError enum values with their expected error message strings

   Testing

   0# Run the specific test
   1./build/bin/test_bitcoin --run_test=psbt_wallet_tests
   

   More Context

   mentioned in: #31622 #31622 (review)

2. test: add unit test for psbt_wallet error messages efc58a3d45
3. DrahtBot added the label Tests on Jul 20, 2025
4. 
   DrahtBot commented at 7:55 pm on July 20, 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/33025.

   Reviews

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

5. 
   fjahr commented at 8:06 pm on July 20, 2025: contributor

   Unfortunately this is not what I had in mind when I wrote my comment. What I meant was that there should be a test that triggers the uncovered error message through wallet usage. Can you give this a try instead?

   Direct coverage of the PSBTErrorString function isn’t very valuable IMO because it is just a wrapper of a switch statement which can be tested implicitly with tests such as I mention above. Test that are this closely tied to implementations are usually more hassle than they are worth in terms of the value they add because they tend to need changes often but don’t cover any meaningful logic.

6. 
   marshallshen commented at 8:16 pm on July 20, 2025: none

   his is not what I had in mind when I wrote my comment. What I meant was that there should be a test that triggers the uncovered error message through wallet usage. Can you give this a try instead?

   Gotcha, so it’s more of a functional test for the behavior that triggers PSBTError::INCOMPLETE, Not a unit test to cover PSBTErrorString.

   And I agree that testing case statement isn’t really super valuable. I will give it another try, thanks!

7. 
   maflcko commented at 6:29 am on July 21, 2025: member

   Closing for now. You can open a new pull request for the new approach, once there is one.
8. maflcko closed this on Jul 21, 2025

   ---

Contributors
marshallshen DrahtBot fjahr maflcko

Labels
Tests