Fix tests depending on year 2024 #473
Conversation
There was a problem hiding this comment.
I think it is beacuse the mock patches django.utils but the the ShareOwner object is initiated with datetime.date.today() in line 111, which is not recommended in Django because of timezone-inawareness. Better
def with_status(self, status: str, date=None):
if date is None:
date = timezone.now().today()
Maybe you can have a look if you can find more if these within models
|
I would rather keep the mocked time to have the tests date-independant. Even if we will never reach the new years you set, it's a bit nicer to have believable date ranges. Have you tried @crosspolar 's suggestion already? |
|
@Theophile-Madet Yes, I tried that already. Check #475. |
There was a problem hiding this comment.
We're getting a lot of warnings like this one:
tapir/shifts/tests/test_calendars.py::TestCalendars::test_day_printable_view_status_200
/root/.cache/pypoetry/virtualenvs/tapir-9TtSrW0h-py3.11/lib/python3.11/site-packages/django/db/models/fields/__init__.py:1416: RuntimeWarning: DateTimeField Shift.end_time received a naive datetime (2023-06-15 16:00:00) while time zone support is active.
warnings.warn("DateTimeField %s received a naive datetime (%s)"
Looks like we should be avoiding constructs like: |
| @@ -299,7 +298,7 @@ def overlapping_with(self, obj) -> DurationModelMixinQuerySet: | |||
| def active_temporal(self, effective_date=None) -> DurationModelMixinQuerySet: | |||
| if not effective_date: | |||
| # if no effective date was given, use today as the default | |||
| effective_date = date.today() | |||
| effective_date = timezone.now() | |||
There was a problem hiding this comment.
Should this maybe be timezone.now().date()? The DurationModelMixin uses date fields, not datetime fields.
Though I'm not sure if dates can be timezone-aware
There was a problem hiding this comment.
Looks like the date class doesn't have a timezone. But do you see any problem with that?
There was a problem hiding this comment.
datetime.date.today() returns the local time, not UTC. I haven't thought it through, but it sounds like people could have an offset of 1 or 2 hours?!
Anyway, maybe it's parsed automatically or we ensure timezone-aware dates by timezone.now().date() (which is using UTC and not local time)
| @@ -108,7 +108,7 @@ def with_name(self, search_string: str): | |||
|
|
|||
| def with_status(self, status: str, date=None): | |||
| if date is None: | |||
| date = datetime.date.today() | |||
| date = timezone.now() | |||
There was a problem hiding this comment.
Same question as in DurationModelMixinQuerySet
The correct way of getting time is:
timezone.now().(source: https://docs.python.org/3/library/datetime.html#datetime.datetime.now)