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

py38: replace invalid assertRaises kwargs with simpler assertRaisesMessage #28696

Merged
merged 2 commits into from
Sep 20, 2021

Conversation

joshuarli
Copy link
Member

In Python 3.8, regex and expected_regex are invalid kwargs to assertRaises. django.test.SimpleTestCase has a simpler and faster assertRaisesMessage - we already have that in our base TestCase but for tests that are using unittest.TestCase, can just use SimpleTestCase.

…e which still has assertRaisesMessage

also this regex isn't correct and always passes, lol
@joshuarli joshuarli requested review from a team and removed request for a team September 20, 2021 21:13
@@ -384,12 +384,15 @@ def test_allowed_keys(self):
]

def test_invalid_aggregate_column_with_duration_filter(self):
with self.assertRaises(InvalidSearchQuery, regex="not a duration column"):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This regex= was always broken, on 3.6 you can change it to "foo" and it still passes, lol.

If you change it to the correct self.assertRaisesRegexp(InvalidSearchQuery, expected_regex="foo"):

E   AssertionError: "foo" does not match "avg(stack.colno):
column argument invalid: stack.colno is not a numeric column"

@joshuarli joshuarli merged commit af88cef into master Sep 20, 2021
@joshuarli joshuarli deleted the py38/no-expected_regex-in-py38-unittest branch September 20, 2021 22:21
@github-actions github-actions bot locked and limited conversation to collaborators Oct 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants