Split out key-value parsing in ArgsManager into its own function
ClosedPublic
Actions

Authored by jasonbcox on Nov 20 2019, 20:27.

Details

Reviewers

deadalnix
Fabien

Group Reviewers

Restricted Project

Commits

rSTAGING65f363942f17: Split out key-value parsing in ArgsManager into its own function
rABC65f363942f17: Split out key-value parsing in ArgsManager into its own function

Summary

This will allow us to parse CLI args while under test without
erroring on other args the test framework may be adding.

Test Plan

make check

Diff Detail

Repository

rABC Bitcoin ABC

Branch

parse-key-value

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 8196
Build 14418: Default Diff Build & Tests
Build 14417: arc lint + arc unit

Event Timeline

jasonbcox created this revision.Nov 20 2019, 20:27

Herald added a reviewer: deadalnix. · View Herald TranscriptNov 20 2019, 20:27

Build Bitcoin-ABC / Diffs / Diff Testing started.

jasonbcox added a child revision: D4474: Added support for -gravitonactivationtime to unit tests.Nov 20 2019, 20:29

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B8196: Diff 14266.Nov 20 2019, 20:42

Considering there are still many backports to do in there, I don't think this is very wise to go there.

Rebase

Build Bitcoin-ABC / Diffs / Diff Testing started.

In D4496#108676, @deadalnix wrote:

Considering there are still many backports to do in there, I don't think this is very wise to go there.

I'm aware of that. I checked Core's latest and this part of the code has remained the same so far. I only see one trivial backport that is missing between the lines that were pulled out.

All in all, I think taking ownership on this small piece is worth what we're getting out of it.

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B8226: Diff 14320.Nov 22 2019, 02:51

Mind adding an unit test ?

src/util/system.cpp
427 ↗	(On Diff #14320)	`size_t`

This revision now requires changes to proceed.Nov 22 2019, 07:24

Added some basic unit tests
int -> size_t
Funny how forcing yourself to write some tests reveals subtle design flaws. Moved ParseKeyValue() out of ArgsManager because it doesn't actually belong in that class. I still kept the implementations close to make backporting easier.