test(project): close the gaps the merge-wave reviews left in the guard tests (#1902)

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 ``<name>.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 ``<name>.unparse`` or ``getattr(<name>, '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,18 +152,20 @@ 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):
             return node.module == 'ast' and not node.level and any(alias.name == 'unparse' for alias in node.names)
         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__':

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)
 

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):