Act block with context not recognised correctly
k0nG opened this issue · 6 comments
Describe the bug
When I've got a with
statement providing context as part of an act block and I have a arrange block above I get flake8-aaa
errors when I don't think I should.
I encountered this when upgrading from flake8-aaa
0.11.1
to 0.12.2
Share debugging output
With # act
comment:
------+------------------------------------------------------------------------
24 DEF|def test_with():
25 ARR| a_class = AClass()
26 BL |
^ AAA05 blank line in block
27 ARR| with freeze_time("2021-02-02 12:00:02"): # act
28 ACT| result = a_class.action('test')
^ AAA03 expected 1 blank line before Act block, found none
29 BL |
30 ASS| assert result == 'test'
------+------------------------------------------------------------------------
2 | ERRORS
======+========================================================================
FAILED with 2 ERRORS
without # act
comment:
------+------------------------------------------------------------------------
24 DEF|def test_with():
25 ARR| a_class = AClass()
26 BL |
^ AAA05 blank line in block
27 ARR| with freeze_time("2021-02-02 12:00:02"):
28 ACT| result = a_class.action('test')
^ AAA03 expected 1 blank line before Act block, found none
29 BL |
30 ASS| assert result == 'test'
------+------------------------------------------------------------------------
2 | ERRORS
======+========================================================================
FAILED with 2 ERRORS
Flake8:
4.0.1 (aaa: 0.12.2, mccabe: 0.6.1, pycodestyle: 2.8.0, pyflakes: 2.4.0) CPython 3.8.5 on Darwin
Please share the Python version and platform you are using:
- Python version (output of
python --version
): Python 3.8.5 - Platform (GNU Linux / Mac OSX / Windows): OSX
Expected behavior
Expect to distinguish an act block with a with
statement wrapping the test action. This seems to happen for me with with freeze_time(""):
but it's ok when using with pytest.raises
Additional context
Using:
freezegun = "1.2.1"
Source code:
from freezegun import freeze_time
class AClass():
def action(a):
return a
def test_with():
a_class = AClass()
with freeze_time("2021-02-02 12:00:02"):
result = a_class.action('test')
assert result == 'test'
Thanks for raising this - it probably should be better documented in the "rules and error codes" which will be handled by #191
This behaviour was changed in #146 < which contains further examples.
TL;DR the context manager starting with the with
clause is considered arrangement by Flake8-AAA. In this case, with freeze_time()
is adjusting underlying Python time-related return values, so it's arranging for the test. The debug output is hinting at that when it shows you ARR
on the line here:
27 ARR| with freeze_time("2021-02-02 12:00:02"):
Flake8-AAA wants all the ARR
lines together, then a blank line, then the ACT
lines, so reformatting your test would give:
def test_with():
a_class = AClass()
with freeze_time("2021-02-02 12:00:02"):
result = a_class.action('test')
assert result == 'test'
Does that formatting work in your project?
Thanks for the really quick reply.
Yes, that does indeed work. Sorry, I did try something like that after reading #146 , but for some reason it didn't work.
I've updated my tests now and they are all passing in flake8-aaa
.
Thanks!
As of Black 23.1.0
(and possibly earlier versions), the solution of separating the context manager into the Arrange block and keeping the Act block simple won't work because Black now complains about the blank line after the context manager:
def test(cli_runner, audit_path: pathlib.Path) -> None:
with mock.patch("validate.__main__.download", new=fake_download(audit_path)):
-
result = cli_runner.invoke(main, ["all", "201901", "-v"])
assert result.exit_code == 0
Black wants to remove the blank line.
I'm not going to reopen this, but will create a new ticket.
Paper trail:
- Black issue about inconsistent empty line handling: psf/black#3300
- Black stable style PR: psf/black#3302
Hi @jamescooke thank you for handling the black
issue (it's driving me crazy) can you put a link here to the newly created issue once it's done so we can follow it?
Thanks
New issue for Black and context managers is here: #200