Tests: Create functional tests for coin selection #25936

1. 
   akankshakashyap commented at 6:29 PM on August 26, 2022: contributor

   Added functional test for coin selection which compare waste metric

2. akankshakashyap force-pushed on Aug 26, 2022
3. 
   brunoerg commented at 6:37 PM on August 26, 2022: contributor

   You should change the file permission to 755 instead of 644.

4. akankshakashyap force-pushed on Aug 26, 2022
5. 
   akankshakashyap commented at 6:55 PM on August 26, 2022: contributor

   You should change the file permission to 755 instead of 644.

   done.

6. akankshakashyap force-pushed on Aug 26, 2022
7. DrahtBot added the label Tests on Aug 26, 2022
8. create functional tests for coin selection 93dcd91e6e
9. akankshakashyap force-pushed on Aug 27, 2022
10. 
    ishaanam commented at 4:53 PM on August 28, 2022: contributor

    Thanks for working on increasing test coverage, I would find it a bit easier to review this PR if you explained why a functional test is needed for this as opposed to extending the existing coin selection unit tests?

11. fanquake requested review from murchandamus on Aug 31, 2022
12. 
    DrahtBot commented at 4:55 AM on September 23, 2022: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    No conflicts as of last run.

13. 
    murchandamus commented at 7:23 PM on September 29, 2022: contributor

    Concept NACK

    In functional tests, we test whether outside calls to Bitcoin Core will receive sensible responses, treating all of Bitcoin Core as one black box. Functional tests are expensive to run in comparison to unit tests, because we spin up whole instances of the node for each test suite. Because functional tests are expensive, we want to use them when we are verifying that the whole system works correctly integrally. However overall, we would like to reduce the count of functional tests by converting them to unit tests to reduce the runtime of the functional tests when possible.

    The coin selection functionality is a small, well-delineated part of the transaction building within the wallet code, with the waste metric itself being an even smaller subset thereof. The proposed tests are testing the current internal implementation logic of this small part of the wallet code by executing the whole stack of calls, but functionally, we could have a different internal implementation that would still produce valid, functionally equivalent transactions. I would second @ishaanam's question from above that the tests would be better situated in a unit test that specifically focuses on the waste metric and coin selection.

14. 
    fanquake commented at 2:50 PM on October 2, 2022: member

    Thanks for contributing changes, however there's no agreement that this is a good approach. Going to close this PR for now.

15. fanquake closed this on Oct 2, 2022

    ---

16. bitcoin locked this on Oct 2, 2023

Review Requested
murchandamus