test: chain reorg for coinstatsindex #31885

1. 
   mabu44 commented at 6:04 pm on February 16, 2025: none

   Improved coverage of src/index/coinstatsindex.cpp with a new unit test that simulate a reorg of the chain (one block deep).
2. 
   DrahtBot commented at 6:04 pm on February 16, 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/31885.

   Reviews

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

3. DrahtBot added the label Tests on Feb 16, 2025
4. 
   DrahtBot commented at 6:56 pm on February 16, 2025: contributor

   🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/37302278090

   Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

   • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

   • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

   • An intermittent issue.

   Leave a comment here, if you need help tracking down a confusing failure.

5. DrahtBot added the label CI failed on Feb 16, 2025
6. Unit test for chain reorg in coinstatsindex 9679761a88
7. mabu44 force-pushed on Feb 16, 2025
8. DrahtBot removed the label CI failed on Feb 16, 2025
9. 
   fjahr commented at 9:43 pm on February 16, 2025: contributor

   Hm, we have a reorg test for coinstatsindex in the functional tests: https://github.com/bitcoin/bitcoin/blob/master/test/functional/feature_coinstatsindex.py#L258 Can you give additional context why you think this improves coverage or other motivation to add a unit test for this?
10. 
    davidgumberg commented at 2:06 am on February 18, 2025: contributor

    @fjahr It seems to me that there are still parts of CoinStatsIndex that could be tested that might be appropriate for a unit test, for example, in ReverseBlock() that the hash of the block being reversed matches the hash of the latest block and some that could be either in a unit test or a functional test e.g. checking that unspendable totals are reversed, but it seems like the interface for BaseIndex‘es in general kind of enforces integration or functional style testing, because they basically only expose Init(), StartBackgroundSync(), Stop() and reading, which seems well-designed, but difficult to unit test.

    This is a more general question, and maybe this PR is not the venue, but if what I’m saying makes sense, because of this limitation should testing of the indexes generally be done in functional tests and we should avoid trying to directly test private interfaces or should some mechanism like e.g. making members protected and creating a test class that inherits from baseindex or coinstatsindex be used to make testing the private interface possible for unit testing?

Contributors
mabu44 DrahtBot fjahr davidgumberg

Labels
Tests