fix(dashboard): drop obsoleted openQA jobs and stop the false "All jobs passed"

Two correctness bugs in the dashboard-backed auto-workflow openQA provider
(DashboardAutoOpenQA), both in how it interprets job results.

Obsoleted (superseded) jobs were never filtered. _normalize_job captured the
dashboard's `obsolete` flag but nothing used it, so a retriggered job's stale
older run stayed in the working set: its old result was folded into the install
verdict (_has_passed_install_jobs) and it surfaced as a phantom entry in the
failed-jobs listing. A qam-incidentinstall that failed, was retriggered, and now
passes was therefore reported as "no successful install". Drop obsoleted runs in
_load_jobs (new _is_obsolete helper: the `obsolete` flag or an `obsoleted`
result), matching oqa_search.incident_jobs; only the current run per scenario is
kept.

The section trailer printed "All jobs passed." whenever failed_by_group was
empty, even when problem_keys was not. A group whose only problem lives in the
`other` bucket (still-running result=none, parallel_failed, skipped, ...) is
flagged in the Summary but carries no failed/incomplete/timeout job, so the
section contradicted itself. Gate the trailer on problem_keys: emit a
"some groups need review" note for that case, keep "All jobs passed." only when
there are genuinely no problem groups, and never print an empty Failed jobs
block.

Both fixes are revert-verified by the added tests. Resolves findings openSUSE#7 and openSUSE#8
from the recent code review.

Author: plusky

CHANGELOG.md

@@ -349,6 +349,18 @@ and this project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.ht
   logged and abandoned so `close()` — and the http registry's idle-sweep behind
   it — always returns. Previously the bound was defeated by the thread pool's
   context-manager exit, which re-joined every worker regardless of the timeout.
+- The dashboard-backed auto-workflow openQA report no longer counts superseded
+  (obsoleted) jobs. When a job is retriggered the dashboard keeps the older run;
+  its stale result was still folded into the install verdict and listed as a
+  phantom failure. Obsoleted runs (marked by the dashboard's `obsolete` flag or
+  an `obsoleted` result) are now dropped, matching the openQA-search connector,
+  so a retriggered-and-now-passing install is reported as passed and only the
+  current run per scenario is shown.
+- The dashboard-backed auto-workflow openQA section no longer prints
+  "All jobs passed." while its own Summary lists a problem group. A group whose
+  only problem is a still-running, `parallel_failed`, or otherwise non-failed
+  job (i.e. it has no failed/incomplete/timeout job) is now reported with a
+  "some groups need review" note instead of a contradictory all-passed verdict.
 - An unscoped multi-template fan-out of a host-phase command (e.g. `lock`,
   `run`) no longer fails the whole fan-out — or, for `lock`, pretends to
   succeed — when a loaded template has no connected host. A host-less template

mtui/data_sources/qem_dashboard/dashboard_openqa.py

@@ -98,9 +98,17 @@ def _fetch(source: str, setting_id: int) -> list[dict[str, Any]]:
                     )
                     future.cancel()
                     continue
-                jobs.extend(
-                    self._normalize_job(job, source, setting) for job in setting_jobs
-                )
+                for job in setting_jobs:
+                    normalized = self._normalize_job(job, source, setting)
+                    if self._is_obsolete(normalized):
+                        logger.debug(
+                            "dropping obsoleted %s job %s (%s)",
+                            source,
+                            normalized.get("id"),
+                            normalized.get("test"),
+                        )
+                        continue
+                    jobs.append(normalized)
 
         return jobs
 
