fix(ci): fail tests-ok when workspace discovery fails

.asf.yaml

@@ -169,9 +169,9 @@ github:
           # protection and rulesets both require exact-match status
           # check names, so listing matrix entries here would force
           # an `.asf.yaml` change on every matrix add / rename /
-          # removal. `tests-ok` `needs:` every matrix entry and
-          # fails unless all succeed, so the gate semantics are the
-          # same.
+          # removal. `tests-ok` requires successful member discovery
+          # and a passing matrix (or a confirmed empty selection
+          # when the matrix is skipped).
           - "tests-ok"
       # TEMPORARILY DISABLED — see the note above. Restore by removing
       # the three leading `# ` markers; the values are the ones that were

.github/workflows/tests.yml

@@ -60,10 +60,12 @@ jobs:
         # - Applicability mirrors `tools/dev/run-workspace-check.sh`: a
         #   member with `[tool.pytest.ini_options]` and no
         #   `[tool.magpie.checks] skip = [..., "pytest"]`.
-        # - Path filter (PRs only): a member is picked when its own
-        #   Python source/test files changed. On push to `main`, and when
-        #   a shared build input changes (root `pyproject.toml`,
-        #   `uv.lock`, the helpers, this workflow), every member is.
+        # - Path filter (PRs only): the helper selects members whose
+        #   eligible files or linked sources changed, plus their
+        #   dependents; `member_changed` documents the exclusions.
+        #   On push to `main`, and when a shared build input changes
+        #   (root `pyproject.toml`, `uv.lock`, the helpers, this workflow),
+        #   every member is selected.
         # - Grouping: most of a job is setup, so up to six members get a
         #   job each (named after the member, as before); more are split
         #   into at most eight jobs balanced by test size.
@@ -95,16 +97,15 @@ jobs:
   # pull request touching a few projects gets one job per project, so
   # per-project pass/fail shows in the checks list; a full run batches
   # them, and each batch job prints a group and a summary row per
-  # member. The `prek` workflow still exercises pytest as a hook, so
-  # this workflow is the visible signal — not the gate.
+  # member. The required `tests-ok` job below aggregates discovery
+  # and matrix results; the `prek` workflow skips workspace pytest.
   pytest:
     name: "pytest (${{ matrix.batch.name }})"
     runs-on: ubuntu-slim
     needs: members
     # Guard against an empty selection. When the path filter selects no
-    # members (a PR that changed no project's Python), the batches output
-    # is `[]`; feeding an empty array to `matrix` makes GitHub mark this
-    # job `failure` ("matrix vector does not contain any values"), not
+    # members, the batches output is `[]`. An empty `matrix` makes GitHub
+    # mark this job `failure` ("matrix vector does not contain any values"), not
     # `skipped` — which would fail `tests-ok`. Skipping the job on an
     # empty list yields the `skipped` result `tests-ok` treats as a pass.
     if: ${{ needs.members.outputs.batches != '[]' }}
@@ -204,28 +205,40 @@ jobs:
   # with N entries would otherwise force N `.asf.yaml` lines that
   # must be kept in lock-step with the matrix. This single
   # `tests-ok` job is the only pytest-related context required by
-  # branch protection — it passes iff every entry in the `pytest`
-  # matrix above passed. Adding, renaming, or removing matrix
-  # entries no longer touches `.asf.yaml`.
+  # branch protection — it requires successful member discovery and
+  # a passing matrix (or a confirmed empty selection). Matrix changes
+  # no longer require `.asf.yaml` updates.
   tests-ok:
     name: tests-ok
     runs-on: ubuntu-slim
-    needs: pytest
+    needs: [members, pytest]
     # `always()` — without it, this job would be skipped (not
     # failed) when an upstream matrix entry fails, and a skipped
-    # required-check is treated as missing by branch protection,
-    # which leaves the PR mergeable. Run unconditionally and assert
-    # on the result instead.
+    # required check does not block merging. Run unconditionally and
+    # assert on discovery as well: its failure skips the matrix too.
     if: always()
     steps:
