test: check assumeutxo file for changed outpoint index + de-duplications #28669

1. 
   fjahr commented at 4:37 pm on October 17, 2023: contributor

   Also doing some de-duplications. I kept the second commit separate for now as I am not 100% if this is overdoing it and makes it harder to reason about. But it also makes it easier to add more cases where we change more data.
2. 
   DrahtBot commented at 4:37 pm on October 17, 2023: contributor

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

   Code Coverage

   For detailed information about the code coverage, see the test coverage report.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	maflcko, achow101

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

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #28659 (assumeutxo, rpc: Add ‘start’ parameter to loadtxoutset by hernanmarino)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

3. DrahtBot added the label Tests on Oct 17, 2023
4. 
   maflcko commented at 11:29 am on October 19, 2023: member

   nit: duplicate is written with p
5. fanquake renamed this:
   test: check assumeutxo file for changed outpoint index + de-dublications
   test: check assumeutxo file for changed outpoint index + de-duplications
   on Oct 19, 2023
6. in test/functional/feature_assumeutxo.py:75 in 0ffc3ab55e outdated

   69@@ -70,6 +70,10 @@ def test_invalid_snapshot_scenarios(self, valid_snapshot_path):
   70             valid_snapshot_contents = f.read()
   71         bad_snapshot_path = valid_snapshot_path + '.mod'
   72 
   73+        def expected_log(msg):
   74+            with self.nodes[1].assert_debug_log([msg]):
   75+                assert_raises_rpc_error(-32603, "Unable to load UTXO snapshot", self.nodes[1].loadtxoutset, bad_snapshot_path)
   

   ---

   ---

   

   maflcko commented at 11:31 am on October 19, 2023:

   Not sure about this. Won’t this hard-code make it harder to check against the actual error message, if it is provided in the future.

   ---

   

   fjahr commented at 6:20 pm on October 19, 2023:

   You mean the RPC error message, right? Unless there is already a PR that conflicts with this, isn’t it better to hardcode it in one place rather than many, i.e. following DRY? When there are changes on the RPC this will certainly evolve, maybe by removing the helper function or maybe by adding the RPC error message as an additional param to the helper.

   ---

   

   maflcko commented at 8:02 pm on October 19, 2023:

   In any case, there is already one place right below where you are not using the helper. I think it would be better to either use the helper everywhere or not at all.

   ---

   

   fjahr commented at 12:36 pm on October 20, 2023:

   Alright, I adapted it so it the help is also used for that first case.
7. glozow added this to the milestone 26.0 on Oct 20, 2023
8. fjahr force-pushed on Oct 20, 2023
9. test: check au file with changed outpoint index 0a576d62fe
10. test: De-dublicate/optimize assumeutxo test for further extensions d3223685b1
11. fjahr force-pushed on Oct 20, 2023
12. 
    fjahr commented at 12:40 pm on October 20, 2023: contributor

    Addressed @maflcko ’s feedback and rebased.
13. 
    maflcko commented at 1:50 pm on October 20, 2023: member

    lgtm ACK d3223685b1bb3fb4b4626d2afe4bf753e04f7b0a
14. DrahtBot added the label CI failed on Oct 20, 2023
15. 
    maflcko commented at 2:45 pm on October 20, 2023: member

    fuzz timeout can be ignored for now
16. 
    achow101 commented at 8:08 pm on October 20, 2023: member

    ACK d3223685b1bb3fb4b4626d2afe4bf753e04f7b0a
17. achow101 merged this on Oct 20, 2023
18. achow101 closed this on Oct 20, 2023

    ---

Contributors
fjahr DrahtBot maflcko achow101

Labels
Tests CI failed

Milestone
26.0