diff --git a/tests/project/test_ast_unparse_is_gated_1899_unit.py b/tests/project/test_ast_unparse_is_gated_1899_unit.py index ba51c45ba..c77bcbbab 100644 --- a/tests/project/test_ast_unparse_is_gated_1899_unit.py +++ b/tests/project/test_ast_unparse_is_gated_1899_unit.py @@ -10,8 +10,10 @@ A test reads a node back as written with :func:`ast.get_source_segment`, compares it with :func:`ast.dump`, or matches its fields, all of which 3.8 has. One that has to render a node puts the call under a gate on -:data:`sys.version_info`: an enclosing ``if``, or a decorator such as -:func:`unittest.skipIf` on the test. +:data:`sys.version_info`: the branch of an enclosing ``if`` that runs only on +3.9 or later, or a :func:`unittest.skipIf` or :func:`unittest.skipUnless` on the +test that runs it only there (#1902). The other branch, a gate that points the +wrong way, and a ``getattr(ast, 'unparse')`` all count as ungated. This module is unit-tier: it parses source and imports nothing it scans. @@ -31,27 +33,107 @@ TESTS = ROOT / 'tests' -def _reads_version_info(node: 'ast.AST') -> bool: - """Whether ``node`` reads ``version_info``, as ``sys.version_info`` or imported bare.""" - return any((isinstance(sub, ast.Attribute) and sub.attr == 'version_info') - or (isinstance(sub, ast.Name) and sub.id == 'version_info') for sub in ast.walk(node)) +#: The first release with :func:`ast.unparse`. +UNPARSE_SINCE = (3, 9) +#: The comparison each operator becomes with its operands swapped round. +_FLIPPED = {ast.Lt: ast.Gt, ast.LtE: ast.GtE, ast.Gt: ast.Lt, ast.GtE: ast.LtE} -def _is_gate(node: 'ast.AST') -> bool: - """Whether ``node`` gates what it holds on the interpreter's version.""" + +def _is_version_info(node: 'ast.AST') -> bool: + """Whether ``node`` is ``version_info``, as ``sys.version_info`` or imported bare, or a slice of it.""" + if isinstance(node, ast.Subscript) and isinstance(node.slice, ast.Slice): + node = node.value + return (isinstance(node, ast.Attribute) and node.attr == 'version_info') \ + or (isinstance(node, ast.Name) and node.id == 'version_info') + + +def _version(node: 'ast.AST') -> 'tuple[int, ...] | None': + """The version a tuple of integer literals spells, as in ``(3, 9)``.""" + if isinstance(node, ast.Tuple) and node.elts and all( + isinstance(elt, ast.Constant) and type(elt.value) is int for elt in node.elts): + return tuple(elt.value for elt in node.elts) + return None + + +def _assures(test: 'ast.AST', outcome: bool) -> bool: + """Whether ``test`` coming out ``outcome`` means the interpreter has :func:`ast.unparse`. + + A comparison of ``version_info`` with a version of 3.9 or later, such as + ``(3, 9)``, does: ``>=`` or ``>`` when true, ``<`` or ``<=`` when false, with + the operands either way round. ``not``, ``and`` and ``or`` combine those as + they evaluate. Anything else assures nothing. + + """ + if isinstance(test, ast.UnaryOp) and isinstance(test.op, ast.Not): + return _assures(test.operand, not outcome) + if isinstance(test, ast.BoolOp): + # ``and`` is true only when every operand is, ``or`` false only when every one is. + if isinstance(test.op, ast.And) == outcome: + return any(_assures(value, outcome) for value in test.values) + return all(_assures(value, outcome) for value in test.values) + if not (isinstance(test, ast.Compare) and len(test.ops) == 1): + return False + op, left, right = type(test.ops[0]), test.left, test.comparators[0] + if _is_version_info(right) and not _is_version_info(left): + op, left, right = _FLIPPED.get(op, op), right, left + version = _version(right) + if not _is_version_info(left) or version is None or version < UNPARSE_SINCE: + return False + # ``version_info > version`` when true, or ``version_info < version`` when + # false, puts the interpreter at ``version`` or later: past 3.9 either way. + if outcome: + return op in (ast.Gt, ast.GtE) + return op in (ast.Lt, ast.LtE) + + +def _skip_condition(decorator: 'ast.AST') -> 'tuple[ast.AST, bool] | None': + """The condition of a :func:`unittest.skipIf` or :func:`unittest.skipUnless`, and the outcome it runs on.""" + if not (isinstance(decorator, ast.Call) and decorator.args): + return None + func = decorator.func + name = func.attr if isinstance(func, ast.Attribute) else func.id if isinstance(func, ast.Name) else '' + if name == 'skipIf': + return decorator.args[0], False + if name == 'skipUnless': + return decorator.args[0], True + return None + + +def _gated_children(node: 'ast.AST') -> 'Iterator[tuple[ast.AST, bool]]': + """Each child of ``node``, and whether a version gate in ``node`` itself encloses it. + + The branch of an ``if``, or of a conditional expression, that runs only on + 3.9 or later is gated, and so is the body of a function or class a skip + decorator runs only there. The test itself, the other branch, and the + decorators are not. + + """ if isinstance(node, (ast.If, ast.IfExp)): - return _reads_version_info(node.test) + body, orelse = (node.body, node.orelse) if isinstance(node, ast.If) else ([node.body], [node.orelse]) + yield node.test, False + for child in body: + yield child, _assures(node.test, True) + for child in orelse: + yield child, _assures(node.test, False) + return + gated = False if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)): - return any(_reads_version_info(decorator) for decorator in node.decorator_list) - return False + for decorator in node.decorator_list: + condition = _skip_condition(decorator) + gated = gated or (condition is not None and _assures(*condition)) + decorators = {id(decorator) for decorator in getattr(node, 'decorator_list', ())} + for child in ast.iter_child_nodes(node): + yield child, gated and id(child) not in decorators def ungated(source: str) -> 'Iterator[int]': """Yield the line of each use of :func:`ast.unparse` that no version gate encloses. - A use is ``.unparse`` for a name :mod:`ast` is imported as, an - ``from ast import unparse``, or a load of the name that import binds. Imports - are collected from the whole file, function bodies included. + A use is ``.unparse`` or ``getattr(, 'unparse')`` for a name + :mod:`ast` is imported as, an ``from ast import unparse``, or a load of the + name that import binds. Imports are collected from the whole file, function + bodies included. Args: source: Module source text. @@ -70,6 +152,9 @@ def ungated(source: str) -> 'Iterator[int]': functions.update(alias.asname or alias.name for alias in node.names if alias.name == 'unparse') def use(node: 'ast.AST') -> bool: + if isinstance(node, ast.Call) and isinstance(node.func, ast.Name) and node.func.id == 'getattr': + return len(node.args) >= 2 and isinstance(node.args[0], ast.Name) and node.args[0].id in modules \ + and isinstance(node.args[1], ast.Constant) and node.args[1].value == 'unparse' if isinstance(node, ast.Attribute): return node.attr == 'unparse' and isinstance(node.value, ast.Name) and node.value.id in modules if isinstance(node, ast.ImportFrom): @@ -77,11 +162,10 @@ def use(node: 'ast.AST') -> bool: return isinstance(node, ast.Name) and isinstance(node.ctx, ast.Load) and node.id in functions def walk(node: 'ast.AST', gated: bool) -> 'Iterator[int]': - gated = gated or _is_gate(node) if not gated and isinstance(node, (ast.expr, ast.stmt)) and use(node): yield node.lineno - for child in ast.iter_child_nodes(node): - yield from walk(child, gated) + for child, gate in _gated_children(node): + yield from walk(child, gated or gate) yield from walk(tree, False) @@ -98,7 +182,12 @@ def test_no_test_calls_unparse_outside_a_version_gate(self) -> None: 'ast.get_source_segment or match its fields, or gate on sys.version_info') def test_each_use_and_gate_is_matched(self) -> None: - """The scan sees each spelling of a use, and each gate, so a pass is not vacuous.""" + """The scan sees each spelling of a use, and each gate, so a pass is not vacuous. + + From line 21 on is #1902: the uses a gate covers only from the branch, + or the direction, that runs on 3.9 or later, and ``getattr``. + + """ source = ( 'import ast\n' 'import ast as tree\n' @@ -120,8 +209,33 @@ def test_each_use_and_gate_is_matched(self) -> None: 'def helper(node):\n' ' """ast.unparse is 3.9+"""\n' ' return ast.get_source_segment(source, node), node.unparse\n' + "@unittest.skipUnless(sys.version_info >= (3, 9), 'ast.unparse is 3.9+')\n" + 'def test():\n' + ' return ast.unparse(node)\n' + 'if sys.version_info >= (3, 9):\n' + ' pass\n' + 'else:\n' + ' ast.unparse(node)\n' + 'if sys.version_info < (3, 9):\n' + ' ast.unparse(node)\n' + "@unittest.skipIf(sys.version_info >= (3, 9), 'reversed')\n" + 'def test():\n' + ' return ast.unparse(node)\n' + "getattr(ast, 'unparse')(node)\n" + 'getattr(tree, "unparse")(node)\n' + "if sys.version_info >= (3, 9):\n" + " getattr(ast, 'unparse')(node)\n" + 'if sys.version_info < (3, 9):\n' + ' ast.dump(node)\n' + 'else:\n' + ' ast.unparse(node)\n' + 'from sys import version_info\n' + 'if version_info >= (3, 9):\n' + ' ast.unparse(node)\n' + 'if (3, 9) <= sys.version_info[:2] and node:\n' + ' ast.unparse(node)\n' ) - self.assertEqual(list(ungated(source)), [5, 6, 7, 8, 9]) + self.assertEqual(list(ungated(source)), [5, 6, 7, 8, 9, 27, 29, 32, 33, 34]) if __name__ == '__main__': diff --git a/tests/project/test_conda_platform_pin_refresh_1851.py b/tests/project/test_conda_platform_pin_refresh_1851.py index f8afc4af9..342a169f4 100644 --- a/tests/project/test_conda_platform_pin_refresh_1851.py +++ b/tests/project/test_conda_platform_pin_refresh_1851.py @@ -35,6 +35,7 @@ import io import json import pathlib +import re import shlex import sys import tarfile @@ -590,6 +591,52 @@ def test_the_table_is_not_a_bot_managed_path(self) -> None: self.assertNotIn('conda/platform-pins.toml', verdict.BOT_MANAGED_PATHS) +#: A requirement's project name, at the start of a requirement specifier. +REQUIREMENT_NAME = re.compile(r'[A-Za-z0-9](?:[A-Za-z0-9._-]*[A-Za-z0-9])?') + + +def pip_installs(line: 'str') -> 'list[str]': + """The projects a ``python -m pip install`` shell line names, normalised; none for any other line. + + Split as the shell would, so nothing after a ``#`` counts (#1902), and each + requirement read for its project name, so ``requests[socks]`` and + ``requests>=2`` are both :mod:`requests`. Options, and an argument that is + not a project name, such as ``.[all]``, name nothing. + + """ + # Checked before the split, which raises on a quote another line closes. + if line.split()[:4] != ['python', '-m', 'pip', 'install']: + return [] + names = [] # type: list[str] + for word in shlex.split(line, comments=True)[4:]: + match = None if word.startswith('-') else REQUIREMENT_NAME.match(word) + if match is not None: + names.append(re.sub(r'[-_.]+', '-', match.group()).lower()) + return names + + +class PipInstallsTests(unittest.TestCase): + """:func:`pip_installs` reads an install line for the projects it installs, as pip would.""" + + def test_a_requirement_is_read_for_its_name(self) -> None: + for line, expected in ( + ('python -m pip install -U pip requests', ['pip', 'requests']), + ('python -m pip install requests[socks]', ['requests']), + ("python -m pip install 'requests>=2'", ['requests']), + ('python -m pip install Requests_Toolbelt', ['requests-toolbelt']), + ('python -m pip install -e .[all]', []), + ('python util/conda_vendor.py requests', []), + ("echo 'requests", []), + ): + with self.subTest(line=line): + self.assertEqual(pip_installs(line), expected) + + def test_a_name_after_a_comment_is_not_installed(self) -> None: + # #1902: split without comment handling, ``requests`` here read as installed. + self.assertEqual(pip_installs('python -m pip install -e .[all] # requests comes later'), []) + self.assertEqual(pip_installs('python -m pip install pip # requests'), ['pip']) + + @unittest.skipIf(yaml is None, 'PyYAML, from the test extra, is not installed') class WorkflowTests(unittest.TestCase): """``conda-update`` refreshes the pins after installing ``.[all]`` and before writing the requirements.""" @@ -624,9 +671,8 @@ def test_the_refresh_runs_between_the_install_and_the_writer(self) -> None: def test_the_job_installs_requests_before_the_refresh(self) -> None: """``.[all]`` does not bring :mod:`requests`, which ``--refresh-pins`` reads PyPI with.""" refresh = self.index(lambda step: self.runs(step) == ['python util/conda_vendor.py --refresh-pins']) - # Only the install lines are split: another step's run carries a quote across lines. installs = [index for index, step in enumerate(self.steps) for line in self.runs(step) - if line.startswith('python -m pip install ') and 'requests' in shlex.split(line)] + if 'requests' in pip_installs(line)] self.assertTrue(installs, 'no step of conda-update installs requests') self.assertLess(min(installs), refresh) diff --git a/tests/project/test_no_version_directives_in_generated_tables_1884_unit.py b/tests/project/test_no_version_directives_in_generated_tables_1884_unit.py index 9b06bec50..a73dfe7d9 100644 --- a/tests/project/test_no_version_directives_in_generated_tables_1884_unit.py +++ b/tests/project/test_no_version_directives_in_generated_tables_1884_unit.py @@ -38,8 +38,8 @@ #: Repository root. ROOT = pathlib.Path(__file__).resolve().parents[2] -#: A version directive, as Sphinx spells it. -DIRECTIVE = re.compile(r'\.\. version(?:added|changed|removed)::.*') +#: A version directive, as Sphinx spells it: reST allows any whitespace after the ``..`` (#1902). +DIRECTIVE = re.compile(r'\.\.\s+version(?:added|changed|removed)::.*') #: The crawler modules whose templates carried a directive before #1884. ISSUE_VENDORS = ( @@ -191,6 +191,8 @@ def test_only_template_text_is_scanned(self) -> 'None': LINE = lambda NAME: f"class {NAME}:\\n .. versionadded:: 1.0.3\\n" + WIDE = ".. versionchanged:: 1.0.10" + def helper(): """Hand-written helper. @@ -230,8 +232,16 @@ class Helper: crawlers = vendor_classes([tree]) self.assertEqual(crawlers, frozenset({'Vendor', 'Base', 'Child'})) found = sorted(directive for _, directive in template_directives(tree, crawlers)) - self.assertEqual(found, ['.. versionadded:: 1.0.3', '.. versionadded:: 1.0.8', - '.. versionchanged:: 1.0.5', '.. versionchanged:: 1.0.7']) + self.assertEqual(found, ['.. versionchanged:: 1.0.10', '.. versionadded:: 1.0.3', + '.. versionadded:: 1.0.8', '.. versionchanged:: 1.0.5', + '.. versionchanged:: 1.0.7']) + + def test_a_directive_spaced_wider_is_matched(self) -> 'None': + # #1902: reST takes any whitespace between ``..`` and the directive name. + for text in ('.. versionchanged:: 1.5.0', '..\tversionadded:: 1.5.0', '.. versionremoved:: 1.5.0'): + with self.subTest(text=text): + self.assertIsNotNone(DIRECTIVE.search(text)) + self.assertIsNone(DIRECTIVE.search('..versionadded:: 1.5.0')) class TestDocsPages(unittest.TestCase): diff --git a/tests/test_support_helpers.py b/tests/test_support_helpers.py index ae8c17385..cbea1f5f2 100644 --- a/tests/test_support_helpers.py +++ b/tests/test_support_helpers.py @@ -68,6 +68,7 @@ import types import unittest import urllib.error +from typing import TYPE_CHECKING from unittest import mock from tests._support import (MAX_TIMEOUT, PCAPNG_GENERATOR, TIMEOUT_SCALE_ENV, @@ -77,11 +78,23 @@ skip_if_stand_in, snapshot_modules, stand_in_reason, time_limit, upstream_digests) +if TYPE_CHECKING: + from typing import Iterator + #: Seconds a :class:`TimeLimitTests` body spins waiting for the deadline before it #: gives up and fails by name: ten times the one-second deadline those tests arm, #: so a deadline that fires at all, its repeat included, lands well inside it. SPIN_LIMIT = 10.0 +#: Seconds a stalled :class:`TimeLimitTests` body sleeps before its ``try``: past +#: the one-second deadline, so an alarm armed as the ``with`` was entered comes due +#: in it (#1902). +STALL = 1.5 + +#: :func:`signal.alarm` itself, for :meth:`TimeLimitTests.spin` to reach while +#: :meth:`TimeLimitTests.held_deadline` stands in for the name. +_ALARM = signal.alarm + class Closeable: """A stand-in for a resource that records having been closed. @@ -239,15 +252,59 @@ class TimeLimitTests(unittest.TestCase): deadline that never gets through fails this test in seconds rather than hanging the worker until pytest-timeout's backstop ends it. + A body that drops the first raise has to have it land inside the ``try``, or + the finaliser, that drops it (#1902). So it runs under :meth:`held_deadline`, + and the deadline is armed only once :meth:`spin` is called from in there. + Armed as the ``with`` is entered, it counts down through everything before + that ``try``, and a runner stalled there for the whole second gets the raise + outside it. A body that only needs a raise to land somewhere in it takes the + deadline as armed. + """ + #: Seconds ``time_limit`` asked :func:`signal.alarm` for as it was entered under + #: :meth:`held_deadline`, or ``0``. + deadline = 0 + + #: Seconds :meth:`held_deadline` held back and :meth:`spin` has not yet armed. + held = 0 + + @contextlib.contextmanager + def held_deadline(self) -> 'Iterator[None]': + """Hold the deadline's arm back for :meth:`spin`, and check it was armed there. + + Within the block, the first non-zero :func:`signal.alarm` is ``time_limit`` + arming its deadline. It is recorded in :attr:`deadline` and cancels what + was pending, as arming does, but nothing is armed until :meth:`spin` arms + it. Every other call reaches the alarm, so the deadline's repeats, its + cancel and the restoring of an enclosing alarm are real. + + """ + self.deadline = self.held = 0 + + def alarm(seconds: int) -> int: + if seconds and not self.deadline: + self.deadline = self.held = seconds + return _ALARM(0) + return _ALARM(seconds) + + with mock.patch.object(signal, 'alarm', alarm): + yield + self.assertEqual((self.deadline, self.held), (1, 0), + 'the one-second deadline was not held back and then armed by spin') + def spin(self, until: float) -> None: """Busy-wait in pure Python until :func:`time.monotonic` reaches ``until``. The stand-in for a hang, but a bounded one. Pure Python, so the deadline's - signal is delivered inside it, between bytecodes. + signal is delivered inside it, between bytecodes. A deadline + :meth:`held_deadline` holds is armed first, so its first raise lands in + here, wherever the caller is. """ + if self.held: + seconds, self.held = self.held, 0 + _ALARM(seconds) while time.monotonic() < until: pass @@ -447,7 +504,8 @@ def __del__(self) -> None: spin(end) with mock.patch.object(sys, 'unraisablehook', - lambda unraisable: dropped.append(unraisable.exc_type)): + lambda unraisable: dropped.append(unraisable.exc_type)), \ + self.held_deadline(): with self.assertRaisesRegex(TimeoutError, r'within 1s'): with time_limit(1): held = Finalised() @@ -469,7 +527,7 @@ def test_a_raise_an_except_oserror_drops_is_raised_again(self) -> None: end = time.monotonic() + SPIN_LIMIT dropped = [] # type: list[type] - with self.assertRaisesRegex(TimeoutError, r'within 1s'): + with self.held_deadline(), self.assertRaisesRegex(TimeoutError, r'within 1s'): with time_limit(1): try: self.hang(end) @@ -480,6 +538,30 @@ def test_a_raise_an_except_oserror_drops_is_raised_again(self) -> None: self.assertEqual(dropped, [TimeoutError]) self.assert_nothing_left_behind(own_handler) + def test_a_stall_before_the_try_still_leaves_a_raise_to_drop(self) -> None: + """#1902: a runner stalled past the deadline before the body's ``try`` changes nothing. + + Armed as the ``with`` was entered, the deadline came due in the stall, + before the ``try`` meant to drop its first raise. That raise went straight + through and none was dropped. + + """ + own_handler = signal.getsignal(signal.SIGALRM) + dropped = [] # type: list[type] + + with self.held_deadline(), self.assertRaisesRegex(TimeoutError, r'within 1s'): + with time_limit(1): + time.sleep(STALL) + end = time.monotonic() + SPIN_LIMIT + try: + self.hang(end) + except OSError as exc: + dropped.append(type(exc)) + self.hang(end) + + self.assertEqual(dropped, [TimeoutError]) + self.assert_nothing_left_behind(own_handler) + def test_the_repeat_never_outlives_the_block(self) -> None: """Left normally, by timeout, or by another exception, nothing stays armed. @@ -494,16 +576,17 @@ def test_the_repeat_never_outlives_the_block(self) -> None: with self.subTest(exit=how): end = time.monotonic() + SPIN_LIMIT raised = None - try: - with time_limit(1): - with contextlib.suppress(TimeoutError): - self.hang(end) - if expected is TimeoutError: - self.hang(end) - elif expected is LookupError: - raise LookupError(how) - except (TimeoutError, LookupError) as exc: - raised = type(exc) + with self.held_deadline(): + try: + with time_limit(1): + with contextlib.suppress(TimeoutError): + self.hang(end) + if expected is TimeoutError: + self.hang(end) + elif expected is LookupError: + raise LookupError(how) + except (TimeoutError, LookupError) as exc: + raised = type(exc) self.assertIs(raised, expected) self.assert_nothing_left_behind(own_handler) @@ -540,7 +623,7 @@ def test_an_enclosing_alarm_survives_a_deadline_that_repeated(self) -> None: end = time.monotonic() + SPIN_LIMIT signal.alarm(30) - with self.assertRaisesRegex(TimeoutError, r'within 1s'): + with self.held_deadline(), self.assertRaisesRegex(TimeoutError, r'within 1s'): with time_limit(1): with contextlib.suppress(TimeoutError): self.hang(end)