Refactor conditional raise assertion in tests

[akaihola]

• refactor context manager conditionals into a helper
• fix test failures for Python 3.8
• fix test failures for Python 3.7
• fix test failures for Python 3.6

Some parameterized tests have both values and exceptions as expected outcomes. With recent Pytest and Mypy versions, the type of the conditional context manager doesn't check when written using Python's ternary operator. We get these errors when Pytest runs Mypy checks for Darker:

__________________ src/darker/tests/test_argparse_helpers.py ___________________
33: error: "object" has no attribute "__enter__"
33: error: "object" has no attribute "__exit__"
____________________ src/darker/tests/test_command_line.py _____________________
31: error: "object" has no attribute "__enter__"
31: error: "object" has no attribute "__exit__"
86: error: "object" has no attribute "__enter__"
86: error: "object" has no attribute "__exit__"
____________________ src/darker/tests/test_main_revision.py ____________________
102: error: "object" has no attribute "__enter__"
102: error: "object" has no attribute "__exit__"

The problem is that pytest.raises and contextlib.nullcontext in an ... if ... else ... ternary operator have no common ancestor classes, so Mypy assigns it the type object. To reproduce, create this Python source file:

# ternary_context_manager.py

import pytest
from contextlib import nullcontext

def myfunc(should_raise: bool) -> None:
    cm = pytest.raises(Exception) if should_raise else nullcontext()
    reveal_type(cm)
    with cm:
        pass

Analyze it with Mypy in a Python 3.8 virtualenv:

python3.8 -m venv venv
. venv/bin/activate
pip install pytest==6.1.1 mypy==0.782
mypy ternary_context_manager.py

You'll get:

ternary_context_manager.py:8: note: Revealed type is 'builtins.object'
ternary_context_manager.py:9: error: "object" has no attribute "__enter__"
ternary_context_manager.py:9: error: "object" has no attribute "__exit__"
Found 2 errors in 1 file (checked 1 source file)

The with statement expects the expression to have __enter__ and __exit__ methods (i.e. to be a context manager), which object of course doesn't have. At run time this isn't a problem, but for Mypy it is.

Fix this by refactoring the conditional into a helper function which returns different context managers using an if..else instead. This also reduces duplication of code in tests.

[CorreyL]

Happy to take a look at this after my shift today!

[Mystic-Mirage]

Looks nice

[CorreyL]

Changes seem reasonable to me, especially given how much they've tightened up existing tests.

[akaihola]

While continuing to experiment with different Python, Pytest and Mypy versions, I expanded the description of this PR with a simple way to reproduce the original situation.

[akaihola]

Also Mypy now fails on Python 3.6 and 3.7. Some clever solution is needed in tests/helpers.py so Mypy is happy across Python versions.

Update: Solved, see reply below.

[sourcery-ai]

Sourcery Code Quality Report

This PR has an average code quality of 71.86%

Quality metrics	Before	After	Change
Complexity		0.65 ⭐
Method Length		93.20 🙂
Working memory		13.80 😞
Quality	%	71.86% 🙂	%

Other metrics	Before	After	Change
Lines		561

Changed files	Quality Before	Quality After	Quality Change
src/darker/tests/helpers.py	%	89.32% ⭐	%
src/darker/tests/test_argparse_helpers.py	%	63.23% 🙂	%
src/darker/tests/test_command_line.py	%	72.95% 🙂	%
src/darker/tests/test_main_revision.py	%	40.87% 😞	%

Here are some functions in these files that still need a tune-up:

File	Function	Complexity	Length	Working Memory	Quality	Recommendation
src/darker/tests/test_command_line.py	test_parse_command_line	4 ⭐	469 ⛔	60 ⛔	30.03% 😞	Try splitting into smaller methods. Extract out complex expressions
src/darker/tests/test_main_revision.py	test_revision	1 ⭐	236 ⛔	24 ⛔	40.87% 😞	Try splitting into smaller methods. Extract out complex expressions
src/darker/tests/test_command_line.py	test_black_options	0	194 😞	20 ⛔	46.59% 😞	Try splitting into smaller methods. Extract out complex expressions
src/darker/tests/test_command_line.py	test_options	0	133 😞	22 ⛔	51.46% 🙂	Try splitting into smaller methods. Extract out complex expressions
src/darker/tests/test_command_line.py	test_black_options_skip_string_normalization	0	156 😞	14 😞	55.68% 🙂	Try splitting into smaller methods. Extract out complex expressions

Legend and Explanation

The emojis denote the absolute quality of the code:

• ⭐ excellent
• 🙂 good
• 😞 poor
• ⛔ very poor

The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request.

---

Please see our documentation here for details on how these metrics are calculated.

We are actively working on this report - lots more documentation and extra metrics to come!

Let us know what you think of it by mentioning @sourcery-ai in a comment.

[akaihola]

Mypy now fails on Python 3.6 and 3.7

Fixed by updating Pytest to >6.1.0 which introduced type hints for the _pytest.python_api module.

And by the way, I deem the import from private package _pytest acceptable in the test suite. It won't break Darker itself even if that API is changed. We'll only need to fix the test suite if that happens.