Merge ubuntu-archive-tools:blocked-by-tests into ubuntu-archive-tools:main

[Brian Murray]

We'd been having issues with tests receiving a 403 from the proxy server and I wanted to rerun all the test failures of ubuntu-release-upgrader. I thought the --blocked-by argument would be the right one but it isn't. To avoid confusion going forward I've added a new '--blocked-by-tests' argument which might be clearer (I'm open to suggestions on the name and help). Additionally, this seems better than 'retry-autopkgtest-regressions | grep ubuntu-release-upgrader'.

[Brian Murray (brian-murray)]

 $ ./retry-autopkgtest-regressions --blocked-by-tests ubuntu-release-upgrader
https://autopkgtest.ubuntu.com/request.cgi?release=kinetic&arch=arm64&package=ubuntu-release-upgrader&trigger=psmisc%2F23.5-2
https://autopkgtest.ubuntu.com/request.cgi?release=kinetic&arch=s390x&package=ubuntu-release-upgrader&trigger=psmisc%2F23.5-2

Sample output.

[Lukas Märdian (slyon)]

The new "--blocked-by-tests" logic LGTM and behaves in a way that I would have assumed "--blocked-by" works. BUT: There's one line "matches_blocked_by = False" that unconditionally disables the "--blocked-by" logic IMO, which you should have another look at (see inline comment below).

After that line is double checked (and potentially fixed) this MP is good for merging IMO.

=========

Handling the "--blocked-by" confusion is another topic, that we might or might not address in a different MP:

Looking at a report like this:
https://people.canonical.com/~ubuntu-archive/proposed-migration/update_excuses_by_team.html

binutils blocking gcc-11 (11.3.0-4ubuntu1 to 11.3.0-5ubuntu1) for 4 days
Regressions
binutils/2.38.90.20220713-2ubuntu1: arm64 (log, history)

I would assume "./retry-autopkgtests --blocks gcc-11" to output some "&package=binutils&trigger=gcc-11/..." tests (this works as expected) and "./retry-autopkgtests --blocked-by binutils" (being the reverse) to output the same "&package=binutils&trigger=gcc-11/..." test, but using the reverse search key. => This is what "--blocked-by-tests binutils" is now handling properly.

IMO the "--blocked-by" parameter is confusing and should maybe be renamed. Looking at the source code, it checks for "migrate-after", so maybe it should be renamed to something like "--depends"/"--requires"/"--migrate-after"/("--after-transition"). WDYT?

[Lukas Märdian (slyon)]

Thanks, LGTM.

review: Approve

[Paride Legovini (paride)]

Admittedly I had to stop and think a bit about what --blocked-by / --blocked-by-tests do, but it's mostly --blocked-by that confuses me. This LGTM, thanks.

review: Approve

[Brian Murray (brian-murray)]

I opened https://bugs.launchpad.net/ubuntu-archive-tools/+bug/1983059 regarding --blocked-by.

diff --git a/retry-autopkgtest-regressions b/retry-autopkgtest-regressions
index 8752f92..7ff5a05 100755
--- a/retry-autopkgtest-regressions
+++ b/retry-autopkgtest-regressions
@@ -83,6 +83,9 @@ def parse_args():
     parser.add_argument('--blocked-by',
                         help='rerun only those tests that are blocked by (rdeps of) '
                         'the named package')
+    parser.add_argument('--blocked-by-tests',
+                        help='rerun only those tests that are blocked by test '
+                        'failures of the named package')
     parser.add_argument('--no-proposed', action='store_true',
                         help='run tests against release+updates instead of '
                         'against proposed, to re-establish a baseline for the '
@@ -224,7 +227,8 @@ def already_triggered(release, arch, pkg, triggers, extra_params, running, queue
 
 
 def get_regressions(excuses_url, release, retry_states, min_age, max_age,
-                    blocks, blocked_by, no_proposed, log_regex, force_cached):
+                    blocks, blocked_by, blocked_by_tests, no_proposed,
+                    log_regex, force_cached):
     '''Return dictionary with regressions
 
     Return dict: release → pkg → arch → [trigger, ...]
@@ -255,8 +259,16 @@ def get_regressions(excuses_url, release, retry_states, min_age, max_age,
             matches_blocked_by = blocked_by and blocked_by in excuse['dependencies']['migrate-after']
         except KeyError:
             matches_blocked_by = False
+        try:
+            matches_blocked_by_tests = False
+            for issue in excuse['excuses']:
+                if issue.startswith('autopkgtest for %s' % blocked_by_tests):
+                    matches_blocked_by_tests = True
+                    break
+        except KeyError:
+            mactches_blocked_by_tests = False
 
-        if (blocks or blocked_by) and not (matches_blocks or matches_blocked_by):
+        if (blocks or blocked_by or blocked_by_tests) and not (matches_blocks or matches_blocked_by or matches_blocked_by_tests):
                 continue
 
         try:
@@ -277,6 +289,8 @@ def get_regressions(excuses_url, release, retry_states, min_age, max_age,
                 if no_proposed:
                     continue
                 pkg_ver = None
+            if matches_blocked_by_tests and pkg != blocked_by_tests:
+                continue
             if no_proposed:
                 trigger = 'migration-reference/0'
             else:
@@ -356,8 +370,9 @@ else:
     excuses_url = 'http://people.canonical.com/~ubuntu-archive/proposed-migration/%s/update_excuses.yaml.xz' % args.series
 regressions = get_regressions(excuses_url, args.series, args.state,
                               args.min_age, args.max_age, args.blocks,
-                              args.blocked_by, args.no_proposed,
-                              args.log_regex, args.force_cached)
+                              args.blocked_by, args.blocked_by_tests,
+                              args.no_proposed, args.log_regex,
+                              args.force_cached)
 
 
 # load JSON running and queued tests