Resolve context manager issues with Black v23.1.0 formatted tests raising AAA03
jamescooke opened this issue ยท 6 comments
Problematic code
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_with():
a_class = AClass()
with freeze_time("2021-02-02 12:00:02"):
result = a_class.action('test')
assert result == 'test'
Black wants to remove the blank link after the context manager and the action, resulting in:
def test_with():
a_class = AClass()
with freeze_time("2021-02-02 12:00:02"):
result = a_class.action("test")
assert result == "test"
However, current Flake8-AAA raises AAA03
because there is no blank line before the Act block ๐
test.py|4 col 9| AAA03 expected 1 blank line before Act block, found none
Possible solution
One possible solution is to allow for a "greedy context manager" - the freeze_time()
context is allowed to join the Act block, even though it's really arrangement:
def test_with():
a_class = AClass()
with freeze_time("2021-02-02 12:00:02"):
result = a_class.action("test")
assert result == "test"
This passes Black, but would need some fixing on the Flake8-AAA side.
One option would be to create a setting greedy_context_managers
or similar which can be set to true
for Black users, but I'm concerned that the effort required to test and manage such a setting would be a weight on Flake8-AAA long term.
Greedy context managers were removed in #146 released in 0.10.0
, see https://github.com/jamescooke/flake8-aaa/blob/master/CHANGELOG.rst#changed-4
Paper trail
- Black issue about inconsistent empty line handling: psf/black#3300
- Black stable style PR: psf/black#3302
- lyz-code/autoimport@92db729 ๐๐ป Fixes in the wild ๐
Until this is solved a workaround is to add a comment:
def test_with():
a_class = AClass()
with freeze_time("2021-02-02 12:00:02"):
# Remove once https://github.com/jamescooke/flake8-aaa/issues/200 is solved
result = a_class.action('test')
assert result == 'test'
Plan Of Action ๐๐ป
I'm hoping that I can roll out a fix across two releases...
Add a setting, next release 0.14.1
- New setting added for specifying how Act blocks and context managers behave:
aaa_act_block_style
. - Setting default will be
'default'
. In this mode context managers that wrap Act blocks remain in the Arrange block, leaving the Act block small:def test_with(): a_class = AClass() with freeze_time("2021-02-02 12:00:02"): result = a_class.action('test') assert result == 'test'
- This way of handling context managers is the current behaviour. Therefore, this release can be rolled out as a patch release.
Add a 'large'
option for Act blocks, second release 0.15.0
- New option added for
aaa_act_block_style
setting:'large'
. - Act blocks get large by absorbing the context managers that wrap them.
Setting this "large" behaviour resolves will resolve the formatting issues with Black.
def test_with(): a_class = AClass() with freeze_time("2021-02-02 12:00:02"): result = a_class.action("test") assert result == "test"
- Adding
'large'
option toaaa_act_block_style
setting won't be a breaking change, so this will be rolled out as a minor version.
Edits
- 2023/04/11: Prefixed setting with
aaa_
so that it becomesaaa_act_block_style
. This creates a namespace for Flake8-AAA settings. My guess is that there will be a setting for using result variables called something other thanresult
, e.g.r
. - 2023/04/13: Commit where fat Act blocks were removed: f9e5edb
- 2023/04/17: Adjust options to be
--aaa-act-block-style=[default|large]
, i.e. rename "thin" to "default", rename "fat" to "large".
@lyz-code If you have some time to test out master, passing --aaa-act-block-style=large
should give you some compatibility with Black. Please let me know how you get on ๐๐ป
I'll try it out with some of my internal projects tomorrow and hopefully write up docs for a release soon, all being well ๐ค๐ป .
Hey @jamescooke thanks for the detailed issue evolution, it's very helpful. Sadly I'm a bit overwhelmed these days so sadly I'm not sure if I'll have the time to try it :(.
Thank you so much for developing the fix though <3
Have experimented with the current master on a couple of projects using Black and everything seems to work OK.
Only one caveat: Just formatting the tests with Black is not enough when those tests have context managers. When the context manager has joined the Act block, then there will need to be a blank line added above it to resolve the "AAA03 expected 1 blank line before Act block, found none" errors.
I'll prep release 0.15.0
now.
Thank you so much <3