test: classify every skip in eight files as unproven or not-applicable

AGENTS.md

@@ -389,6 +389,7 @@ Register warning filters in `src/vip/plugin.py::pytest_configure` (via `config.a
 -   Adding a Workbench scenario that ends the shared auth session (sign-out, session revocation, password change) without ordering it last *and* restoring the session afterwards. Under `--interactive-auth` / `--headless-auth` every Workbench scenario shares one account, so ending that session breaks every scenario still running on other xdist workers, plus the cached auth session on disk. `test_workbench_signout` is the worked example.
 -   Creating `.py` step files without a matching `.feature` file (or vice versa).
 -   Forgetting the `@connect`/`@workbench`/`@package_manager` tag in feature files (breaks auto-skip).
+-   Adding a bare `pytest.skip()` to a file listed in `selftests/test_skip_triage.py`. Every skip in those files has been deliberately classified, and `test_skip_triage.py` fails the build if a new unclassified one appears — use `attest.unproven()` or `attest.not_applicable()`.
 -   Reaching for a bare `pytest.skip()` when the real situation is "I could not check this". That is the failure mode #616 exists to close: an unverified deployment reporting itself as a passing one. If the product was configured and you still could not run the check, use `vip.attest.unproven()`.
 -   Using non-conventional PR titles (must be `type: description`).
 -   Relying on multi-line formatting to shorten lines -- `ruff format` will collapse list comprehensions back to one line if they fit within 100 chars. Extract a helper function instead.

selftests/test_skip_triage.py

@@ -0,0 +1,122 @@
+"""Guards the skip-classification triage (#616).
+
+A bare ``pytest.skip()`` still means "not applicable", which is the right
+default for the sites nobody has looked at yet. But once a file *has* been
+triaged, every skip in it should say which kind it is out loud -- otherwise
+the next person to add a skip there silently reintroduces the ambiguity the
+triage just removed, and the file quietly drifts back.
+
+Add a file to ``TRIAGED_FILES`` when every skip in it has been deliberately
+classified as ``attest.unproven`` or ``attest.not_applicable``.
+"""
+
+from __future__ import annotations
+
+import re
+from pathlib import Path
+
+import pytest
+
+_SRC = Path(__file__).resolve().parent.parent / "src" / "vip_tests"
+
+# Files whose every skip has been deliberately classified. Growing this list
+# is the point; see #616.
+TRIAGED_FILES = [
+    "connect/test_content_deploy.py",
+    "cross_product/test_ssl.py",
+    "prerequisites/test_versions.py",
+    "workbench/test_ide_launch.py",
+    "workbench/test_jobs.py",
+    "workbench/test_session_capacity.py",
+    "workbench/test_session_capacity_k8s.py",
+    "workbench/test_sessions.py",
+]
+
+# Tolerates whitespace anywhere the parser does, including a newline, so the
+# guard cannot be evaded by reformatting. Scanned over the whole file rather
+# than line by line for the same reason. ``\b`` before ``pytest`` keeps
+# ``request.node.skip(...)`` and similar attribute calls out of it.
+_BARE_SKIP = re.compile(r"\bpytest\s*\.\s*skip\s*\(")
+_IMPORTS_ATTEST = re.compile(
+    r"^\s*(?:from\s+vip\s+import\s+.*\battest\b|import\s+vip\.attest\b)", re.M
+)
+
+
+def find_bare_skips(text: str) -> list[int]:
+    """Line numbers of every bare ``pytest.skip(`` call in *text*.
+
+    A ``pytest.skip`` written inside a comment or docstring counts too. That
+    is a deliberate false positive: it is loud and trivially reworded, whereas
+    missing a real one is silent, and silence is the failure mode this whole
+    guard exists to prevent.
+    """
+    return [text[: m.start()].count("\n") + 1 for m in _BARE_SKIP.finditer(text)]
+
+
+def imports_attest(text: str) -> bool:
+    """True when *text* really imports the helpers, not merely mentions them."""
+    return _IMPORTS_ATTEST.search(text) is not None
+
+
+@pytest.mark.parametrize("relpath", TRIAGED_FILES)
+def test_triaged_file_has_no_unclassified_skip(relpath: str):
+    path = _SRC / relpath
+    assert path.exists(), f"{relpath} moved or was deleted; update TRIAGED_FILES"
+    offenders = [f"{relpath}:{n}" for n in find_bare_skips(path.read_text())]
+    assert not offenders, (
+        "bare pytest.skip() in a triaged file -- say which kind of skip this is "
+        "with attest.unproven() or attest.not_applicable():\n  " + "\n  ".join(offenders)
+    )
+
+
+@pytest.mark.parametrize("relpath", TRIAGED_FILES)
+def test_triaged_file_actually_uses_the_helpers(relpath: str):
+    """Guards against 'triaging' a file by deleting its skips."""
+    text = (_SRC / relpath).read_text()
+    assert imports_attest(text), f"{relpath} is listed as triaged but never imports attest"
+
+
+class TestBareSkipDetection:
+    """The guard's own tests. A guard that is easy to evade is not a guard."""
+
+    def test_finds_a_plain_call(self):
+        assert find_bare_skips("x = 1\npytest.skip('no')\n") == [2]
+
+    def test_finds_a_call_split_across_lines(self):
+        # ruff would not format it this way, but a hand edit can, and a
+        # line-by-line scan misses it entirely.
+        assert find_bare_skips("pytest.\nskip('no')\n") == [1]
+
+    def test_finds_a_call_with_whitespace_around_the_dot(self):
+        assert find_bare_skips("pytest . skip ('no')\n") == [1]
+
+    def test_ignores_the_attest_helpers(self):
+        text = "attest.unproven('a')\nattest.not_applicable('b')\n"
+        assert find_bare_skips(text) == []
+
+    def test_ignores_an_unrelated_skip_attribute(self):
+        assert find_bare_skips("request.node.skip('x')\nself.skip()\n") == []
+
+    def test_reports_every_occurrence_in_order(self):
+        text = "pytest.skip('a')\nx = 2\npytest.skip('b')\n"
+        assert find_bare_skips(text) == [1, 3]
+
+
+class TestAttestImportDetection:
+    """ "attest" appearing anywhere is not evidence the file uses the helpers."""
+
+    def test_accepts_the_real_import(self):
+        assert imports_attest("from vip import attest\n")
+
+    def test_accepts_a_module_import(self):
+        assert imports_attest("import vip.attest\n")
+
+    def test_rejects_a_mere_mention_in_a_comment(self):
+        assert not imports_attest("# remember to use attest here\nimport pytest\n")
+
+    def test_rejects_a_mention_in_a_docstring(self):
+        assert not imports_attest('"""Uses attest for skips."""\nimport pytest\n')
+
+
+def test_triage_list_has_no_duplicates():
+    assert len(TRIAGED_FILES) == len(set(TRIAGED_FILES))

src/vip_tests/connect/test_content_deploy.py

@@ -15,6 +15,7 @@
 import pytest
 from pytest_bdd import scenario, then, when
 
+from vip import attest
 from vip_tests.connect.bundles import _latest_version, build_shiny_bundle_files
 from vip_tests.connect.conftest import _make_tar_gz
 
@@ -190,7 +191,7 @@ def _get_bundle(name: str, connect_client) -> dict[str, str]:
     if name == "vip-plumber-test":
         r_versions = connect_client.r_versions()
         if not r_versions:
-            pytest.skip("No R versions available on Connect — cannot deploy Plumber")
+            attest.not_applicable("No R versions available on Connect — cannot deploy Plumber")
         manifest = json.loads((pathlib.Path(__file__).parent / "plumber_manifest.json").read_text())
         manifest["platform"] = _latest_version(r_versions)
         return {
@@ -201,7 +202,7 @@ def _get_bundle(name: str, connect_client) -> dict[str, str]:
     if name == "vip-quarto-test":
         quarto_versions = connect_client.quarto_versions()
         if not quarto_versions:
-            pytest.skip("No Quarto installations available on Connect")
+            attest.not_applicable("No Quarto installations available on Connect")
         r_versions = connect_client.r_versions()
         manifest: dict = {
             "version": 1,
@@ -226,13 +227,13 @@ def _get_bundle(name: str, connect_client) -> dict[str, str]:
     if name == "vip-shiny-test":
         r_versions = connect_client.r_versions()
         if not r_versions:
-            pytest.skip("No R versions available on Connect — cannot deploy Shiny")
+            attest.not_applicable("No R versions available on Connect — cannot deploy Shiny")
         return build_shiny_bundle_files(r_versions)
 
     if name == "vip-dash-test":
         py_versions = connect_client.python_versions()
         if not py_versions:
-            pytest.skip("No Python versions available on Connect — cannot deploy Dash")
+            attest.not_applicable("No Python versions available on Connect — cannot deploy Dash")
         return {
             "app.py": (
                 'import dash\napp = dash.Dash(__name__)\napp.layout = dash.html.Div("VIP test")\n'
@@ -257,7 +258,7 @@ def _get_bundle(name: str, connect_client) -> dict[str, str]:
     if name == "vip-rmarkdown-test":
         r_versions = connect_client.r_versions()
         if not r_versions:
-            pytest.skip("No R versions available on Connect — cannot deploy R Markdown")
+            attest.not_applicable("No R versions available on Connect — cannot deploy R Markdown")
         # Use the pre-built manifest with the full transitive dependency closure
         # (rmarkdown → knitr → evaluate/highr/xfun/yaml, bslib, stringr, etc.).
         # An incomplete ``packages`` block causes packrat-restore to fail with
@@ -278,7 +279,9 @@ def _get_bundle(name: str, connect_client) -> dict[str, str]:
     if name == "vip-jupyter-test":
         py_versions = connect_client.python_versions()
         if not py_versions:
-            pytest.skip("No Python versions available on Connect — cannot deploy Jupyter Notebook")
+            attest.not_applicable(
+                "No Python versions available on Connect — cannot deploy Jupyter Notebook"
+            )
         notebook_content = json.dumps(
             {
                 "nbformat": 4,
@@ -347,7 +350,7 @@ def _get_bundle(name: str, connect_client) -> dict[str, str]:
     if name == "vip-fastapi-test":
         py_versions = connect_client.python_versions()
         if not py_versions:
-            pytest.skip("No Python versions available on Connect — cannot deploy FastAPI")
+            attest.not_applicable("No Python versions available on Connect — cannot deploy FastAPI")
         return {
             "app.py": (
                 "from fastapi import FastAPI\n"
@@ -445,14 +448,16 @@ def upload_and_deploy(connect_client, deploy_state):
 def link_git_repository(connect_client, deploy_state):
     quarto_versions = connect_client.quarto_versions()
     if not quarto_versions:
-        pytest.skip("No Quarto on Connect — cannot deploy git-backed Quarto document")
+        attest.not_applicable("No Quarto on Connect — cannot deploy git-backed Quarto document")
     # Check that the remote repository is reachable before attempting to link.
     try:
         resp = httpx.head(_GIT_REPO_URL, follow_redirects=True, timeout=10)
         if resp.status_code >= 400:
-            pytest.skip(f"Git repository not reachable (HTTP {resp.status_code}): {_GIT_REPO_URL}")
+            attest.unproven(
+                f"Git repository not reachable (HTTP {resp.status_code}): {_GIT_REPO_URL}"
+            )
     except httpx.TransportError as exc:
-        pytest.skip(f"Git repository not reachable: {exc}")
+        attest.unproven(f"Git repository not reachable: {exc}")
     connect_client.set_repository(
         deploy_state["guid"], _GIT_REPO_URL, branch=_GIT_BRANCH, directory=_GIT_DIRECTORY
     )
@@ -623,7 +628,7 @@ def content_renders_expected_output(connect_client, deploy_state):
     content = connect_client.get_content(deploy_state["guid"])
     url = content.get("content_url", "")
     if not url:
-        pytest.skip("Content URL not available — skipping output verification")
+        attest.unproven("Content URL not available — skipping output verification")
 
     if expected["type"] == "json":
         # Plumber: append the route path and verify JSON response.

src/vip_tests/cross_product/test_ssl.py

@@ -14,9 +14,10 @@
 from urllib.parse import urlparse
 
 import httpx
-import pytest
 from pytest_bdd import parsers, scenarios, then, when
 
+from vip import attest
+
 # ---------------------------------------------------------------------------
 # Scenarios
 # ---------------------------------------------------------------------------
@@ -34,16 +35,16 @@ def check_ssl_cert(product, vip_config):
     product_key = product.lower().replace(" ", "_")
     pc = vip_config.product_config(product_key)
     if not pc.is_configured:
-        pytest.skip(f"{product} is not configured")
+        attest.not_applicable(f"{product} is not configured")
 
     product_url = pc.url
     parsed = urlparse(product_url)
     if parsed.scheme != "https":
-        pytest.skip(f"URL is not HTTPS: {product_url}")
+        attest.not_applicable(f"URL is not HTTPS: {product_url}")
     if vip_config.insecure:
         # The user explicitly disabled certificate verification
         # (tls.insecure=true) — asserting cert validity contradicts that. #268.
-        pytest.skip("tls.insecure=true — certificate validity checks are disabled")
+        attest.not_applicable("tls.insecure=true — certificate validity checks are disabled")
 
     hostname = parsed.hostname
     port = parsed.port or 443
@@ -55,7 +56,7 @@ def check_ssl_cert(product, vip_config):
         # host is unreachable, which is not a certificate finding. Same
         # connect-vs-handshake split as ``_attempt_tls``/``_ConnectError``
         # below: only the TCP connect stage is skip-worthy. #555.
-        pytest.skip(f"Could not connect to {hostname}:{port}: {exc}")
+        attest.unproven(f"Could not connect to {hostname}:{port}: {exc}")
 
     ctx = ssl.create_default_context()
     try:
@@ -149,12 +150,12 @@ def request_http(product, vip_config):
     product_key = product.lower().replace(" ", "_")
     pc = vip_config.product_config(product_key)
     if not pc.is_configured:
-        pytest.skip(f"{product} is not configured")
+        attest.not_applicable(f"{product} is not configured")
 
     product_url = pc.url
     if urlparse(product_url).scheme != "https":
         # No HTTPS endpoint to redirect to on an HTTP-only deployment. See #268.
-        pytest.skip(f"URL is not HTTPS: {product_url}")
+        attest.not_applicable(f"URL is not HTTPS: {product_url}")
     http_url = product_url.replace("https://", "http://", 1)
     parsed = urlparse(http_url)
     if parsed.scheme != "http":
@@ -365,12 +366,12 @@ def attempt_tls_connection(product, vip_config):
     product_key = product.lower().replace(" ", "_")
     pc = vip_config.product_config(product_key)
     if not pc.is_configured:
-        pytest.skip(f"{product} is not configured")
+        attest.not_applicable(f"{product} is not configured")
 
     product_url = pc.url
     parsed = urlparse(product_url)
     if parsed.scheme != "https":
-        pytest.skip(f"URL is not HTTPS: {product_url}")
+        attest.not_applicable(f"URL is not HTTPS: {product_url}")
 
     hostname = parsed.hostname
     port = parsed.port or 443
@@ -396,15 +397,15 @@ def attempt_tls_connection(product, vip_config):
             ),
         }
     except _ConnectError as exc:
-        pytest.skip(f"Could not reach {hostname}:{port}: {exc}")
+        attest.unproven(f"Could not reach {hostname}:{port}: {exc}")
 
     unsupported = [
         label
         for label, key in (("TLS 1.0", "tls1_0"), ("TLS 1.1", "tls1_1"), ("TLS 1.2", "tls1_2"))
         if results[key]["status"] == "client_unsupported"
     ]
     if unsupported:
-        pytest.skip(
+        attest.unproven(
             f"Runner cannot configure {', '.join(unsupported)} — cannot "
             f"assess server TLS enforcement on this client."
         )

src/vip_tests/prerequisites/test_versions.py

@@ -2,9 +2,10 @@
 
 from __future__ import annotations
 
-import pytest
 from pytest_bdd import given, scenario, then, when
 
+from vip import attest
+
 
 @scenario("test_versions.feature", "Connect version matches configuration")
 def test_connect_version():
@@ -33,9 +34,9 @@ def test_package_manager_version():
 )
 def connect_version_configured(vip_config):
     if not vip_config.connect.is_configured:
-        pytest.skip("Connect is not configured")
+        attest.not_applicable("Connect is not configured")
     if not vip_config.connect.version:
-        pytest.skip(
+        attest.not_applicable(
             "No Connect version configured in vip.toml — set connect.version to enable this check"
         )
     return vip_config.connect.version
@@ -47,9 +48,9 @@ def connect_version_configured(vip_config):
 )
 def pm_version_configured(vip_config):
     if not vip_config.package_manager.is_configured:
-        pytest.skip("Package Manager is not configured")
+        attest.not_applicable("Package Manager is not configured")
     if not vip_config.package_manager.version:
-        pytest.skip(
+        attest.not_applicable(
             "No Package Manager version configured in vip.toml — "
             "set package_manager.version to enable this check"
         )
@@ -61,7 +62,7 @@ def fetch_connect_version(connect_client):
     info = connect_client.server_settings()
     version = info.get("version")
     if not version:
-        pytest.skip("Connect server_settings did not return a version field")
+        attest.unproven("Connect server_settings did not return a version field")
     return version
 
 
@@ -70,7 +71,7 @@ def fetch_pm_version(pm_client):
     info = pm_client.status()
     version = info.get("version")
     if not version:
-        pytest.skip("Package Manager status endpoint did not return a version field")
+        attest.unproven("Package Manager status endpoint did not return a version field")
     return version