Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A batch of issues, with thanks #54

Open
adamchainz opened this issue Sep 3, 2024 · 0 comments
Open

A batch of issues, with thanks #54

adamchainz opened this issue Sep 3, 2024 · 0 comments

Comments

@adamchainz
Copy link

Hi

Thanks for the tool. I just used it to mass-modernize asserts across a ~250k line project.

I found a bunch of bugs on the way. Rather than spam you with loads of individual issues, I’m reporting them all here. Happy to split them if you’d prefer, but I see development isn’t ongoing, so I don’t want to burden you.

I also see that Ruff has some powers to do this rewriting now, such as from its pytest-unittest-assertion rule, so maybe this tool is less relevant now.

Anyway, here’s the list, and thanks again!

  1. It added a second isinstance argument outside of parentheses:
assert isinstance(
    response.context_data["inline_admin_formset"].formset
), ClickToRenderInlineFormSetBase
  1. Inserted a \ before a comment, which is a SyntaxError:
assert test_count.short_description == \  # type: ignore[union-attr, attr-defined]
            "test relation"
  1. Badly indented a comment, resulting in:
    assert something == \
                something2
# some comment
  1. The rewriting of == True to is True didn’t work in many cases, same for False / None:
assert form["file_reviewed"].value() == True
  1. It incorrectly ported the places argument to pytest.approx when not provided as a keyword argument:
self.assertAlmostEqual(x, y, 2)
assert x = pytest.approx(y), 2
  1. It incorrectly placed is not None when there was a msg argument:
-self.assertIsNotNone(x, y)
+assert x, y is not None
  1. It incorrectly placed not for an expression, which needed parnthesizing:
-self.assertFalse(a and b)
+assert not a and b
  1. It poorly rewrote this bad single-argument assertEqual call:
def test_example(self):
    with self.assertRaises(NotImplementedError):
        self.assertEqual(BaseDoor().open())

(This call didn't get a chance to crash at runtime because the NotImplementedError was raised first.)

...it ended up attaching the == for the assertEqual later on in the file,.

  1. It inserted \ into multiline strings, changing their contents:
-self.assertEqual(
-    something,
-    """
-    bla
-    bla
-    """
-)
+assert \
+    something == \
+    """ \
+    bla \
+    bla \
+    """ \
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant