From 9b82120ddb81373377b58da5de7caa9495a1551e Mon Sep 17 00:00:00 2001 From: John Litborn <11260241+jakkdl@users.noreply.github.com> Date: Thu, 28 Sep 2023 16:03:24 +0200 Subject: [PATCH 1/1] add support for printing the diff of AST trees when running tests (#3902) Co-authored-by: Jelle Zijlstra --- docs/contributing/the_basics.md | 38 +++++++++++++++++++++++++++++++++ src/black/debug.py | 21 ++++++++++++------ tests/conftest.py | 27 +++++++++++++++++++++++ tests/test_black.py | 29 ++++++++++++++++++++++--- tests/util.py | 24 ++++++++++++++++----- tox.ini | 2 +- 6 files changed, 125 insertions(+), 16 deletions(-) diff --git a/docs/contributing/the_basics.md b/docs/contributing/the_basics.md index 40d2332..864894b 100644 --- a/docs/contributing/the_basics.md +++ b/docs/contributing/the_basics.md @@ -37,6 +37,44 @@ the root of the black repo: (.venv)$ tox -e run_self ``` +### Development + +Further examples of invoking the tests + +```console +# Run all of the above mentioned, in parallel +(.venv)$ tox --parallel=auto + +# Run tests on a specific python version +(.venv)$ tox -e py39 + +# pass arguments to pytest +(.venv)$ tox -e py -- --no-cov + +# print full tree diff, see documentation below +(.venv)$ tox -e py -- --print-full-tree + +# disable diff printing, see documentation below +(.venv)$ tox -e py -- --print-tree-diff=False +``` + +`Black` has two pytest command-line options affecting test files in `tests/data/` that +are split into an input part, and an output part, separated by a line with`# output`. +These can be passed to `pytest` through `tox`, or directly into pytest if not using +`tox`. + +#### `--print-full-tree` + +Upon a failing test, print the full concrete syntax tree (CST) as it is after processing +the input ("actual"), and the tree that's yielded after parsing the output ("expected"). +Note that a test can fail with different output with the same CST. This used to be the +default, but now defaults to `False`. + +#### `--print-tree-diff` + +Upon a failing test, print the diff of the trees as described above. This is the +default. To turn it off pass `--print-tree-diff=False`. + ### News / Changelog Requirement `Black` has CI that will check for an entry corresponding to your PR in `CHANGES.md`. If diff --git a/src/black/debug.py b/src/black/debug.py index 150b448..cebc487 100644 --- a/src/black/debug.py +++ b/src/black/debug.py @@ -1,5 +1,5 @@ -from dataclasses import dataclass -from typing import Iterator, TypeVar, Union +from dataclasses import dataclass, field +from typing import Any, Iterator, List, TypeVar, Union from black.nodes import Visitor from black.output import out @@ -14,26 +14,33 @@ T = TypeVar("T") @dataclass class DebugVisitor(Visitor[T]): tree_depth: int = 0 + list_output: List[str] = field(default_factory=list) + print_output: bool = True + + def out(self, message: str, *args: Any, **kwargs: Any) -> None: + self.list_output.append(message) + if self.print_output: + out(message, *args, **kwargs) def visit_default(self, node: LN) -> Iterator[T]: indent = " " * (2 * self.tree_depth) if isinstance(node, Node): _type = type_repr(node.type) - out(f"{indent}{_type}", fg="yellow") + self.out(f"{indent}{_type}", fg="yellow") self.tree_depth += 1 for child in node.children: yield from self.visit(child) self.tree_depth -= 1 - out(f"{indent}/{_type}", fg="yellow", bold=False) + self.out(f"{indent}/{_type}", fg="yellow", bold=False) else: _type = token.tok_name.get(node.type, str(node.type)) - out(f"{indent}{_type}", fg="blue", nl=False) + self.out(f"{indent}{_type}", fg="blue", nl=False) if node.prefix: # We don't have to handle prefixes for `Node` objects since # that delegates to the first child anyway. - out(f" {node.prefix!r}", fg="green", bold=False, nl=False) - out(f" {node.value!r}", fg="blue", bold=False) + self.out(f" {node.prefix!r}", fg="green", bold=False, nl=False) + self.out(f" {node.value!r}", fg="blue", bold=False) @classmethod def show(cls, code: Union[str, Leaf, Node]) -> None: diff --git a/tests/conftest.py b/tests/conftest.py index 6751726..1a0dd74 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1 +1,28 @@ +import pytest + pytest_plugins = ["tests.optional"] + +PRINT_FULL_TREE: bool = False +PRINT_TREE_DIFF: bool = True + + +def pytest_addoption(parser: pytest.Parser) -> None: + parser.addoption( + "--print-full-tree", + action="store_true", + default=False, + help="print full syntax trees on failed tests", + ) + parser.addoption( + "--print-tree-diff", + action="store_true", + default=True, + help="print diff of syntax trees on failed tests", + ) + + +def pytest_configure(config: pytest.Config) -> None: + global PRINT_FULL_TREE + global PRINT_TREE_DIFF + PRINT_FULL_TREE = config.getoption("--print-full-tree") + PRINT_TREE_DIFF = config.getoption("--print-tree-diff") diff --git a/tests/test_black.py b/tests/test_black.py index d22b685..c665eee 100644 --- a/tests/test_black.py +++ b/tests/test_black.py @@ -9,7 +9,6 @@ import os import re import sys import types -import unittest from concurrent.futures import ThreadPoolExecutor from contextlib import contextmanager, redirect_stderr from dataclasses import replace @@ -1047,9 +1046,10 @@ class BlackTestCase(BlackBaseTestCase): self.assertEqual(len(n.children), 1) self.assertEqual(n.children[0].type, black.token.ENDMARKER) + @patch("tests.conftest.PRINT_FULL_TREE", True) + @patch("tests.conftest.PRINT_TREE_DIFF", False) @pytest.mark.incompatible_with_mypyc - @unittest.skipIf(os.environ.get("SKIP_AST_PRINT"), "user set SKIP_AST_PRINT") - def test_assertFormatEqual(self) -> None: + def test_assertFormatEqual_print_full_tree(self) -> None: out_lines = [] err_lines = [] @@ -1068,6 +1068,29 @@ class BlackTestCase(BlackBaseTestCase): self.assertIn("Actual tree:", out_str) self.assertEqual("".join(err_lines), "") + @patch("tests.conftest.PRINT_FULL_TREE", False) + @patch("tests.conftest.PRINT_TREE_DIFF", True) + @pytest.mark.incompatible_with_mypyc + def test_assertFormatEqual_print_tree_diff(self) -> None: + out_lines = [] + err_lines = [] + + def out(msg: str, **kwargs: Any) -> None: + out_lines.append(msg) + + def err(msg: str, **kwargs: Any) -> None: + err_lines.append(msg) + + with patch("black.output._out", out), patch("black.output._err", err): + with self.assertRaises(AssertionError): + self.assertFormatEqual("j = [1, 2, 3]\n", "j = [1, 2, 3,]\n") + + out_str = "".join(out_lines) + self.assertIn("Tree Diff:", out_str) + self.assertIn("+ COMMA", out_str) + self.assertIn("+ ','", out_str) + self.assertEqual("".join(err_lines), "") + @event_loop() @patch("concurrent.futures.ProcessPoolExecutor", MagicMock(side_effect=OSError)) def test_works_in_mono_process_only_environment(self) -> None: diff --git a/tests/util.py b/tests/util.py index 967d576..541d21d 100644 --- a/tests/util.py +++ b/tests/util.py @@ -12,6 +12,8 @@ from black.debug import DebugVisitor from black.mode import TargetVersion from black.output import diff, err, out +from . import conftest + PYTHON_SUFFIX = ".py" ALLOWED_SUFFIXES = (PYTHON_SUFFIX, ".pyi", ".out", ".diff", ".ipynb") @@ -34,22 +36,34 @@ fs = partial(black.format_str, mode=DEFAULT_MODE) def _assert_format_equal(expected: str, actual: str) -> None: - if actual != expected and not os.environ.get("SKIP_AST_PRINT"): + if actual != expected and (conftest.PRINT_FULL_TREE or conftest.PRINT_TREE_DIFF): bdv: DebugVisitor[Any] - out("Expected tree:", fg="green") + actual_out: str = "" + expected_out: str = "" + if conftest.PRINT_FULL_TREE: + out("Expected tree:", fg="green") try: exp_node = black.lib2to3_parse(expected) - bdv = DebugVisitor() + bdv = DebugVisitor(print_output=conftest.PRINT_FULL_TREE) list(bdv.visit(exp_node)) + expected_out = "\n".join(bdv.list_output) except Exception as ve: err(str(ve)) - out("Actual tree:", fg="red") + if conftest.PRINT_FULL_TREE: + out("Actual tree:", fg="red") try: exp_node = black.lib2to3_parse(actual) - bdv = DebugVisitor() + bdv = DebugVisitor(print_output=conftest.PRINT_FULL_TREE) list(bdv.visit(exp_node)) + actual_out = "\n".join(bdv.list_output) except Exception as ve: err(str(ve)) + if conftest.PRINT_TREE_DIFF: + out("Tree Diff:") + out( + diff(expected_out, actual_out, "expected tree", "actual tree") + or "Trees do not differ" + ) if actual != expected: out(diff(expected, actual, "expected", "actual")) diff --git a/tox.ini b/tox.ini index d34dbbc..018cef9 100644 --- a/tox.ini +++ b/tox.ini @@ -1,6 +1,6 @@ [tox] isolated_build = true -envlist = {,ci-}py{37,38,39,310,311,py3},fuzz,run_self +envlist = {,ci-}py{38,39,310,311,py3},fuzz,run_self [testenv] setenv = -- 2.39.5