Skip to content

Remove test warnings & logline, prevent introducing them again in the future#684

Merged
MarkKoz merged 3 commits into
mainfrom
kill-test-warnings
Mar 2, 2022
Merged

Remove test warnings & logline, prevent introducing them again in the future#684
MarkKoz merged 3 commits into
mainfrom
kill-test-warnings

Conversation

@jchristgit
Copy link
Copy Markdown
Contributor

Closes #671

@jchristgit jchristgit added area: backend Related to internal functionality and utilities area: tests Related to `unittest` tests priority: 2 - normal Normal Priority level: 2 - advanced python Used by dependabot type: enhancement Changes or improvements to existing features labels Feb 26, 2022
@netlify
Copy link
Copy Markdown

netlify Bot commented Feb 26, 2022

✔️ Deploy Preview for pydis-static ready!

🔨 Explore the source changes: 2734a68

🔍 Inspect the deploy log: https://app.netlify.com/sites/pydis-static/deploys/621fbb854245670007fafb91

😎 Browse the preview: https://deploy-preview-684--pydis-static.netlify.app

@coveralls
Copy link
Copy Markdown

coveralls commented Feb 26, 2022

Coverage Status

Coverage remained the same at 100.0% when pulling 2734a68 on kill-test-warnings into 193f0a9 on main.

Comment thread pydis_site/settings.py
Comment on lines +52 to +64
# Prevent verbose warnings emitted when passing a non-timezone aware
# datetime object to the database, whilst we have time zone support
# active. See the Django documentation for more details:
# https://docs.djangoproject.com/en/dev/topics/i18n/timezones/
warnings.filterwarnings(
'error', r"DateTimeField .* received a naive datetime",
RuntimeWarning, r'django\.db\.models\.fields',
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wait wait wait, I don't get it. You're solving this problem so that we're passing timezone-aware objects, right? Why do we then still need to surpress the warnings?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this isn't suppressing them, this is basically saying "error out instead of warning for this message from that warning from that module", so it raises that instead of printing the warning

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is a win, and we are all part of it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ohhh. It'll raise a RuntimeWarning instead of silently warning, that's the thing?

Okay, I like it, but maybe we can improve the comment a little bit?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

nonononono, it will raise an error because it got that runtime warning, that's why I pass 'error' as the first argument. I actually stole this, I did not write it myself

Comment thread pydis_site/apps/api/viewsets/bot/infraction.py Outdated
Comment thread pydis_site/apps/api/viewsets/bot/infraction.py Outdated
@jchristgit jchristgit requested a review from MarkKoz March 2, 2022 18:46
Add a `warnings.warnings` clause to prevent these from being raised
again in the future, and raise a full traceback if they don't.
@jchristgit jchristgit force-pushed the kill-test-warnings branch from b78bdd0 to 2734a68 Compare March 2, 2022 18:46
Copy link
Copy Markdown
Contributor

@MarkKoz MarkKoz left a comment

Choose a reason for hiding this comment

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

yesyesyesyesyes!

@MarkKoz MarkKoz merged commit e5b2750 into main Mar 2, 2022
@MarkKoz MarkKoz deleted the kill-test-warnings branch March 2, 2022 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: backend Related to internal functionality and utilities area: tests Related to `unittest` tests level: 2 - advanced priority: 2 - normal Normal Priority python Used by dependabot type: enhancement Changes or improvements to existing features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove useless output from manage.py test

4 participants