-      - name: Assert all pytest matrix entries succeeded
+      - name: Assert member discovery and pytest matrix results
+        # Explicit, so the runner uses the same flags the gate test runs it
+        # with (`--noprofile --norc -eo pipefail`), not the `bash -e` default.
+        shell: bash
+        env:
+          BATCHES: ${{ needs.members.outputs.batches }}
         # `success` — every selected member passed.
-        # `skipped` — the path filter selected no members (the PR
-        #   changed no project's Python source/test files), so there
-        #   was nothing to run; that is a pass, not a gap.
+        # `skipped` — accepted only after successful discovery with an
+        #   explicitly empty batches output, confirming nothing was selected.
         # Anything else (`failure` / `cancelled`) fails the gate.
         run: |
+          members_result="${{ needs.members.result }}"
+          if [ "${members_result}" != "success" ]; then
+            echo "workspace member discovery result: ${members_result}"
+            exit 1
+          fi
           result="${{ needs.pytest.result }}"
+          if [ "${result}" = "skipped" ] && [ "${BATCHES:-}" != "[]" ]; then
+            echo "pytest matrix skipped without an empty selection"
+            exit 1
+          fi
           if [ "${result}" != "success" ] && [ "${result}" != "skipped" ]; then
             echo "pytest matrix result: ${result}"
             exit 1

tools/dev/tests/test_ci_tests_gate.py

