test: add coverage for abandoning unconfirmed transaction #31943

1. 
   rkrux commented at 10:20 am on February 24, 2025: contributor

   Previous discussion: #31794#pullrequestreview-2605174936 Current Coverage: https://maflcko.github.io/b-c-cov/total.coverage/src/wallet/wallet.cpp.gcov.html#L1326
2. test: add coverage for abandoning unconfirmed transaction

   Co-authored-by: Eunovo <eunovo9@gmail.com>

   073a017016
3. 
   DrahtBot commented at 10:20 am on February 24, 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/31943.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	Eunovo, janb84, maflcko, brunoerg
   Concept ACK	Prabhat1308

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

4. DrahtBot added the label Tests on Feb 24, 2025
5. rkrux marked this as ready for review on Feb 24, 2025
6. 
   Eunovo commented at 12:22 pm on February 24, 2025: contributor

   Tested ACK https://github.com/bitcoin/bitcoin/pull/31943/commits/073a017016e62c7d52562aa61d942024379c110d
7. 
   Prabhat1308 commented at 11:26 pm on February 24, 2025: none

   Concept ACK 073a017
8. 
   maflcko commented at 10:11 am on February 25, 2025: member

   Covers the previously uncovered TransactionCanBeAbandoned() function increasing the coverage .

   I don’t understand this comment. TransactionCanBeAbandoned is only called by the GUI, but this new test isn’t a gui test.

   Also, https://corecheck.dev/bitcoin/bitcoin/pulls/31943 doesn’t show new code coverage for TransactionCanBeAbandoned or anything else in wallet cpp

9. 
   Prabhat1308 commented at 10:32 am on February 25, 2025: none

   I don’t understand this comment. TransactionCanBeAbandoned is only called by the GUI, but this new test isn’t a gui test.

   The confusion arose from my side . The RPC has the response transaction not eligible for abandonment and from the current coverage in the PR description (which I thought to be the pre-pull coverage) , the marked line was already covered. So , I incidently thought the TransactionCanBeAbandoned to be the check for abandonment function refered to increase the coverage.

   Removed the comment.

10. 
    rkrux commented at 11:09 am on February 25, 2025: contributor

    I don’t understand this comment. TransactionCanBeAbandoned is only called by the GUI, but this new test isn’t a gui test.

    Correct, this doesn’t cover TransactionCanBeAbandoned, which is called by GUI only. Instead, this covers the second condition in this if check here: https://github.com/bitcoin/bitcoin/blob/e486597f9a57903600656fb5106858941885852f/src/wallet/wallet.cpp#L1326C43-L1326C56

    Also, https://corecheck.dev/bitcoin/bitcoin/pulls/31943 doesn’t show new code coverage for TransactionCanBeAbandoned or anything else in wallet cpp

    This is most likely because it checks on new lines covered by the code. However, this test doesn’t cover a new line but a new branch of the already covered line.

11. 
    janb84 commented at 8:25 pm on February 25, 2025: none

    Tested ACK 073a017
12. DrahtBot requested review from Prabhat1308 on Feb 25, 2025
13. 
    maflcko commented at 8:43 pm on February 25, 2025: member

    lgtm ACK 073a017016e62c7d52562aa61d942024379c110d
14. brunoerg approved
15. 
    brunoerg commented at 2:34 pm on February 26, 2025: contributor

    utACK 073a017016e62c7d52562aa61d942024379c110d
16. fanquake merged this on Feb 26, 2025
17. fanquake closed this on Feb 26, 2025

    ---

Contributors
rkrux DrahtBot Eunovo Prabhat1308 maflcko janb84 brunoerg

Review Requested
Prabhat1308

Labels
Tests