Skip to content

Fix test with Django 5 when pytz is available #9715

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.

Sign up for GitHub

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Fix test with Django 5 when pytz is available #9715

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
656d835
Fix test with Django 5 when pytz is available
kulikjak Jun 4, 2025
4e42975
fix formatting
kulikjak Jun 4, 2025
5b25346
remove original condition
kulikjak Jun 5, 2025
3817fb9
remove trailing whitespace
kulikjak Jun 5, 2025
d3ff351
further improvements
kulikjak Jun 5, 2025
a0fbdbd
let's not skip the pytz test - it should always be executed when test…
kulikjak Jun 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions requirements/requirements-testing.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ pytest-django>=4.5.2,<5.0
importlib-metadata<5.0
# temporary pin of attrs
attrs==22.1.0
pytz
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add a note here, similar to

"Django 5.0 has removed pytz; this test should eventually be able to get removed."

from the skip reason?

Copy link
Member

Choose a reason for hiding this comment

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

Sure, we can, won't hurt. How about this?

Suggested change
pytz
pytz # Remove when dropping support for Django<5.0

However, if I was making the change to drop support for Django<5.0, I'd probably remove the skip from the test and forget about this file... Would help to clean it up later in case we forget though 👍🏻

6 changes: 5 additions & 1 deletion tests/test_fields.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
except ImportError:
pytz = None
Copy link
Member

Choose a reason for hiding this comment

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

This try/except block is no longer need, is it? The assumption is that we should always have pytz installed when running the test, no?


import django
from django.core.exceptions import ValidationError as DjangoValidationError
from django.db.models import IntegerChoices, TextChoices
from django.http import QueryDict
Expand Down Expand Up @@ -1624,7 +1625,10 @@ def test_should_render_date_time_in_default_timezone(self):
assert rendered_date == rendered_date_in_timezone


@pytest.mark.skipif(pytz is None, reason="Django 5.0 has removed pytz; this test should eventually be able to get removed.")
@pytest.mark.skipif(
Copy link
Member

Choose a reason for hiding this comment

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

I've just expanded the diff further down and realised that we can probably remove the if pytz: at line 1645...

condition=django.VERSION >= (5,),
reason="Django 5.0 has removed pytz; this test should eventually be able to get removed.",
)
class TestPytzNaiveDayLightSavingTimeTimeZoneDateTimeField(FieldValues):
"""
Invalid values for `DateTimeField` with datetime in DST shift (non-existing or ambiguous) and timezone with DST.
Expand Down
Loading