Skip to content

Commit

Permalink
Remove directly imported skipIf and skipUnless (#499)
Browse files Browse the repository at this point in the history
For #364.
  • Loading branch information
adamchainz authored Oct 10, 2024
1 parent 666a6ce commit 14eae88
Show file tree
Hide file tree
Showing 4 changed files with 160 additions and 16 deletions.
6 changes: 4 additions & 2 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -232,12 +232,12 @@ Like the above, it requires comparisons of the form:
Supports these test skip decorators:

* |unittest.skipIf|__:
* |unittest.skipIf|__

.. |unittest.skipIf| replace:: ``@unittest.skipIf``
__ https://docs.python.org/3/library/unittest.html#unittest.skipIf

* |unittest.skipUnless|__:
* |unittest.skipUnless|__

.. |unittest.skipUnless| replace:: ``@unittest.skipUnless``
__ https://docs.python.org/3/library/unittest.html#unittest.skipUnless
Expand All @@ -246,6 +246,8 @@ For example:

.. code-block:: diff
import unittest
import django
from django.test import TestCase
Expand Down
5 changes: 4 additions & 1 deletion src/django_upgrade/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,10 @@ def visit(
and node.level == 0
and (
node.module is not None
and (node.module.startswith("django.") or node.module == "django")
and (
node.module.startswith("django.")
or node.module in ("django", "unittest")
)
)
):
state.from_imports[node.module].update(
Expand Down
41 changes: 35 additions & 6 deletions src/django_upgrade/fixers/versioned_test_skip_decorators.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,19 @@
"""
Drop test skip decorators for old Django versions like:
import django
from django.test import TestCase
class ExampleTests(TestCase):
@unittest.skipIf(django.VERSION < (5, 1), "Django 5.1+")
def test_one(self):
...
@unittest.skipUnless(django.VERSION >= (5, 1), "Django 5.1+")
def test_two(self):
...
"""

from __future__ import annotations

import ast
Expand Down Expand Up @@ -31,19 +47,32 @@ def visit_FunctionDef(
for decorator in node.decorator_list:
if (
isinstance(decorator, ast.Call)
and isinstance(decorator.func, ast.Attribute)
and isinstance(decorator.func.value, ast.Name)
and decorator.func.value.id == "unittest"
and decorator.func.attr in ("skipIf", "skipUnless")
and (
(
isinstance(decorator.func, ast.Attribute)
and isinstance(decorator.func.value, ast.Name)
and decorator.func.value.id == "unittest"
and (unittest_decorator := decorator.func.attr)
in ("skipIf", "skipUnless")
)
or (
isinstance(decorator.func, ast.Name)
and (
(unittest_decorator := decorator.func.id)
in ("skipIf", "skipUnless")
)
and unittest_decorator in state.from_imports["unittest"]
)
)
and len(decorator.args) == 2
and isinstance(decorator.args[0], ast.Compare)
and (
(pass_fail := is_passing_comparison(decorator.args[0], state))
is not None
)
and (
(decorator.func.attr == "skipIf" and pass_fail == "fail")
or (decorator.func.attr == "skipUnless" and pass_fail == "pass")
(unittest_decorator == "skipIf" and pass_fail == "fail")
or (unittest_decorator == "skipUnless" and pass_fail == "pass")
)
):
yield ast_start_offset(decorator), partial(erase_decorator, node=decorator)
Expand Down
124 changes: 117 additions & 7 deletions tests/fixers/test_versioned_test_skip_decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
check_transformed = partial(tools.check_transformed, settings=settings)


def test_unittest_skip_left():
def test_unittest_attr_skip_left():
check_noop(
"""\
import unittest
Expand All @@ -22,7 +22,7 @@ def test_thing(self):
)


def test_unittest_skipIf_too_few_args():
def test_unittest_attr_skipIf_too_few_args():
check_noop(
"""\
import unittest
Expand All @@ -35,7 +35,7 @@ def test_thing(self):
)


def test_unittest_skipIf_too_many_args():
def test_unittest_attr_skipIf_too_many_args():
check_noop(
"""\
import unittest
Expand All @@ -48,7 +48,7 @@ def test_thing(self):
)


def test_unittest_skipIf_passing_comparison():
def test_unittest_attr_skipIf_passing_comparison():
check_noop(
"""\
import unittest
Expand All @@ -61,7 +61,7 @@ def test_thing(self):
)


def test_unittest_skipIf_unknown_comparison():
def test_unittest_attr_skipIf_unknown_comparison():
check_noop(
"""\
import unittest
Expand All @@ -74,7 +74,20 @@ def test_thing(self):
)


def test_unittest_skipUnless_failing_comparison():
def test_unittest_bare_skipIf_passing_comparison():
check_noop(
"""\
from unittest import skipIf
import django
@skipIf(django.VERSION < (4, 2), "Django 4.2+")
def test_thing(self):
pass
""",
)


def test_unittest_attr_skipUnless_failing_comparison():
check_noop(
"""\
import unittest
Expand All @@ -87,7 +100,20 @@ def test_thing(self):
)


def test_unittest_skipIf_removed():
def test_unittest_bare_skipUnless_failing_comparison():
check_noop(
"""\
from unittest import skipUnless
import django
@skipUnless(django.VERSION >= (4, 2), "Django 4.2+")
def test_thing(self):
pass
""",
)


def test_unittest_attr_skipIf_removed():
check_transformed(
"""\
import unittest
Expand All @@ -107,6 +133,49 @@ def test_thing(self):
)


def test_unittest_bare_skipIf_removed():
check_transformed(
"""\
from unittest import skipIf
import django
@skipIf(django.VERSION < (4, 1), "Django 4.1+")
def test_thing(self):
pass
""",
"""\
from unittest import skipIf
import django
def test_thing(self):
pass
""",
)


def test_unittest_skipIf_mixed():
check_transformed(
"""\
import unittest
from unittest import skipIf
import django
@unittest.skipUnless(django.VERSION >= (4, 1), "Django 4.1+")
@skipIf(django.VERSION < (4, 1), "Django 4.1+")
def test_thing(self):
pass
""",
"""\
import unittest
from unittest import skipIf
import django
def test_thing(self):
pass
""",
)


def test_skipUnless_removed():
check_transformed(
"""\
Expand All @@ -125,3 +194,44 @@ def test_thing(self):
pass
""",
)


def test_unittest_bare_skipUnless_removed():
check_transformed(
"""\
from unittest import skipUnless
import django
@skipUnless(django.VERSION >= (4, 1), "Django 4.1+")
def test_thing(self):
pass
""",
"""\
from unittest import skipUnless
import django
def test_thing(self):
pass
""",
)


def test_unittest_bare_skipIf_skipUnless_mixed():
check_transformed(
"""\
from unittest import skipIf, skipUnless
import django
@skipUnless(django.VERSION >= (4, 1), "Django 4.1+")
@skipIf(django.VERSION < (4, 1), "Django 4.1+")
def test_thing(self):
pass
""",
"""\
from unittest import skipIf, skipUnless
import django
def test_thing(self):
pass
""",
)

0 comments on commit 14eae88

Please sign in to comment.