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

unit test assertion fixes #1203

Merged
merged 1 commit into from
Jul 28, 2023
Merged

unit test assertion fixes #1203

merged 1 commit into from
Jul 28, 2023

Conversation

azmeuk
Copy link
Contributor

@azmeuk azmeuk commented Jul 28, 2023

Playing with the pytest branch I found that there were occurences of self.assertTrue(200, resp.status_code) style assertions in the tests. This really looks like someone intended to write self.assertEqual instead, as testing that any number is True will always pass 🙈

ihatemoney/tests/api_test.py Outdated Show resolved Hide resolved
`self.assertTrue(200, resp.status_code)` style are always True
and thus are useless. It looks like the original author wanted
`self.assertEqual` there instead.
@almet
Copy link
Member

almet commented Jul 28, 2023

Haha, you're right, and I'm probably the one who introduced this a few years back. Thanks for the changes.

@almet almet merged commit b1d4f34 into spiral-project:master Jul 28, 2023
@azmeuk azmeuk deleted the unittest-fix branch July 28, 2023 15:44
@zorun
Copy link
Collaborator

zorun commented Jul 28, 2023

Good catch! That's actually a good argument in favour of pytest-style plain asserts...

TomRoussel pushed a commit to TomRoussel/ihatemoney that referenced this pull request Mar 2, 2024
`self.assertTrue(200, resp.status_code)` style are always True
and thus are useless. It looks like the original author wanted
`self.assertEqual` there instead.
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

Successfully merging this pull request may close these issues.

3 participants