Add checks for test id length and content (#21)

Author: ssbarnea

.github/workflows/tox.yml

@@ -1,9 +1,7 @@
 ---
 name: tox
+
 on:
-  create: # is used for publishing to PyPI and TestPyPI
-    tags: # any tag regardless of its name, no branches
-      - "**"
   push: # only publishes pushes to the main branch to TestPyPI
     branches: # any integration branch but not tag
       - "main"
@@ -38,6 +36,7 @@ jobs:
             devel
   build:
     name: ${{ matrix.name }}
+    environment: test
     runs-on: ${{ matrix.os || 'ubuntu-22.04' }}
     needs:
       - prepare
@@ -48,7 +47,7 @@ jobs:
       fail-fast: false
       matrix: ${{ fromJson(needs.prepare.outputs.matrix) }}
     env:
-      PYTEST_REQPASS: 1
+      PYTEST_REQPASS: 4
 
     steps:
       - uses: actions/checkout@v4

README.md

@@ -33,6 +33,20 @@ files, that makes it harder to identify and copy/paste the test name in order
 to reproduce the failure locally. That is why this plugin forces its users to
 avoid having the same function name anywhere in the tested project.
 
+You can disable this check by defining `PYTEST_CHECK_TEST_DUPLICATE=0`.
+
+## Avoiding problematic test identifiers
+
+This plugin will raise errors when it encounters test IDs that are either too
+long or that contain unsafe characters. While pytest is very flexible in allowing
+a wide range of test IDs, using these does make development harder as it prevents
+people from doing a copy/paste with failed test and pasting in in their terminal
+to reproduce the failed test locally.
+
+You can disable regex check by defining `PYTEST_CHECK_TEST_ID_REGEX=0`.
+
+You can disable the length check by defining `PYTEST_MAX_TEST_ID_LENGTH=0`.
+
 ## Release process
 
 Releases are triggered from [GitHub Releases](https://github.com/pytest-dev/pytest-plus/releases)

pyproject.toml

@@ -41,7 +41,7 @@ classifiers = [
   "Topic :: Utilities",
 ]
 keywords = ["testing", "pytest", "plugin"]
-dependencies = ["pytest>=6.0.1"]
+dependencies = ["pytest>=7.4.2"]
 
 [project.urls]
 homepage = "https://github.com/pytest-dev/pytest-plus"
@@ -55,15 +55,22 @@ test = ["coverage>=7.0.0", "pytest-html"]
 plus = "pytest_plus"
 
 [tool.coverage.run]
-omit = ["tests/*"]
+omit = ["test/*", "/private/var/folders/*", "/tmp/*"]
 
 [tool.coverage.report]
-fail_under = 81
-omit = ["tests/*"]
+fail_under = 100
+omit = ["test/*", "/private/var/folders/*", "/tmp/*"]
+show_missing = true
 
 [tool.pytest.ini_options]
 addopts = "-p no:flaky"
 
+filterwarnings = [
+  "error",
+  "ignore:ast.(Str|Num|NameConstant) is deprecated and will be removed in Python 3.14:DeprecationWarning:_pytest.assertion.rewrite",
+  "ignore:Attribute s is deprecated and will be removed in Python 3.14:DeprecationWarning:_pytest.assertion.rewrite",
+]
+
 [tool.ruff]
 ignore = [
   "D203", # incompatible with D211

requirements.txt

@@ -4,14 +4,14 @@
 #
 #    pip-compile --extra=test --no-annotate --output-file=requirements.txt --strip-extras pyproject.toml
 #
-attrs==22.1.0
-coverage==7.0.0
-exceptiongroup==1.0.4
-iniconfig==1.1.1
-packaging==22.0
-pluggy==1.0.0
-py==1.11.0
-pytest==7.2.0
-pytest-html==3.2.0
-pytest-metadata==2.0.4
+coverage==7.3.2
+exceptiongroup==1.1.3
+iniconfig==2.0.0
+jinja2==3.1.2
+markupsafe==2.1.3
+packaging==23.2
+pluggy==1.3.0
+pytest==7.4.2
+pytest-html==4.0.2
+pytest-metadata==3.0.0
 tomli==2.0.1

src/pytest_plus/__init__.py

@@ -1,10 +1,26 @@
 """PyTest Config File."""
+from __future__ import annotations
 
 import os
+import re
 
 import pytest
 from _pytest.terminal import TerminalReporter
 
+PYTEST_CHECK_TEST_DUPLICATE = int(os.environ.get("PYTEST_CHECK_TEST_DUPLICATE", "1"))
+
+
+def get_max_test_id_length() -> int:
+    """Return max test id length."""
+    return int(os.environ.get("PYTEST_MAX_TEST_ID_LENGTH", "60"))
+
+
+def get_test_id_regex() -> None | re.Pattern[str]:
+    """Return regex to use for checking test ids."""
+    if int(os.environ.get("PYTEST_CHECK_TEST_ID_REGEX", "1")):
+        return re.compile(r"^\w+$")
+    return None
+
 
 def pytest_sessionfinish(session: pytest.Session) -> None:
     """Assure passed test match PYTEST_REQPASS value.
@@ -15,7 +31,7 @@ def pytest_sessionfinish(session: pytest.Session) -> None:
     """
     terminalreporter = session.config.pluginmanager.get_plugin("terminalreporter")
     if not isinstance(terminalreporter, TerminalReporter):
-        raise TypeError
+        raise TypeError  # pragma: no cover
     req_passed = int(os.environ.get("PYTEST_REQPASS", "0"))
     if req_passed and not session.config.option.collectonly:
         passed = 0
@@ -35,14 +51,26 @@ def pytest_collection_modifyitems(items: list[pytest.Item]) -> None:
     """Ensure testing fails if tests have duplicate names."""
     errors = []
     names = {}
+    max_test_id_length = get_max_test_id_length()
+    test_id_regex = get_test_id_regex()
     for item in items:
         base_name = item.name.split("[")[0]
         if base_name not in names:
             names[base_name] = item.location
-        elif item.location[:2] != names[base_name][:2]:
+        elif item.location[:2] != names[base_name][:2] and PYTEST_CHECK_TEST_DUPLICATE:
             error = f"Duplicate test name '{base_name}', found at {item.location[0]}:{item.location[1]} and {names[base_name][0]}:{names[base_name][1]}"
             if error not in errors:
                 errors.append(error)
+        if hasattr(item, "callspec"):
+            test_id = item.callspec.id
+            if max_test_id_length and len(test_id) > max_test_id_length:
+                errors.append(
+                    f"{item} has an id that looks above {max_test_id_length} characters.",
+                )
+            elif test_id_regex and not test_id_regex.match(test_id):
+                errors.append(
+                    f"Test {item} has an id that does not match our safe pattern '{test_id_regex.pattern}' for use with a terminal.",
+                )
     if errors:
         msg = f"Failed run due to following issues being identified:\n{os.linesep.join(errors)}"
         raise pytest.UsageError(

test/test_plugin.py

@@ -1,4 +1,6 @@
 """Tests."""
+import os
+
 import pytest
 
 pytest_plugins = ["pytester"]
@@ -29,3 +31,59 @@ def test_a():
         == "Duplicate test name 'test_a', found at test_two.py:0 and test_one.py:0"
     )
     assert result.ret == pytest.ExitCode.USAGE_ERROR
+
+
+@pytest.mark.parametrize(
+    ("rc", "disable"),
+    [
+        pytest.param(pytest.ExitCode.USAGE_ERROR, False, id="0"),
+        pytest.param(pytest.ExitCode.OK, True, id="1"),
+    ],
+)
+def test_check_test_id(pytester: pytest.Pytester, rc: int, *, disable: bool) -> None:
+    """Validates that we can detect duplicate test names."""
+    if disable:
+        os.environ["PYTEST_CHECK_TEST_ID_REGEX"] = "0"
+    p1 = pytester.makepyfile(
+        test_one="""
+            import pytest
+
+            @pytest.mark.parametrize(
+                "some",
+                (pytest.param("", id="invalid name"),),
+            )
+            def test_a(some: str):
+                assert True
+            """,
+    )
+
+    result = pytester.runpytest_inprocess("--collect-only", p1)
+    if not disable:
+        assert (
+            "Test <Function test_a[invalid name]> has an id that does not match our safe pattern '^\\w+$' for use with a terminal."
+            in result.stderr.lines
+        )
+    assert result.ret == rc
+
+
+def test_check_test_id_length(pytester: pytest.Pytester) -> None:
+    """Validates that we can detect duplicate test names."""
+    p1 = pytester.makepyfile(
+        test_one="""
+            import pytest
+
+            @pytest.mark.parametrize(
+                "some",
+                (pytest.param("", id="this-is-too-long-for-our-taste-so-we-ask-you-to-make-it-shorter"),),
+            )
+            def test_a(some: str):
+                assert True
+            """,
+    )
+
+    result = pytester.runpytest_inprocess("--collect-only", p1)
+    assert (
+        "<Function test_a[this-is-too-long-for-our-taste-so-we-ask-you-to-make-it-shorter]> has an id that looks above 60 characters."
+        in result.stderr.lines
+    )
+    assert result.ret == pytest.ExitCode.USAGE_ERROR

tox.ini

@@ -18,10 +18,11 @@ commands_pre =
 commands =
     pytest --collect-only
     # Do not use PYTEST_REQPASS as here, read README.md for details
-    sh -c "PYTEST_REQPASS=1 coverage run -m pytest --color=yes --html={envlogdir}/reports.html --self-contained-html {tty:-s}"
+    sh -c "PYTEST_REQPASS=4 coverage run -m pytest --color=yes --html={envlogdir}/reports.html --self-contained-html {tty:-s}"
     # We want to fail if result code is zero:
     sh -c "PYTEST_REQPASS=100 pytest >/dev/null && exit 1 || true"
     sh -c "coverage combine -a -q --data-file=.coverage .tox/.coverage.*"
+    sh -c "COVERAGE_FILE=.coverage python3 -m coverage report"
 deps =
     -e .[test]
     devel: git+https://github.com/pytest-dev/pytest.git