@@ -154,6 +162,20 @@ def _normalize_job(
             normalized["incidents"] = setting.get("incidents") or []
         return normalized
 
+    @staticmethod
+    def _is_obsolete(job: dict[str, Any]) -> bool:
+        """True for a superseded run that must not count toward results.
+
+        When an openQA job is retriggered the dashboard keeps the older
+        run but marks it superseded — either with an ``obsolete`` flag or
+        an ``"obsoleted"`` result. Both must be dropped so a stale failure
+        does not poison the install verdict (:meth:`_has_passed_install_jobs`)
+        or surface as a phantom entry in the failed-jobs listing. Mirrors
+        :func:`mtui.data_sources.oqa_search.search.incident_jobs`, which
+        filters ``result == "obsoleted"``; only the current run matters.
+        """
+        return bool(job.get("obsolete")) or job.get("result") == "obsoleted"
+
     @staticmethod
     def _normalize_result(result: str | None) -> bool:
         return result in ("passed", "softfailed")
@@ -429,6 +451,16 @@ def _pretty_print_section(
                         ret.append(f"      {test.ljust(width)}{suffix}\n")
                     else:
                         ret.append(f"      {test.ljust(width)}  [{result}]{suffix}\n")
+        elif problem_keys:
+            # Problem groups exist but none carry a failed/incomplete/
+            # timeout_exceeded job, so `failed_by_group` is empty — their
+            # problems are entirely in the `other` bucket (still-running,
+            # parallel_failed, skipped, ...). The Summary already flags
+            # them; don't claim success (the old bug) and don't print an
+            # empty `Failed jobs:` block.
+            ret.append(
+                "  No failed jobs, but some groups need review (see Summary above).\n"
+            )
         else:
             ret.append("  All jobs passed.\n")
         ret.append("\n")

tests/test_qem_dashboard_connector.py

@@ -759,3 +759,146 @@ def test_has_passed_install_jobs_counts_slfo_jobs(mock_config):
     assert DashboardAutoOpenQA._has_passed_install_jobs(jobs) is True
     failed = [{"test": "qam-incidentinstall-SLFO", "result": "failed"}]
     assert DashboardAutoOpenQA._has_passed_install_jobs(failed) is False
+
+
+# --------------------------------------------------------------------------- #
+# Finding #7: a problem group whose only problem is in the `other` bucket      #
+# (still-running / parallel_failed / ...) must not be reported "All passed".   #
+# --------------------------------------------------------------------------- #
+
+
+def test_pretty_print_parallel_failed_not_reported_as_all_passed(mock_config):
+    """A `parallel_failed` job flags a problem group; the section must not lie.
+
+    `parallel_failed` is not in `FAILED_RESULTS`, so it lands in the `other`
+    bucket and populates no `failed_by_group` entry. The old trailer gated on
+    `failed_by_group` and therefore printed "All jobs passed." even though the
+    Summary listed the group as a problem.
+    """
+    dashboard = _make_dashboard(mock_config)
+    # A clean group (folds into an all-passed row) plus a distinct group whose
+    # only job is parallel_failed (a problem living entirely in `other`).
+    jobs = [_incident_job(4000 + i, f"qam-pass-{i}", "passed") for i in range(3)]
+    jobs.append(
+        _incident_job(4100, "qam-parallel", "parallel_failed", flavor="Other-Flavor")
+    )
+    out = "".join(dashboard._pretty_print(jobs))
+
+    # The parallel_failed job is counted under `other`, so its group is a problem.
+    assert "other: 1" in out
+    # It must NOT claim success while the Summary flags a problem group.
+    assert "All jobs passed." not in out
+    # No failed/incomplete/timeout job -> no (empty) `Failed jobs:` block either.
+    assert "Failed jobs:" not in out
+    # Instead, an explicit needs-review note is emitted.
+    assert "some groups need review" in out
+
+
+def test_pretty_print_running_job_not_reported_as_all_passed(mock_config):
+    """A still-running job (openQA result ``none``) must not read as all-passed."""
+    dashboard = _make_dashboard(mock_config)
+    jobs = [_incident_job(4200 + i, f"qam-pass-{i}", "passed") for i in range(2)]
+    jobs.append(_incident_job(4300, "qam-running", "none"))
+    out = "".join(dashboard._pretty_print(jobs))
+
+    assert "other: 1" in out
+    assert "All jobs passed." not in out
+    assert "some groups need review" in out
+
+
+# --------------------------------------------------------------------------- #
+# Finding #8: obsoleted (superseded) jobs must be dropped from the working set #
+# so a stale failure poisons neither the install verdict nor the listing.     #
+# --------------------------------------------------------------------------- #
+
+
+def test_is_obsolete_flags_superseded_runs():
+    """Both the `obsolete` flag and an `obsoleted` result mark a stale run."""
+    assert DashboardAutoOpenQA._is_obsolete({"obsolete": True}) is True
+    assert DashboardAutoOpenQA._is_obsolete({"result": "obsoleted"}) is True
+    assert (
+        DashboardAutoOpenQA._is_obsolete({"obsolete": False, "result": "failed"})
+        is False
+    )
+    assert DashboardAutoOpenQA._is_obsolete({}) is False
+
+
+@responses.activate
+def test_load_jobs_drops_obsoleted_runs(mock_config):
+    """A retriggered install's stale failed run must not sink the verdict.
+
+    The dashboard returns the superseded run alongside the current one. If it
+    is not filtered, `_has_passed_install_jobs` sees a failed
+    `qam-incidentinstall` and returns False, so `results` is None and the
+    failed run shows up as a phantom failure.
+    """
+    rrid = RequestReviewID("SUSE:Maintenance:12358:199773")
+    responses.add(
+        responses.GET,
+        f"{API}/incidents/12358",
+        json={"number": 12358, "packages": ["bash"], "channels": []},
+        status=200,
+    )
+    responses.add(
+        responses.GET,
+        f"{API}/incident_settings/12358",
+        json=[
+            {
+                "id": 7,
+                "incident": 12358,
+                "version": "15-SP5",
+                "flavor": "Server-DVD-Incidents",
+                "arch": "x86_64",
+                "settings": {"DISTRI": "sle"},
+            }
+        ],
+        status=200,
+    )
+
+    def _install_job(job_id, status, *, obsolete=False):
+        return {
+            "job_id": job_id,
+            "incident_settings": 7,
+            "update_settings": None,
+            "name": "qam-incidentinstall",
+            "job_group": "Maintenance",
+            "group_id": 1,
+            "status": status,
+            "distri": "sle",
+            "flavor": "Server-DVD-Incidents",
+            "arch": "x86_64",
+            "version": "15-SP5",
+            "build": ":12358:bash",
+            "obsolete": obsolete,
+        }
+
+    responses.add(
+        responses.GET,
+        f"{API}/jobs/incident/7",
+        json=[
+            _install_job(1000, "failed", obsolete=True),  # superseded (flag)
+            _install_job(999, "obsoleted"),  # superseded (result marker)
+            _install_job(1001, "passed"),  # the current run
+        ],
+        status=200,
+    )
+    responses.add(
+        responses.GET,
+        f"{API}/update_settings/12358",
+        json=[],
+        status=200,
+    )
+
+    incident = QEMIncident(rrid, API)
+    dashboard = DashboardAutoOpenQA(mock_config, OPENQA_HOST, incident, rrid).run()
+
+    # Only the current (non-superseded) run survives.
+    assert len(dashboard.jobs) == 1
+    assert dashboard.jobs[0]["id"] == 1001
+    # The stale failed run did not poison the install verdict.
+    assert dashboard.results is not None
+    assert len(dashboard.results) == 1
+    # And it is not listed as a phantom failure.
+    pp = "".join(dashboard.pp)
+    assert "Failed jobs:" not in pp
+    assert "obsoleted" not in pp