Merge ppa-dev-tools:add_unpack_to_dict into ppa-dev-tools:main

Proposed by Bryce Harrington
Status: Merged
Merge reported by: Bryce Harrington
Merged at revision: 8a1b18cebbfd84db7011df6569febb757531fa9e
Proposed branch: ppa-dev-tools:add_unpack_to_dict
Merge into: ppa-dev-tools:main
Diff against target: 312 lines (+300/-0)
2 files modified
ppa/dict.py (+183/-0)
tests/test_dict.py (+117/-0)
Reviewer Review Type Date Requested Status
Lena Voytek (community) Approve
Canonical Server Pending
Canonical Server Reporter Pending
Review via email: mp+437970@code.staging.launchpad.net

Description of the change

ppa-dev-tools needs to be able to parse strings of comma-separated values. There are three different types of these strings: Argument values (i.e. --architectures=...); Debian control fields (i.e. "Depends: foo, bar (>= 1.2), baw | bay | baz"); and autopkgtest trigger lists.

This branch adds an "unpack_to_dict()" helper routine that strives to solve all these needs. It supports overriding the markers to allow customizing the parsing behavior to cover the aforementioned use cases. Hopefully it'll be useful in other situations too.

I recognize one issue with trying to solve multiple special cases with one generic routine is there can be exceptions for one case that interfere with others, and handling them can make the generic routine less useful than just having separate routines for each case. Thankfully, I haven't yet run into any exceptions this can't handle, but admittedly I haven't used this in practice very widely. To mitigate this problem, I've implemented an extensive set of test cases that can be expanded as exceptions and new use cases are discovered to ensure that the existing use cases remain correctly handled.

The tests can be invoked via:

    $ pytest-3 ./tests/test_dict.py

There is also a smoketest that demonstrates parsing of some different real-world examples, to give a better feel for how this will be used in practice.

    $ python3 ./ppa/dict.py

To post a comment you must log in.
Revision history for this message
Lena Voytek (lvoytek) wrote :

The additions here look good to me. I also ran the unit tests on my system and everything passed. I added some comments for nitpicks + code clarity below

review: Approve
Revision history for this message
Bryce Harrington (bryce) wrote :

Thanks! I've incorporated all your feedback (and some from Athos on another MP). I've force-pushed the branch with all the corrections here, and will land the same branch to main.

To git+ssh://git.launchpad.net/ppa-dev-tools
 + 8a1b18c...555b089 add_unpack_to_dict -> add_unpack_to_dict (forced update)

To git+ssh://git.launchpad.net/ppa-dev-tools
   8ca9a46..555b089 main -> main

There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: