Merge ppa-dev-tools:testbed-setup-failures-are-bad into ppa-dev-tools:main

[Bryce Harrington]

If there is an error setting up the testbed, it'll error out before any subtests are generated. ppa-dev-tools was not correctly checking for or handling this state and required a few different fixups.

Thanks Mitchell for finding and reporting this corner case.

(Hmm, come to think of it I should probably also write test cases to accompany this...)

[Mitchell Dzurick (mitchdz)]

Huh I don't see this branch fixing my issue for some reason. Am I installing it wrong?

1. I checked out testbed-setup-failures-are-bad
2. `snapcraft`
3. `snap install ppa-dev-tools_v0.5.0+git43.0c5c395_amd64.snap --dangerous`

It just looks identical to the failure in the bug

[Athos Ribeiro (athos-ribeiro)]

I ran the following command from the main brainch:

$ ppa tests https://launchpad.net/~mitchdz/+archive/ubuntu/rabbitmq-server-mre-2204/

and got the following results (leaving only relevant bits):

* Results:
  -
    + ✅ rabbitmq-server on focal for amd64 @ 16.05.24 01:43:46 Log️ 🗒️
    + ✅ rabbitmq-server on focal for arm64 @ 12.05.24 03:07:20 Log️ 🗒️
    + ✅ rabbitmq-server on focal for armhf @ 11.05.24 08:01:13 Log️ 🗒️
  - rabbitmq-server/3.8.3-0ubuntu0.1~focal5
    + ✅ rabbitmq-server on focal for i386 @ 11.05.24 13:35:11 Log️ 🗒️
    + ✅ rabbitmq-server on focal for ppc64el @ 14.05.24 23:10:14 Log️ 🗒️
    + ✅ rabbitmq-server on focal for s390x @ 14.05.24 03:20:42 Log️ 🗒️

Then, I ran the same command from the proposed branch and got the following results:

* Results:
  - rabbitmq-server/3.8.3-0ubuntu0.1~focal5
    + ⛔ rabbitmq-server on focal for amd64 @ 16.05.24 01:43:46 Log️ 🗒️
      • testbed BAD ⛔
    + ⛔ rabbitmq-server on focal for arm64 @ 12.05.24 03:07:20 Log️ 🗒️
      • testbed BAD ⛔
    + ⛔ rabbitmq-server on focal for armhf @ 11.05.24 08:01:13 Log️ 🗒️
      • testbed BAD ⛔
    + ✅ rabbitmq-server on focal for i386 @ 11.05.24 13:35:11 Log️ 🗒️
    + ✅ rabbitmq-server on focal for ppc64el @ 14.05.24 23:10:14 Log️ 🗒️
    + ✅ rabbitmq-server on focal for s390x @ 14.05.24 03:20:42 Log️ 🗒️

This works for me.

Mitchell, have you checked the code in your installed snap to make sure it was indeed carrying this patch?

[Athos Ribeiro (athos-ribeiro)]

As Bryce mentioned, it would be nice to have some tests for this one.

Still, this LGTM. It would also be nice to verify why Mitchell did not get the same results as I did while testing this before merging.

review: Approve

diff --git a/ppa/result.py b/ppa/result.py
index 7488f54..125c134 100755
--- a/ppa/result.py
+++ b/ppa/result.py
@@ -163,12 +163,9 @@ class Result:
         """
         regex_triggers = re.compile(r'--env=ADT_TEST_TRIGGERS=(.*?) -- ')
         header_split = self.log.split(": @@@@@@@@@@@@@@@@@@@@", 1)
-        if len(header_split) < 2:
-            return []
         m = re.search(regex_triggers, header_split[0])
         if not m:
             return []
-
         return m.group(1).strip("'").split()
 
     @lru_cache
@@ -201,7 +198,8 @@ class Result:
         """
         result_split = self.log.split("@@@@@@@@@@@@@@@@@@@@ summary", 1)
         if len(result_split) < 2:
-            return []
+            self.error_message = "Failure setting up testbed ⚪"
+            return [Subtest("testbed setup failure  BAD")]
 
         subtests = []
         result_sum = result_split[1]
@@ -235,6 +233,8 @@ class Result:
         for subtest in self.get_subtests():
             if subtest.status == 'FAIL':
                 return 'FAIL'
