test: added coverage to bitcoin-chainstate testing invalid block header #32190

1. 
   kevkevinpal commented at 7:42 pm on April 1, 2025: contributor

   Summary

   This adds coverage to src/bitcoin-chainstate.cpp where we get BLOCK_INVALID_HEADER and the error message invalid proof of work or time too old

   Where coverage is added

   this change was motivated by this comment

   ---

   You can check that you cannot find this message with test coverage by using grep grep -nri "or time too old" ./test/functional/

2. test: added coverage to bitcoin-chainstate testing invalid block header 847e7f96c0
3. 
   DrahtBot commented at 7:42 pm on April 1, 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/32190.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   Approach ACK	shahsb

   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 Apr 1, 2025
5. 
   shahsb commented at 8:32 am on April 2, 2025: none

   LGTM!
6. 
   shahsb commented at 8:35 am on April 2, 2025: none

   Concept ACK + Approach ACK!
7. 
   maflcko commented at 8:42 am on April 2, 2025: member

   Not sure about this. The executable is just a test-only dev-only experimental dummy, expected to change in the future. So adding tests for all edge cases is verbose and will make it harder to change the behavior in the future.
8. 
   Prabhat1308 commented at 8:47 am on April 2, 2025: contributor

   Generating a report I see that almost all the cases don’t have coverage .

   It might be intentional that this has been left. Also adding for only 1 case seems too less . Might add more if you think it might be worth it to add coverage.

   EDIT: While building to test this from an unclean built to build with -DBUILD_UTIL_CHAINSTATE=ON flag , faced an issue , possibly due to issue 31942 since cleaning the build fixed it.

9. 
   kevkevinpal commented at 1:59 pm on April 2, 2025: contributor

   Not sure about this. The executable is just a test-only dev-only experimental dummy, expected to change in the future. So adding tests for all edge cases is verbose and will make it harder to change the behavior in the future.

   Ok no problem, I can close this PR and can repopen in the future if its needed

   thanks everyone for the review on this one

10. kevkevinpal closed this on Apr 2, 2025

    ---

Contributors
kevkevinpal DrahtBot shahsb maflcko Prabhat1308

Labels
Tests