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.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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/ppa/ppa.py b/ppa/ppa.py
2index 57ec14b..23ea58a 100755
3--- a/ppa/ppa.py
4+++ b/ppa/ppa.py
5@@ -528,7 +528,7 @@ class Ppa:
6 print("Successfully published all builds for all architectures")
7 return retval
8
9- def get_autopkgtest_waiting(self, releases):
10+ def get_autopkgtest_waiting(self, releases: list[str]):
11 """Return iterator of queued autopkgtests for this PPA.
12
13 See get_waiting() for details
14@@ -542,7 +542,7 @@ class Ppa:
15 return get_waiting(response, releases=releases, ppa=str(self))
16 return []
17
18- def get_autopkgtest_running(self, releases):
19+ def get_autopkgtest_running(self, releases: list[str]):
20 """Return iterator of queued autopkgtests for this PPA.
21
22 See get_running() for details
23@@ -556,7 +556,7 @@ class Ppa:
24 return get_running(response, releases=releases, ppa=str(self))
25 return []
26
27- def get_autopkgtest_results(self, releases, architectures):
28+ def get_autopkgtest_results(self, releases: list[str], architectures: list[str]):
29 """Returns iterator of results from autopkgtest runs for this PPA.
30
31 See get_results() for details
32diff --git a/scripts/ppa b/scripts/ppa
33index 9dbb15a..edf82b7 100755
34--- a/scripts/ppa
35+++ b/scripts/ppa
36@@ -849,8 +849,17 @@ def command_tests(lp: Lp, config: dict[str, str]) -> int:
37 if releases is None:
38 udi = UbuntuDistroInfo()
39 releases = udi.supported()
40+ if isinstance(releases, str):
41+ releases = list(unpack_to_dict(releases).keys())
42+ if not isinstance(releases, list):
43+ raise TypeError(f"Parameter releases={releases} not of type list")
44
45 packages = config.get('packages', None)
46+ if not packages is None:
47+ if isinstance(packages, str):
48+ packages = list(unpack_to_dict(packages).keys())
49+ if not isinstance(packages, list):
50+ raise TypeError(f"Parameter packages={packages} not of type list")
51
52 the_ppa = get_ppa(lp, config)
53 if not the_ppa.exists():
54@@ -859,7 +868,9 @@ def command_tests(lp: Lp, config: dict[str, str]) -> int:
55
56 architectures = config.get('architectures', ARCHES_AUTOPKGTEST)
57 if isinstance(architectures, str):
58- architectures = unpack_to_dict(architectures).keys()
59+ architectures = list(unpack_to_dict(architectures).keys())
60+ if not isinstance(architectures, list):
61+ raise TypeError(f"Parameter architectures={architectures} not of type list")
62
63 try:
64 # Triggers

Subscribers

People subscribed via source and target branches

to all changes: