diff --git a/test/pylib/skip_reason_plugin.py b/test/pylib/skip_reason_plugin.py index 5af173d14c..5b9f4d5457 100644 --- a/test/pylib/skip_reason_plugin.py +++ b/test/pylib/skip_reason_plugin.py @@ -23,7 +23,6 @@ convenience wrappers from :mod:`test.pylib.skip_types`:: from __future__ import annotations -import warnings from collections.abc import Callable from enum import StrEnum @@ -114,15 +113,14 @@ class SkipReasonPlugin: item.stash[SKIP_TYPE_KEY] = str(st) item.stash[SKIP_REASON_KEY] = reason - # Warn on bare pytest.mark.skip not added by typed markers. + # Reject bare pytest.mark.skip not added by typed markers. # skip_mode sets SKIP_TYPE_KEY before this hook runs (trylast). if SKIP_TYPE_KEY not in item.stash: bare = [self._get_reason(m) for m in item.iter_markers("skip")] if bare: alternatives = ", ".join( f"@pytest.mark.{st.marker_name}" for st in self._skip_types) - # TODO: Change to pytest.fail() after full migration. - warnings.warn( + raise pytest.UsageError( f"Untyped skip on {item.nodeid}: {'; '.join(bare)}. " f"Use {alternatives} instead.", ) diff --git a/test/pylib_test/test_no_bare_skips.py b/test/pylib_test/test_no_bare_skips.py new file mode 100644 index 0000000000..a113c8775f --- /dev/null +++ b/test/pylib_test/test_no_bare_skips.py @@ -0,0 +1,87 @@ +# +# Copyright (C) 2026-present ScyllaDB +# +# SPDX-License-Identifier: LicenseRef-ScyllaDB-Source-Available-1.1 +# + +"""Codebase guard tests: verify no bare pytest.skip usage remains. + +These are project-specific tests that scan the ScyllaDB test tree +for bare @pytest.mark.skip decorators and bare pytest.skip() calls. +Any new skip must use the typed markers or the typed skip() helper. +""" + +import ast +import os +import pathlib +import subprocess +import sys + +_TEST_ROOT = pathlib.Path(__file__).resolve().parent.parent + +def _iter_test_py_files(): + """Yield all .py files under test/ excluding pylib_test/, pylib/ and __pycache__.""" + for p in sorted(_TEST_ROOT.rglob("*.py")): + rel = p.relative_to(_TEST_ROOT) + parts = rel.parts + if "__pycache__" in parts: + continue + if parts[0] in ("pylib_test", "pylib"): + continue + yield p + + +def test_no_bare_pytest_skip_calls_in_codebase(): + """Verify no test files use bare pytest.skip() (must use typed skip() helper).""" + violations = [] + for path in _iter_test_py_files(): + source = path.read_text() + try: + tree = ast.parse(source, filename=str(path)) + except SyntaxError: + continue + for node in ast.walk(tree): + if not isinstance(node, ast.Call): + continue + func = node.func + if (isinstance(func, ast.Attribute) and func.attr == "skip" + and isinstance(func.value, ast.Name) + and func.value.id == "pytest"): + violations.append(f" {path}:{node.lineno}") + assert not violations, ( + "Found bare pytest.skip() — use the typed skip() helper instead " + "(from test.pylib.skip_reason_plugin import skip):\n" + + "\n".join(violations) + ) + + +def test_no_bare_skip_markers_in_collection(): + """Collect all real Python tests and verify no bare @pytest.mark.skip exists. + + The skip_reason_plugin raises pytest.UsageError during collection + if any bare skip decorator is found, so --collect-only is enough. + No Scylla binary is needed — only Python collection. + """ + + # When running under xdist (-n), workers inherit PYTEST_XDIST_WORKER. + # If the subprocess inherits it, the runner plugin thinks it is a + # worker and skips creating the log directory — causing a + # FileNotFoundError. Strip xdist env vars so the subprocess runs + # as a standalone main process. + env = {k: v for k, v in os.environ.items() + if not k.startswith("PYTEST_XDIST")} + result = subprocess.run( + [sys.executable, "-m", "pytest", + "--collect-only", + "--ignore=boost", "--ignore=raft", + "--ignore=ldap", "--ignore=vector_search", + "-p", "no:sugar"], + capture_output=True, text=True, + cwd=str(_TEST_ROOT), + env=env, + ) + # If a bare skip exists, plugin raises UsageError → non-zero exit. + assert result.returncode == 0, ( + "Collection failed — a bare @pytest.mark.skip was found.\n" + + result.stdout + result.stderr + ) \ No newline at end of file diff --git a/test/pylib_test/test_skip_reason_plugin.py b/test/pylib_test/test_skip_reason_plugin.py index 878a9d99f0..ec0df70603 100644 --- a/test/pylib_test/test_skip_reason_plugin.py +++ b/test/pylib_test/test_skip_reason_plugin.py @@ -102,27 +102,26 @@ def test_missing_reason_is_rejected(skippytest, marker): assert result.ret != 0 -# -- Bare skip warning ------------------------------------------------------ +# -- Bare skip rejection ----------------------------------------------------- -def test_bare_skip_warns_and_lists_alternatives(skippytest): - """Bare skip must warn and list all typed alternatives.""" +def test_bare_skip_rejected_and_lists_alternatives(skippytest): + """Bare skip must be rejected with UsageError listing all typed alternatives.""" skippytest.makepyfile(""" import pytest @pytest.mark.skip(reason="some bare reason") def test_bare(): pass """) - result = skippytest.runpytest("-W", "all") - result.assert_outcomes(skipped=1) - out = result.stdout.str() - assert "Untyped skip" in out - assert "some bare reason" in out + result = skippytest.runpytest() + result.stderr.fnmatch_lines(["*Untyped skip*some bare reason*"]) + assert result.ret != 0 + out = result.stderr.str() for m in ("skip_bug", "skip_not_implemented", "skip_slow", "skip_env"): - assert m in out, f"expected '{m}' in warning output" + assert m in out, f"expected '{m}' in error output" -def test_bare_skip_in_pytest_param_warns(skippytest): +def test_bare_skip_in_pytest_param_rejected(skippytest): skippytest.makepyfile(""" import pytest @pytest.mark.parametrize("x", [ @@ -133,21 +132,21 @@ def test_bare_skip_in_pytest_param_warns(skippytest): def test_p(x): pass """) - result = skippytest.runpytest("-W", "all") - result.assert_outcomes(passed=1, skipped=1) - assert "Untyped skip" in result.stdout.str() + result = skippytest.runpytest() + result.stderr.fnmatch_lines(["*Untyped skip*bare in param*"]) + assert result.ret != 0 -def test_typed_skip_does_not_warn(skippytest): +def test_typed_skip_does_not_reject(skippytest): skippytest.makepyfile(""" import pytest @pytest.mark.skip_bug(reason="scylladb/scylladb#11111") def test_typed(): pass """) - result = skippytest.runpytest("-W", "error::UserWarning") + result = skippytest.runpytest() result.assert_outcomes(skipped=1) - assert "Untyped skip" not in result.stdout.str() + assert "Untyped skip" not in result.stderr.str() # -- Runtime skip helper ---------------------------------------------------- @@ -297,8 +296,8 @@ def test_skip_mode_prefix_populates_junit(skippytest, tmp_path): assert "not supported in release" in xml -def test_bare_skip_with_skip_mode_no_warn(skippytest): - """When skip_mode uses skip_marker(), bare-skip warning is suppressed +def test_bare_skip_with_skip_mode_no_rejection(skippytest): + """When skip_mode uses skip_marker(), bare-skip rejection is suppressed for the item even if it also has a bare @pytest.mark.skip. The skip_marker() call signals the item already has a typed skip. """ @@ -310,7 +309,6 @@ def test_bare_skip_with_skip_mode_no_warn(skippytest): def test_both_bare_and_mode(): assert False """) - result = skippytest.runpytest("-W", "all") + result = skippytest.runpytest() result.assert_outcomes(skipped=1) - out = result.stdout.str() - assert "Untyped skip" not in out + assert "Untyped skip" not in result.stderr.str()