@@ -0,0 +1,285 @@
+# SPDX-License-Identifier: Apache-2.0
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Exercise the actual tests-ok shell gate, including discovery failures.
+
+The field reader supports literal needs lists (inline or block), scalar
+conditions, quotes, and comments. It is not a general YAML parser: aliases,
+tags, and folded scalars are outside this workflow test's supported syntax.
+"""
+
+from __future__ import annotations
+
+import os
+import re
+import shlex
+import shutil
+import subprocess
+from pathlib import Path
+from textwrap import dedent
+from typing import NoReturn
+
+import pytest
+
+_WORKFLOW = Path(__file__).resolve().parents[3] / ".github/workflows/tests.yml"
+_RESULTS = ("success", "failure", "cancelled", "skipped")
+_BASH_FLAGS = ("--noprofile", "--norc", "-e", "-o", "pipefail")
+
+
+def _field(job: str, name: str, indent: int = 4) -> str:
+    lines = job.splitlines()
+    for index, line in enumerate(lines):
+        match = re.fullmatch(rf" {{{indent}}}{re.escape(name)}:[ \t]*(.*)", line)
+        if match is None:
+            continue
+        value = [match[1]]
+        for following in lines[index + 1 :]:
+            stripped = following.lstrip()
+            if not stripped or stripped.startswith("#"):
+                continue
+            depth = len(following) - len(stripped)
+            if depth < indent or (depth == indent and not stripped.startswith("- ")):
+                break
+            value.append(stripped)
+        return "\n".join(value)
+    raise AssertionError(f"tests-ok is missing {name}")
+
+
+def _tokens(value: str, sequence: bool = False) -> list[str]:
+    lexer = shlex.shlex(value, posix=True)
+    lexer.whitespace_split = True
+    if sequence:
+        lexer.whitespace += ",[]"
+    return list(lexer)
+
+
+def _expression(value: str, require_wrapper: bool = False) -> str:
+    value = " ".join(_tokens(value)).strip()
+    wrapped = value.startswith("${{") and value.endswith("}}")
+    if require_wrapper:
+        assert wrapped, "the environment value must evaluate an Actions expression"
+    if wrapped:
+        value = value[3:-2].strip()
+    return value
+
+
+def _assert_dependencies(job: str) -> None:
+    needs = [token for token in _tokens(_field(job, "needs"), sequence=True) if token != "-"]
+    assert set(needs) == {"members", "pytest"}
+    assert _expression(_field(job, "if")) == "always()"
+
+
+def _unusable_bash(reason: str) -> NoReturn:
+    # Skip only off CI: on the Linux runner a broken Bash must fail, or every
+    # gate test would quietly skip and the required check would stay green.
+    message = f"the workflow gate needs a working Bash ({reason})"
+    if os.environ.get("CI"):
+        pytest.fail(message)
+    pytest.skip(message)
+
+
+def _working_bash() -> str:
+    executable = shutil.which("bash")
+    if executable is None:
+        _unusable_bash("not found")
+    try:
+        probe = subprocess.run(
+            [executable, *_BASH_FLAGS, "-c", '[ "${BATCHES:-}" = "[]" ]'],
+            env={**os.environ, "BATCHES": "[]"},
+            capture_output=True,
+            text=True,
+            timeout=5,
+            check=False,
+        )
+    except (OSError, subprocess.TimeoutExpired) as error:
+        _unusable_bash(type(error).__name__)
+    else:
+        if probe.returncode != 0:
+            _unusable_bash(f"probe exit {probe.returncode}")
+    return executable
+
+
+@pytest.fixture(scope="module")
+def gate_job() -> str:
+    # Read only this job's literal block; no YAML dependency is needed.
+    match = re.search(r"(?ms)^  tests-ok:\n(.*?)(?=^  \S|\Z)", _WORKFLOW.read_text(encoding="utf-8"))
+    assert match is not None, "tests-ok job is missing"
+    return match[1]
+
+
+@pytest.fixture(scope="module")
+def bash() -> str:
+    return _working_bash()
+
+
+def test_gate_waits_for_both_jobs_and_runs_after_failure(gate_job: str) -> None:
+    _assert_dependencies(gate_job)
+
+
+def test_gate_reads_batches_from_discovery(gate_job: str) -> None:
+    value = _field(gate_job, "BATCHES", indent=10)
+    assert _expression(value, require_wrapper=True) == "needs.members.outputs.batches"
+
+
+def _run_gate(
+    gate_job: str, bash: str, members_result: str, pytest_result: str, batches: str | None = "[]"
+) -> subprocess.CompletedProcess[str]:
+    # The assert step's own `run: |` block, found by its name, so a step added
+    # before it can never be the script under test.
+    run = re.search(
+        r"(?ms)^      - name: Assert member discovery and pytest matrix results\n.*?^        run: \|\n((?:          .*\n|\n)+)",
+        gate_job,
+    )
+    assert run is not None, "tests-ok shell assertion is missing"
+    script = dedent(run[1])
+    for job, result in (("members", members_result), ("pytest", pytest_result)):
+        script = script.replace("${{ needs." + job + ".result }}", result)
+    assert "${{" not in script, "unresolved Actions expression in the gate"
+    env = os.environ.copy()
+    env.pop("BATCHES", None)
+    if batches is not None:
+        env["BATCHES"] = batches
+    return subprocess.run(
+        [bash, *_BASH_FLAGS, "-c", script],
+        env=env,
+        capture_output=True,
+        text=True,
+        timeout=20,
+        check=False,
+    )
+
+
+def _assert_result(completed: subprocess.CompletedProcess[str], code: int, diagnostic: str) -> None:
+    details = f"exit={completed.returncode}\n{completed.stdout}{completed.stderr}"
+    assert completed.returncode == code, details
+    assert diagnostic in completed.stdout, details
+
+
+@pytest.mark.parametrize("members_result", _RESULTS)
+@pytest.mark.parametrize("pytest_result", _RESULTS)
+def test_gate_results(gate_job: str, bash: str, members_result: str, pytest_result: str) -> None:
+    completed = _run_gate(gate_job, bash, members_result, pytest_result)
+    if members_result != "success":
+        _assert_result(completed, 1, f"workspace member discovery result: {members_result}")
+    elif pytest_result not in ("success", "skipped"):
+        _assert_result(completed, 1, f"pytest matrix result: {pytest_result}")
+    else:
+        _assert_result(completed, 0, f"pytest gate satisfied (matrix result: {pytest_result})")
+
+
+@pytest.mark.parametrize(
+    "batches",
+    [None, "", "[ ]", '[{"name":"dev"}]', "not-json", "$(exit 23)", "`exit 24`"],
+    ids=["missing", "empty", "noncanonical-empty", "nonempty", "malformed", "substitution", "backticks"],
+)
+def test_skipped_matrix_needs_a_confirmed_empty_selection(
+    gate_job: str, bash: str, batches: str | None
+) -> None:
+    completed = _run_gate(gate_job, bash, "success", "skipped", batches)
+    _assert_result(completed, 1, "pytest matrix skipped without an empty selection")
+
+
+@pytest.mark.parametrize("members_result", ["failure", "cancelled", "skipped"])
+def test_discovery_failure_is_reported_before_invalid_skip(
+    gate_job: str, bash: str, members_result: str
+) -> None:
+    completed = _run_gate(gate_job, bash, members_result, "skipped", '[{"name":"dev"}]')
+    _assert_result(completed, 1, f"workspace member discovery result: {members_result}")
+
+
+def test_successful_matrix_accepts_a_nonempty_selection(gate_job: str, bash: str) -> None:
+    completed = _run_gate(gate_job, bash, "success", "success", '[{"name":"dev"}]')
+    _assert_result(completed, 0, "pytest gate satisfied (matrix result: success)")
+
+
+@pytest.mark.parametrize(
+    ("needs", "condition"),
+    [
+        ("[members, pytest]", "always()"),
+        ("[members, pytest]  # comment", "always()"),
+        ("['members', 'pytest']", "always()"),
+        ('["members", "pytest"]', "always()"),
+        ("\n      - members\n      - pytest", "always()"),
+        ("\n      - 'members'\n      - 'pytest'  # comment", "${{ always() }}"),
+        ("[members, pytest]", "${{ always() }}"),
+        ("[members, pytest]", "'${{ always() }}'  # comment"),
+    ],
+)
+def test_equivalent_dependency_declarations(needs: str, condition: str) -> None:
+    _assert_dependencies(f"    needs: {needs}\n    if: {condition}\n")
+
+
+@pytest.mark.parametrize(
+    ("needs", "condition"),
+    [
+        ("[pytest]", "always()"),
+        ("[members]", "always()"),
+        ("[members, pytest]", "success()"),
+        ("[members, pytest]", "al ways()"),
+        ("[members, pytest, other]", "always()"),
+    ],
+)
+def test_incorrect_dependency_declarations_are_rejected(needs: str, condition: str) -> None:
+    with pytest.raises(AssertionError):
+        _assert_dependencies(f"    needs: {needs}\n    if: {condition}\n")
+
+
+@pytest.mark.parametrize("mode", ["missing", "launch-error", "timeout", "nonzero"])
+def test_unusable_bash_is_skipped(monkeypatch: pytest.MonkeyPatch, mode: str) -> None:
+    monkeypatch.delenv("CI", raising=False)
+    monkeypatch.setattr(shutil, "which", lambda name: None if mode == "missing" else "/probe/bash")
+
+    def probe(*args, **kwargs):
+        if mode == "launch-error":
+            raise OSError("cannot launch Bash")
+        if mode == "timeout":
+            raise subprocess.TimeoutExpired("bash", 5)
+        return subprocess.CompletedProcess(args[0], 1, "", "WSL launcher unavailable")
+
+    monkeypatch.setattr(subprocess, "run", probe)
+    with pytest.raises(pytest.skip.Exception, match="needs a working Bash"):
+        _working_bash()
+
+
+def test_bash_probe_skips_shell_without_gate_environment(bash: str, monkeypatch: pytest.MonkeyPatch) -> None:
+    monkeypatch.delenv("CI", raising=False)
+    monkeypatch.setattr(shutil, "which", lambda name: bash)
+    run = subprocess.run
+
+    def without_batches(*args, **kwargs):
+        env = kwargs.get("env", os.environ).copy()
+        env.pop("BATCHES", None)
+        return run(*args, **{**kwargs, "env": env})
+
+    monkeypatch.setattr(subprocess, "run", without_batches)
+    with pytest.raises(pytest.skip.Exception, match="needs a working Bash"):
+        _working_bash()
+
+
+def test_unusable_bash_fails_on_ci(monkeypatch: pytest.MonkeyPatch) -> None:
+    monkeypatch.setenv("CI", "true")
+    monkeypatch.setattr(shutil, "which", lambda name: None)
+    with pytest.raises(pytest.fail.Exception, match="needs a working Bash"):
+        _working_bash()
+
+
+def test_gate_step_runs_with_the_tested_shell_flags(gate_job: str) -> None:
+    step = gate_job.split("- name: Assert member discovery and pytest matrix results", 1)[1]
+    assert re.search(r"(?m)^        shell: bash\s*$", step), (
+        "the gate step must set `shell: bash` to match _BASH_FLAGS"
+    )