Skip to content

Commit

Permalink
Build out large Act blocks option (jamescooke#218)
Browse files Browse the repository at this point in the history
* Add LARGE act block style option value

* Restore add_node_parents() helper

* Reinstate greedy act blocks for Large act block style

* Reinstate test on add_node_parents

* Add pytest icdiff to test requirements

* Add test on Function.mark_act()

* Fix typo in tox config

* Prevent mypy caches leaking between runs on local

* Fix lint: ignore stuffing of parent attr; update comment

* Bump Black to 23.3.0 from 22.12.0

* Adjust Checker to use a default config class attr

* Remove old file

* Build out FirstChildFinder to help with large style act blocks

* Fix lint: remove unused import

* Switch from adding parents to using first child finder visitor

* Stub in temp tox config to run large act block config

* Update Black formatted examples and add environment to tox

* Update CHANGELOG
  • Loading branch information
jamescooke authored May 9, 2023
1 parent 118bdea commit 8abe040
Show file tree
Hide file tree
Showing 26 changed files with 318 additions and 55 deletions.
13 changes: 9 additions & 4 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,22 @@ Unreleased_
See also `latest documentation
<https://flake8-aaa.readthedocs.io/en/latest/#__unreleased_marker__>`_.

* ⛏️ Move Black formatted test examples to their own directory. This will help
* 🎈 New "large" Act block style option added. This allows context managers to
join the Act block when they directly wrap the Act node. This behaviour is
provided to provide compatibility with Black versions ``23.*``. Fixes `issue
200 <https://github.com/jamescooke/flake8-aaa/issues/200>`_.

* ⛏️ Moved Black formatted test examples to their own directory. This helps
when running Flake8 against Black formatted tests which need
``--aaa-act-block-style=large``. Also fix up associated Makefile recipes and
update example README file.

* ⛏️ Remove list of nodes from ``Block`` class and instead keep the start and
* ⛏️ Removed list of nodes from ``Block`` class and instead kept the start and
end line numbers of the block. This allows for any structural discoveries
while doing AST node traversal (e.g. when parsing Large style Act blocks) to
be used to calculate the size of the Act block itself. The alternative would
be to store the list of nodes in the Act block, and then re-walk them when
working out the block's span, which would be duplication of effort.
have been to store the list of nodes in the Act block, and then re-walk them
when working out the block's span, which would be duplication of effort.

* ⛏️ Remove unused ``MultiNodeBlock``.

Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ lintexamples:
flake8 examples/good examples/bad | sort > flake8.out
diff examples/bad/flake8_expected.out flake8.out
@echo "=== mypy ==="
mypy examples/conftest.py examples/good --ignore-missing-imports --exclude examples/good/black/
mypy examples/conftest.py examples/good --ignore-missing-imports --exclude examples/black/ --no-incremental
mypy examples/bad --ignore-missing-imports
@echo "=== black ==="
black --check --diff --verbose examples/black
Expand Down
2 changes: 2 additions & 0 deletions configs/black_compatible.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[flake8]
aaa_act_block_style = large
6 changes: 6 additions & 0 deletions docs/command_line.rst
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@ debugging. Its goal is to show the state of analysed test functions, which
lines are considered to be parts of which blocks and any errors that have been
found.

.. warning::

Command line mode does not support ``--aaa-act-block-style=large`` option
or associated configuration. `Issue regarding this is open on GitHub
<https://github.com/jamescooke/flake8-aaa/issues/217>`_.

Invocation, output and return value
-----------------------------------

Expand Down
1 change: 0 additions & 1 deletion examples/black/test_django_fakery_factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ def test_default(self):


class TestUserFactory(TestCase):

user_model = get_user_model()

def test_default(self):
Expand Down
47 changes: 32 additions & 15 deletions examples/black/test_with_statement.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import io
import pathlib
import warnings
from typing import Generator, List

Expand All @@ -12,44 +13,61 @@ def test_pytest_raises() -> None:
list()[0]


def test_deprecation_warning():
def test_deprecation_warning() -> None:
with pytest.deprecated_call():
warnings.warn("deprecate warning", DeprecationWarning)


def test_user_warning():
def test_user_warning() -> None:
with pytest.warns(UserWarning):
warnings.warn("my warning", UserWarning)


# --- Use of context managers in tests ---


def test_simple(hello_world_path) -> None:
def test_simple(hello_world_path: pathlib.Path) -> None:
"""
`with` statement is part of arrange. Blank lines are maintained around Act.
Test checks "simple" context manager in both Act block styles:
* DEFAULT: `with` statement is part of arrange. Blank lines are maintained
around Act.
* LARGE: When formatted with Black, context manager is squashed against act
node. Large act block mode allows the context manager to join the act
block and linting passes.
"""
with open(hello_world_path) as f:

result = f.read()

assert result == "Hello World!\n"


def test_whole(hello_world_path) -> None:
def test_whole(hello_world_path: pathlib.Path) -> None:
"""
`with` statement wraps whole of test
`with` statement wraps whole of test. This checks context manager in both
Act block styles:
* DEFAULT: `with` statement is part of Arrange. Result assignment is Act.
Blank lines are maintained.
* LARGE: When formatted with Black, context manager is squashed against
result assignment. Act block grows to consume context manager because
it's the first node in the context manager body. However, new large Act
block _still_ finishes at the end of the result assignment. There is
then a blank line and the assert block, even though that's inside the
context manager.
"""
with open(hello_world_path) as f:

result = f.read()

assert result == "Hello World!\n"
assert f.fileno() > 0


def test_extra_arrange(hello_world_path) -> None:
def test_extra_arrange(hello_world_path: pathlib.Path) -> None:
"""
Any extra arrangement goes in the `with` block.
Any extra arrangement goes in the `with` block. Works "as-is" for Large act
block style because `f.read()` node prevents the `result = ` act node from
joining the `with` context manager.
"""
with open(hello_world_path) as f:
f.read()
Expand All @@ -59,7 +77,7 @@ def test_extra_arrange(hello_world_path) -> None:
assert result == ""


def test_assert_in_block(hello_world_path) -> None:
def test_assert_in_block(hello_world_path: pathlib.Path) -> None:
"""
Any assertion that needs the `with` block open, goes after Act and a BL.
"""
Expand All @@ -73,19 +91,18 @@ def test_assert_in_block(hello_world_path) -> None:
assert result == ""


def test_pytest_assert_raises_in_block(hello_world_path) -> None:
def test_pytest_assert_raises_in_block(hello_world_path: pathlib.Path) -> None:
"""
Checking on a raise in a with block works with Pytest.
"""
with open(hello_world_path) as f:

with pytest.raises(io.UnsupportedOperation):
f.write("hello back")

assert f.read() == "Hello World!\n"


def test_pytest_assert_raises_on_with(hello_world_path) -> None:
def test_pytest_assert_raises_on_with(hello_world_path: pathlib.Path) -> None:
"""
Checking on the raise from a with statement works with Pytest.
"""
Expand All @@ -96,7 +113,7 @@ def test_pytest_assert_raises_on_with(hello_world_path) -> None:
assert "invalid mode" in str(excinfo.value)


def test_with_in_assert(hello_world_path) -> None:
def test_with_in_assert(hello_world_path: pathlib.Path) -> None:
"""
Using with statement in Assert block is valid
"""
Expand Down
1 change: 0 additions & 1 deletion examples/black/test_with_statement_unittest.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ def test_assert_raises_in_block(self):
Checking on a raise in a with block works with unittest.
"""
with open(self.hello_world_path) as f:

with self.assertRaises(io.UnsupportedOperation):
f.write("hello back")

Expand Down
2 changes: 1 addition & 1 deletion requirements/examples.in
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# py37
black<23
black
flake8
mypy
pytest
8 changes: 5 additions & 3 deletions requirements/examples.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
#
# pip-compile --output-file=examples.txt examples.in
#
black==22.12.0
black==23.3.0
# via -r examples.in
click==8.1.3
# via black
Expand All @@ -29,10 +29,12 @@ mypy-extensions==1.0.0
# black
# mypy
packaging==23.1
# via pytest
# via
# black
# pytest
pathspec==0.11.1
# via black
platformdirs==3.2.0
platformdirs==3.5.0
# via black
pluggy==1.0.0
# via pytest
Expand Down
1 change: 1 addition & 0 deletions requirements/test.in
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@ isort
mypy==0.930
pygments
pytest
pytest-icdiff
restructuredtext-lint
yapf
21 changes: 14 additions & 7 deletions requirements/test.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@
#
# pip-compile --output-file=test.txt test.in
#
attrs==22.2.0
# via pytest
docutils==0.19
# via restructuredtext-lint
exceptiongroup==1.1.1
# via pytest
faker==18.4.0
faker==18.6.0
# via -r test.in
flake8==4.0.1
# via -r test.in
icdiff==2.0.6
# via pytest-icdiff
importlib-metadata==4.2.0
# via
# flake8
Expand All @@ -29,17 +29,23 @@ mypy==0.930
# via -r test.in
mypy-extensions==1.0.0
# via mypy
packaging==23.0
packaging==23.1
# via pytest
pluggy==1.0.0
# via pytest
pprintpp==0.4.0
# via pytest-icdiff
pycodestyle==2.8.0
# via flake8
pyflakes==2.4.0
# via flake8
pygments==2.14.0
pygments==2.15.1
# via -r test.in
pytest==7.2.2
pytest==7.3.1
# via
# -r test.in
# pytest-icdiff
pytest-icdiff==0.6
# via -r test.in
python-dateutil==2.8.2
# via faker
Expand All @@ -51,14 +57,15 @@ tomli==2.0.1
# via
# mypy
# pytest
# yapf
typed-ast==1.5.4
# via mypy
typing-extensions==4.5.0
# via
# faker
# importlib-metadata
# mypy
yapf==0.32.0
yapf==0.33.0
# via -r test.in
zipp==3.15.0
# via importlib-metadata
34 changes: 29 additions & 5 deletions src/flake8_aaa/block.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from .exceptions import EmptyBlock
from .helpers import filter_arrange_nodes, get_first_token, get_last_token
from .types import LineType
from .visitors import find_first_child_nodes

_Block = TypeVar('_Block', bound='Block')

Expand Down Expand Up @@ -38,18 +39,41 @@ def __init__(self, first_line_no: int, last_line_no: int, lt: LineType) -> None:
def build_act(
cls: Type[_Block],
node: ast.stmt,
test_func_node: ast.FunctionDef, # use this in TODO200
act_block_style: ActBlockStyle, # use this in TODO200
test_func_node: ast.FunctionDef,
act_block_style: ActBlockStyle,
) -> _Block:
"""
Act block is a single node by default. TODO200
Using the provided `node` as Act Node, build the Act Block depending on
the `act_block_style`.
Act blocks are simply a single Act node in default mode. However,
"large" Act blocks include the Act node and any context managers that
wrap them.
Args:
node: Act node already found by Function.mark_act()
test_func_node: Node of test function / method.
act_block_style: Currently always DEFAULT.
act_block_style: Default Act Blocks are just the Act node. Large
Act Blocks can absorb context managers that wrap them.
"""
first, last = get_span(node, node)
if act_block_style == ActBlockStyle.DEFAULT:
first, last = get_span(node, node)
return cls(first, last, LineType.act)

# --- LARGE Act Block behaviour...

# Walk up the parent nodes of the act node, but only when act node is
# the first child. This allows act block to absorb context managers
# that have the act node first in their body.
child_parent_map = find_first_child_nodes(test_func_node)
wrapper_node = node
while True:
try:
wrapper_node = child_parent_map[wrapper_node]
except KeyError:
break

first, last = get_span(wrapper_node, node)
return cls(first, last, LineType.act)

@classmethod
Expand Down
7 changes: 5 additions & 2 deletions src/flake8_aaa/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,15 @@ class Checker:
name = __short_name__
version = __version__

default_config = Config.default_options()

def __init__(self, tree: AST, lines: List[str], filename: str):
self.tree = tree
self.lines = lines
self.filename = filename
self.ast_tokens: Optional[asttokens.ASTTokens] = None
self.config: Config = Config.default_options()

self.config: Config = self.default_config

@staticmethod
def add_options(option_manager) -> None:
Expand All @@ -56,7 +59,7 @@ def parse_options(cls, option_manager, options: argparse.Namespace, args) -> Non
Raises:
UnexpectedConfigValue: When config can't be loaded.
"""
cls.config = Config.load_options(options)
cls.default_config = Config.load_options(options)

def load(self) -> None:
self.ast_tokens = asttokens.ASTTokens(''.join(self.lines), tree=self.tree)
Expand Down
2 changes: 1 addition & 1 deletion src/flake8_aaa/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
@enum.unique
class ActBlockStyle(enum.Enum):
DEFAULT = 'default'
# LARGE = 'large' # TODO200
LARGE = 'large'

@classmethod
def allowed_values(cls: Type[_ActBlockStyle]) -> List[str]:
Expand Down
2 changes: 1 addition & 1 deletion src/flake8_aaa/function.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ def mark_act(self, act_block_style: ActBlockStyle) -> int:
lines in ``line_markers``.
Args:
act_block_style: Currently only DEFAULT. TODO200
act_block_style: Passed through to `build_act()` method.
Returns:
Number of lines covered by the Act block (used for debugging /
Expand Down
Loading

0 comments on commit 8abe040

Please sign in to comment.