diff --git a/.asf.yaml b/.asf.yaml index a1e7489b4..3b432d413 100644 --- a/.asf.yaml +++ b/.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 diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index bc50a534f..f07174dda 100644 --- a/.github/workflows/tests.yml +++ b/.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,37 @@ 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 + 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 diff --git a/tools/dev/tests/test_ci_tests_gate.py b/tools/dev/tests/test_ci_tests_gate.py new file mode 100644 index 000000000..2860b437f --- /dev/null +++ b/tools/dev/tests/test_ci_tests_gate.py @@ -0,0 +1,254 @@ +# 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 + +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 _working_bash() -> str: + executable = shutil.which("bash") + if executable is None: + pytest.skip("the workflow gate needs a working 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: + pytest.skip(f"the workflow gate needs a working Bash ({type(error).__name__})") + else: + if probe.returncode != 0: + pytest.skip(f"the workflow gate needs a working Bash (probe exit {probe.returncode})") + return executable + + +@pytest.fixture +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]: + run = re.search(r"(?m)^ 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.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.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()