Merge ppa-dev-tools:fix-command-tests-with-releases into ppa-dev-tools:main

Proposed by Bryce Harrington
Status: Merged
Merged at revision: 06e615249c16abc78a23cf448e72405120384f2f
Proposed branch: ppa-dev-tools:fix-command-tests-with-releases
Merge into: ppa-dev-tools:main
Diff against target: 64 lines (+15/-4)
2 files modified
ppa/ppa.py (+3/-3)
scripts/ppa (+12/-1)
Reviewer Review Type Date Requested Status
Andreas Hasenack (community) Approve
PpaDevTools Developers Pending
Canonical Server Pending
Canonical Server Reporter Pending
Review via email: mp+451225@code.staging.launchpad.net

Description of the change

Fix for issue Andreas raised today, that running ppa tests --releases jammy produces no results when it should.

The fix for this is straightforward, and I noticed the same pattern a couple other places and fix them up as well.

There are actually several other bugs reported with related issues around test output filtering. I thought of including fixes for those as well, however they involve some additional plumbing work (and associated unit tests) and a fair bit of refactoring, so am going to leave that work for later, to keep this branch focused more on bug fixing. The refactoring of command_tests() has been an ongoing project, and those fixes will come as that work gets completed.

I'd really like to include some unit test updates with this branch, however test_command_tests() only deals with triggers so far. I started looking at expanding that to also include results, however I think it will be better to work on tests once the aforementioned refactoring is done.

For testing this fix, I ran the command in various ways against different people's PPAs. The commit messages make mention of some of the more interesting ones I tested against.

To post a comment you must log in.
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

+1

Initially I thought unpack_to_dict() was quite a convoluted way to make a comma-separated string a list, but it's a function used in many places and it has tests.

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

Thanks for the review Andreas, branch landed:

$ git merge --ff-only fix-command-tests-with-releases
Updating 6c63268..06e6152
Fast-forward
 ppa/ppa.py | 6 +++---
 scripts/ppa | 13 ++++++++++++-
 2 files changed, 15 insertions(+), 4 deletions(-)
$ git push origin
Total 0 (delta 0), reused 0 (delta 0), pack-reused 0
To git+ssh://git.launchpad.net/ppa-dev-tools
   6c63268..06e6152 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: