test: Add signet witness commitment section parse tests #20004

1. 
   MarcoFalke commented at 1:40 pm on September 23, 2020: member
2. fanquake added the label Tests on Sep 23, 2020
3. 
   DrahtBot commented at 4:46 pm on September 23, 2020: member

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

   Conflicts

   No conflicts as of last run.

   Coverage

   Coverage	Change (pull 20004, feeca2363d4c1c78e6dfb40b8684870fe84b084a)	Reference (master, ec9b4492eba5d32ab833d511984756601d3f39b0)
   Lines	+0.0035 %	90.7088 %
   Functions	+0.0034 %	86.2602 %
   Branches	-0.0028 %	52.0393 %

   Updated at: 2020-09-29T09:37:30.142032.

4. MarcoFalke force-pushed on Sep 24, 2020
5. 
   MarcoFalke commented at 5:43 pm on September 24, 2020: member

   Before: https://drahtbot.github.io/reports/coverage/bitcoin/bitcoin/master/total.coverage/src/signet.cpp.gcov.html After: https://drahtbot.github.io/reports/coverage/bitcoin/bitcoin/20004/total.coverage/src/signet.cpp.gcov.html
6. 
   MarcoFalke commented at 7:28 am on September 28, 2020: member

   cc @kallewoof / @ajtowns :sweat_smile:
7. 
   kallewoof commented at 8:07 am on September 28, 2020: member

   Nice!

   utACK faee74f6c841eea391c08ebbadb045746c3ef77f

8. 
   ajtowns commented at 5:40 am on September 29, 2020: member

   Not really convinced always explicitly passing in an ArgsManager to CreateChainParams is an improvement. Could make it

   0std::unique_ptr<const CChainParams> CreateChainParams(const std::string& chain, ArgsManager* pargs = nullptr)
   1{
   2   ArgsManager& args = (pargs != nullptr ? *pargs : gArgs);
   3    ...
   4}
   

   or similar and avoid touching all the other code?

   Seems kind of a shame not to add a dummy tx a well to get to 100% line coverage?

9. DrahtBot added the label Needs rebase on Sep 29, 2020
10. Remove gArgs global from CreateChainParams to aid testing fa23308e9a
11. test: Add signet witness commitment section parse tests fa29b5ae66
12. MarcoFalke force-pushed on Sep 29, 2020
13. 
    MarcoFalke commented at 9:41 am on September 29, 2020: member

    avoid touching all the other code

    Long term goal is to get rid of the gArgs global in Core (though, not the gui), so I think this patch will be done anyway. Left as is for now.

    Seems kind of a shame not to add a dummy tx a well to get to 100% line coverage?

    Done :100:

14. DrahtBot removed the label Needs rebase on Sep 29, 2020
15. 
    laanwj commented at 2:58 pm on September 30, 2020: member

    ACK fa29b5ae666bbb4c19188f0dcf8a1ba738aac624

    Long term goal is to get rid of the gArgs global in Core (though, not the gui), so I think this patch will be done anyway. Left as is for now.

    I think this is overall an improvement, to pass the arguments in. It’s also a straightforward, pretty much obviously correct change I’m okay with doing it here.

16. laanwj merged this on Sep 30, 2020
17. laanwj closed this on Sep 30, 2020

    ---

18. sidhujag referenced this in commit e03b190aed on Sep 30, 2020
19. MarcoFalke deleted the branch on Sep 30, 2020
20. fanquake referenced this in commit 82b70f15c7 on Oct 2, 2020
21. MarcoFalke referenced this in commit a6a993a888 on Oct 2, 2020
22. sidhujag referenced this in commit 0dbf866d1d on Oct 4, 2020
23. DrahtBot locked this on Feb 15, 2022

Contributors
MarcoFalke DrahtBot kallewoof ajtowns laanwj

Labels
Tests