test: Add signet witness commitment section parse tests #20004

pull MarcoFalke wants to merge 2 commits into bitcoin:master from MarcoFalke:2009-testSignetParse changing 9 files +96 −33

MarcoFalke commented at 1:40 PM on September 23, 2020: member

No description provided.
fanquake added the label Tests on Sep 23, 2020

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 %

<sup>Updated at: 2020-09-29T09:37:30.142032.</sup>

MarcoFalke force-pushed on Sep 24, 2020
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
MarcoFalke commented at 7:28 AM on September 28, 2020: member

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

Nice!

utACK faee74f6c841eea391c08ebbadb045746c3ef77f
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
```
std::unique_ptr<const CChainParams> CreateChainParams(const std::string& chain, ArgsManager* pargs = nullptr)
{
   ArgsManager& args = (pargs != nullptr ? *pargs : gArgs);
    ...
}
```
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?
DrahtBot added the label Needs rebase on Sep 29, 2020
Remove gArgs global from CreateChainParams to aid testing fa23308e9a
test: Add signet witness commitment section parse tests fa29b5ae66
MarcoFalke force-pushed on Sep 29, 2020
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:
DrahtBot removed the label Needs rebase on Sep 29, 2020
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.
laanwj merged this on Sep 30, 2020
laanwj closed this on Sep 30, 2020
sidhujag referenced this in commit e03b190aed on Sep 30, 2020
MarcoFalke deleted the branch on Sep 30, 2020
fanquake referenced this in commit 82b70f15c7 on Oct 2, 2020
MarcoFalke referenced this in commit a6a993a888 on Oct 2, 2020
sidhujag referenced this in commit 0dbf866d1d on Oct 4, 2020
DrahtBot locked this on Feb 15, 2022

Contributors

Labels

Linked (view graph)

#20060 refactor: fix -Wbraced-scalar-init warning in validation tests