Merge pull request #160 from jamescooke/comments

Ban inline comments from Act blocks

CHANGELOG.rst

@@ -14,6 +14,12 @@ Unreleased_
 See also `latest documentation
 <https://flake8-aaa.readthedocs.io/en/latest/#__unreleased_marker__>`_.
 
+Changed
+.......
+
+* Adjust rules for comments: no comments allowed in Act blocks. `#148
+  <https://github.com/jamescooke/flake8-aaa/issues/148>`_. 
+
 0.10.1_ - 2020/06/20
 --------------------
 

Makefile

@@ -1,7 +1,11 @@
 lint_files=setup.py src/flake8_aaa tests
 rst_files=README.rst CHANGELOG.rst
-good_examples = $(wildcard examples/good/*.py examples/good/noqa/test_cmd.py examples/good/black/noqa/test_cmd.py)
-bad_examples = $(wildcard examples/bad/*.py)
+
+# Lists of examples to pass through command line checks
+# NOQA examples in /examples/good will fail because CMD does not respect noqa
+# comments in the same way that flake8 does.
+good_examples = $(wildcard examples/good/*.py examples/good/black/*.py) examples/good/noqa/test_cmd.py
+bad_examples = $(wildcard examples/good/noqa/test_0*.py examples/good/black/noqa/test_0*.py examples/bad/*.py)
 
 
 venv:
@@ -23,15 +27,6 @@ tox:
 
 # --- Tox recipes ---
 
-# Location in `.tox/{envdir}/lib/` of site-packages
-lib_dir = python$$(python --version | grep '.\..' -o)
-
-# Turn on checking for pytest. Extracted as its own recipe for use only when
-# running in tox. E.g. `make lint` works from outside of tox invocation.
-.PHONY: pytyped
-pytyped:
-	touch $$TOXDIR/lib/$(lib_dir)/site-packages/pytest/py.typed $$TOXDIR/lib/$(lib_dir)/site-packages/_pytest/py.typed
-
 .PHONY: lint
 lint:
 	@echo "=== flake8 ==="
@@ -131,3 +126,7 @@ on_master:
 .PHONY: tag
 tag: on_master
 	git tag -a $$(python -c 'from src.flake8_aaa.__about__ import __version__; print("v{}".format(__version__))')
+
+.PHONY: black_examples
+black_examples:
+	$(MAKE) -C examples clean all

docs/commands.rst

@@ -133,6 +133,9 @@ ASS
 BL
     Line is considered a blank line for layout purposes.
 
+CMT
+    Line is a ``#`` comment.
+
 DEF
     Test function definition.
 

docs/discovery.rst

@@ -62,6 +62,19 @@ therefore not counted::
     2,
     ]"""
 
+Mark comments
+.............
+
+All lines that are ``#`` comment lines are marked.
+
+.. code-block:: python
+
+    # This line is considered a comment line
+
+    result = item.act()  # But not this line
+
+This process relies on analysing the tokens that make up the test.
+
 Find the Act block
 ..................
 

docs/rules.rst

@@ -110,6 +110,70 @@ Resolution
 
 Remove the blank line.
 
+AAA06: comment in Act block
+---------------------------
+
+Problematic code
+................
+
+.. code-block:: python
+
+    def test() -> None:
+        shopping = ['apples', 'bananas', 'cabbages']
+
+        # Reverse shopping list operates in place
+        shopping.reverse()  # act
+
+        assert shopping == ['cabbages', 'bananas', 'apples']
+
+Correct code
+............
+
+.. code-block:: python
+
+    def test() -> None:
+        """
+        Reverse shopping list operates in place
+        """
+        shopping = ['apples', 'bananas', 'cabbages']
+
+        shopping.reverse()  # act
+
+        assert shopping == ['cabbages', 'bananas', 'apples']
+
+Rationale
+.........
+
+The Act block carries out a single action on an object. It is the focus of each
+test. Therefore any comments on this single action are really comments on the
+test itself and so should be moved to the test docstring.
+
+By placing these important comments in the docstring we can:
+
+* Make it easier to keep the Act block simple.
+
+* Help to distinguish the Act block from the rest of the test.
+
+* Improve the documentation of tests because any important comments and notes
+  are lifted to the top of the test.
+
+Exceptions
+..........
+
+Inline comments used to pass information to linters are OK:
+
+* Marking the Act block:
+
+  .. code-block:: python
+
+      shopping.reverse()  # act
+
+* Marking lines in the action for linting reasons:
+
+  .. code-block:: python
+
+      result = shopping.reverse()  # type: ignore
+
 AAA99: collision when marking this line as NEW_CODE, was already OLD_CODE
 -------------------------------------------------------------------------
 

examples/Makefile

@@ -0,0 +1,14 @@
+.PHONY: all
+all: good/black/noqa
+
+good/black/noqa: good/black
+	mkdir good/black/noqa
+	cp good/noqa/*.py good/black/noqa/
+
+good/black:
+	mkdir good/black
+	cp good/*.py good/black/
+
+.PHONY: clean
+clean:
+	rm -rf good/black

examples/bad/bad_expected.out

@@ -14,6 +14,10 @@ examples/bad/test_aaa05.py:50:1: AAA05 blank line in block
 examples/bad/test_aaa05.py:54:1: AAA05 blank line in block
 examples/bad/test_aaa05.py:58:1: AAA05 blank line in block
 examples/bad/test_aaa05.py:9:1: AAA05 blank line in block
+examples/bad/test_aaa06.py:19:9: AAA06 comment in Act block
+examples/bad/test_aaa06.py:31:5: AAA06 comment in Act block
+examples/bad/test_aaa06.py:40:9: AAA06 comment in Act block
+examples/bad/test_aaa06.py:9:5: AAA06 comment in Act block
 examples/bad/test_example.py:4:5: AAA03 expected 1 blank line before Act block, found none
 examples/bad/test_noqa_flake8.py:11:17: E225 missing whitespace around operator
 examples/bad/test_noqa_flake8.py:5:10: E201 whitespace after '('

examples/bad/test_aaa06.py

@@ -0,0 +1,41 @@
+import pytest
+
+# Example from AAA06 doc:
+
+
+def test() -> None:
+    shopping = ['apples', 'bananas', 'cabbages']
+
+    # Reverse shopping list operates in place
+    shopping.reverse()  # act
+
+    assert shopping == ['cabbages', 'bananas', 'apples']
+
+
+def test_act():
+    nothing = None
+
+    with pytest.raises(AttributeError):
+        # You can't get something from nothing
+        nothing.get_something()
+
+
+# --- OTHERS ---
+
+
+def test_comment_after_act() -> None:
+    x = 1
+    y = 2
+
+    result = x + y
+    # Now check result
+
+    assert result == 3
+
+
+def test_raises():
+    nothing = None
+
+    with pytest.raises(AttributeError):
+        # You can't get something from nothing
+        nothing.get_something()

examples/good/black/test_comments.py

@@ -1,31 +1,99 @@
-# Test for various comments before and after test blocks
+from typing import List
 
+# Example from AAA06 doc:
+# * inline comment is OK for marking
 
-def test_comment_before_act():
-    x = 1
-    y = 2
 
-    # Sum x and y
-    result = x + y
+def test() -> None:
+    """
+    Reverse shopping list operates in place
+    """
+    shopping = ["apples", "bananas", "cabbages"]
+
+    shopping.reverse()  # act
+
+    assert shopping == ["cabbages", "bananas", "apples"]
+
+
+def test_ignore_typing() -> None:
+    """
+    Reverse shopping list operates in place
+    """
+    shopping = ["apples", "bananas", "cabbages"]
+
+    result = shopping.reverse()  # type: ignore
 
-    assert result == 2
+    assert result is None
+    assert shopping == ["cabbages", "bananas", "apples"]
 
 
-def test_comment_after_act():
+# Comments are OK in Arrange and Assert.
+
+
+def test_in_arrange():
+    x = 3
+    # Let's make a 3, 4, 5 triangle
+    y = 4
+
+    result = x ** 2 + y ** 2
+
+    assert result == 25
+
+
+def test_end_arrange() -> None:
     x = 1
     y = 2
+    # Now test...
 
     result = x + y
-    # Sum x and y
 
-    assert result == 2
+    assert result == 3
+
+
+def test_in_assert():
+    result = list()
 
+    assert not result
+    # A copy of nothing is nothing
+    assert result.copy() == []
 
-def test_comment_before_assert():
+
+def test_startassert() -> None:
     x = 1
     y = 2
 
     result = x + y
 
-    # Always 2
-    assert result == 2
+    # Always 3
+    assert result == 3
+
+
+# Comments are OK in strings
+
+
+def test_strings() -> None:
+    special_chars = """
+    #!$[]{}
+    """.strip()
+
+    result = len(special_chars)
+
+    assert result == 7
+
+
+def test_string_act():
+    result = """
+# Not a comment - it's a string
+"""
+
+    assert len(result) == 33
+
+
+# Comment are OK before the test
+
+
+# NOTE: igore this comment
+def test_empty() -> None:
+    result: List[int] = list()
+
+    assert len(result) == 0