+            elif subtest.status == 'BAD':
+                return 'BAD'
         return 'PASS'
 
     @property
diff --git a/scripts/ppa b/scripts/ppa
index c2f803d..7046f73 100755
--- a/scripts/ppa
+++ b/scripts/ppa
@@ -1057,7 +1057,6 @@ def command_tests(lp: Lp, config: dict[str, str]) -> int:
             )
 
         # Results
-        the_ppa.get_autopkgtest_results(releases, architectures, packages)
         show_results(the_ppa.get_autopkgtest_results(releases, architectures, packages),
                      config.get('show_urls'))
 
diff --git a/tests/data/results-rabbitmq-server-armhf.log.gz b/tests/data/results-rabbitmq-server-armhf.log.gz
new file mode 100644
index 0000000..ad604cf
Binary files /dev/null and b/tests/data/results-rabbitmq-server-armhf.log.gz differ
diff --git a/tests/test_result.py b/tests/test_result.py
index 89a6c8f..b205634 100644
--- a/tests/test_result.py
+++ b/tests/test_result.py
@@ -126,6 +126,7 @@ def test_log(tmp_path):
 
 
 @pytest.mark.parametrize('filename, expected_triggers', [
+    ('results-rabbitmq-server-armhf.log.gz', ['rabbitmq-server/3.9.27-0ubuntu0.1~jammy8']),
     ('results-six-s390x.log.gz', ['pygobject/3.42.2-2', 'six/1.16.0-4']),
     ('results-chrony-armhf.log.gz', ['dpkg/1.22.6ubuntu5'])
 ])
@@ -171,7 +172,7 @@ def test_get_triggers(monkeypatch, triggers, name, expected):
 
 
 @pytest.mark.parametrize('log_text, subtest_name, expected', [
-    ('', None, {}),
+    ('', None, {'testbed': 'BAD'}),
     (
         (
             "x: @@@@@@@@@@@@@@@@@@@@ summary\n"
@@ -300,13 +301,25 @@ def test_status_icon_error(monkeypatch, status, expected_exception):
 
 @pytest.mark.parametrize('log_text, series, arch, source, expected_dict', [
     (
-        # Empty/invalid log should return empty triggers & subtests.
-        'x', 'x', 'x', 'x', {'triggers': [], 'subtests': []}
+        # Empty/invalid log should return empty triggers
+        'x', 'x', 'x', 'x', {'triggers': []}
     ),
 
     (
-        # Empty/invalid log counts as an overall test pass.
-        'x', 'x', 'x', 'x', {'status': 'PASS', 'status_icon': '✅'}
+        # Empty/invalid log counts as an overall test state BAD.
+        'x', 'x', 'x', 'x',
+        {
+            'status': 'BAD',
+            'status_icon': '⛔',
+            'subtests': [
+                {
+                    'desc': 'testbed',
+                    'line': 'testbed setup failure  BAD',
+                    'status': 'BAD',
+                    'status_icon': '⛔'
+                }
+            ]
+        }
     ),
 
     (
@@ -493,10 +506,14 @@ def test_get_results(tmp_path,
 
 
 @pytest.mark.parametrize('data, show_urls, expected_num_lines, expected_in_stdout', [
-    # When there are no results, indicate '(none)'
-    ({}, True, 1, "* Results: (none)\n"),
-    ({'trigger-1': ('a', 'b', 'c', 'x')}, True, 4, "- trigger-1\n    + ✅ c on a for b"),
-    ({'t1': ('a', 'b', 'c', 'x'), 't2': ('a', 'b', 'c', 'x')}, True, 7, "- t2"),
+    (
+        # When there are no results, indicate '(none)'
+        {}, True, 1, "* Results: (none)\n"
+    ),
+    (
+        {'trigger-1': ('a', 'b', 'c', 'x')}, True, 6, "- trigger-1\n    + ⛔ c on a for b"
+    ),
+    ({'t1': ('a', 'b', 'c', 'x'), 't2': ('a', 'b', 'c', 'x')}, True, 11, "- t2"),
 ])
 def test_show_results(capfd, tmp_path,
                       data, show_urls, expected_num_lines, expected_in_stdout):