Change-detectors, tautologies, source-reading tests, redundant duplicates, mock-echo tests and dead/unrunnable tests. Per-test rationale in the lane ledger (category + reason for every removal).
152 lines
5.4 KiB
Python
152 lines
5.4 KiB
Python
"""Tests for the ``subprocess text=True without explicit encoding=`` footgun
|
|
rule in ``scripts/check-windows-footguns.py``.
|
|
|
|
This rule (added alongside PR #60741) catches ``subprocess.run/Popen/call/
|
|
check_output/check_call(..., text=True, ...)`` calls that don't pass an
|
|
explicit ``encoding=``. On Chinese Windows (cp936/GBK) and other non-UTF-8
|
|
default codepages, ``text=True`` without ``encoding=`` decodes child output
|
|
with ``locale.getpreferredencoding(False)`` and crashes ``_readerthread``
|
|
with ``UnicodeDecodeError`` on non-default-codepage bytes.
|
|
|
|
See issues #47939, #53428, #57238.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import importlib.util
|
|
import sys
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
REPO_ROOT = Path(__file__).resolve().parents[2]
|
|
LINTER_PATH = REPO_ROOT / "scripts" / "check-windows-footguns.py"
|
|
|
|
|
|
def _load_linter_module():
|
|
"""Import the linter script as a module (it's not a package).
|
|
|
|
Register the module in sys.modules BEFORE exec_module so that
|
|
``@dataclass`` can resolve ``cls.__module__`` via
|
|
``sys.modules.get(cls.__module__).__dict__`` (CPython 3.11+ dataclass
|
|
internals require this).
|
|
"""
|
|
spec = importlib.util.spec_from_file_location("check_windows_footguns", LINTER_PATH)
|
|
mod = importlib.util.module_from_spec(spec)
|
|
sys.modules["check_windows_footguns"] = mod
|
|
spec.loader.exec_module(mod)
|
|
return mod
|
|
|
|
|
|
@pytest.fixture(scope="module")
|
|
def linter():
|
|
return _load_linter_module()
|
|
|
|
|
|
def _find_footgun(linter, name: str):
|
|
"""Locate a Footgun by name in the FOOTGUNS list."""
|
|
for fg in linter.FOOTGUNS:
|
|
if fg.name == name:
|
|
return fg
|
|
pytest.fail(f"Footgun rule '{name}' not found in FOOTGUNS")
|
|
|
|
|
|
def _scan_line(linter, line: str, footgun_name: str) -> bool:
|
|
"""Return True if the given line triggers the named footgun rule.
|
|
|
|
Uses the linter's own pattern + post_filter logic so the test exercises
|
|
the real detection path (including guard-hint and suppression checks).
|
|
"""
|
|
fg = _find_footgun(linter, footgun_name)
|
|
# Replicate the relevant checks from scan_file(): suppression marker,
|
|
# guard hints, then pattern + post_filter.
|
|
if linter.SUPPRESS_MARKER.search(line):
|
|
return False
|
|
if any(hint in line for hint in linter.GUARD_HINTS):
|
|
return False
|
|
code = linter._strip_code(line)
|
|
if not code.strip():
|
|
return False
|
|
match = fg.pattern.search(code)
|
|
if not match:
|
|
return False
|
|
if fg.post_filter is not None:
|
|
try:
|
|
if not fg.post_filter(match, line):
|
|
return False
|
|
except (IndexError, AttributeError):
|
|
return False
|
|
return True
|
|
|
|
|
|
RULE_NAME = "subprocess text=True without explicit encoding="
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Detection — these SHOULD be flagged
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
class TestDetection:
|
|
def test_flags_subprocess_check_output_text_true(self, linter):
|
|
line = ' out = subprocess.check_output(["git", "status"], text=True)'
|
|
assert _scan_line(linter, line, RULE_NAME)
|
|
|
|
|
|
def test_flags_text_with_spaces_around_equals(self, linter):
|
|
line = ' subprocess.run(cmd, text = True, timeout=10)'
|
|
assert _scan_line(linter, line, RULE_NAME)
|
|
|
|
def test_flags_bare_run_call(self, linter):
|
|
# .run( without explicit subprocess. prefix — still a subprocess call
|
|
line = ' result = obj.run(cmd, text=True)'
|
|
assert _scan_line(linter, line, RULE_NAME)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Suppression — these should NOT be flagged
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
class TestSuppression:
|
|
def test_does_not_flag_comment_only_line(self, linter):
|
|
line = ' # subprocess.run(cmd, text=True) — example'
|
|
assert not _scan_line(linter, line, RULE_NAME)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Helper functions — unit tests for _is_likely_subprocess_call and
|
|
# _looks_like_string_literal
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
class TestHelpers:
|
|
def test_is_likely_subprocess_call_rejects_plain_assignment(self, linter):
|
|
assert not linter._is_likely_subprocess_call("config.text = True")
|
|
|
|
def test_looks_like_string_literal_double_quotes(self, linter):
|
|
import re
|
|
line = ' msg = "use text=True carefully"'
|
|
match = re.search(r"\btext\s*=\s*True\b", line)
|
|
assert match is not None
|
|
assert linter._looks_like_string_literal(line, match)
|
|
|
|
|
|
def test_looks_like_string_literal_false_for_real_code(self, linter):
|
|
import re
|
|
line = ' subprocess.run(cmd, text=True)'
|
|
match = re.search(r"\btext\s*=\s*True\b", line)
|
|
assert match is not None
|
|
assert not linter._looks_like_string_literal(line, match)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Full-repo scan — after PR #60741 merges, the new rule should find ZERO
|
|
# unsuppressed violations in the whole tree (excluding the linter itself
|
|
# and CONTRIBUTING docs). This test will FAIL until PR #60741 is merged;
|
|
# mark it xfail when run on a branch that doesn't include PR #60741's fixes.
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
# All matches are the expected PR #60741 sites — OK on this branch.
|