fix same day compat #563
fix same day compat #563
Conversation
allenporter
left a comment
allenporter
left a comment
There was a problem hiding this comment.
Thank you, these tests are very strong. One quick comment and I can get this merged and released for you.
ical/compat/make_compat.py
Outdated
| timezone_compat.enable_extended_timezones(), | ||
| ): | ||
| yield ics | ||
| elif prodid and _CALENDAR_LABS_PRODID in prodid: |
There was a problem hiding this comment.
I'm OK to enable this for all calendars given as you said it's a common problem. Can you apply in the outer scope and remove the "No compatibility mode needed" debug line?
There is a test failure |
allenporter
left a comment
allenporter
left a comment
There was a problem hiding this comment.
The tests don't pass and i have some requested changes below.
tests/compat/test_make_compat.py
Outdated
|
|
||
| # Normalize DTSTAMP for calendar_labs files since it's auto-generated | ||
| if "calendar_labs" in filename.stem: | ||
| import re |
There was a problem hiding this comment.
The style in this code base is that all imports are at the top of the file, not randomly in the code like AI likes to insert.
tests/compat/test_make_compat.py
Outdated
| assert timezone_compat.is_allow_invalid_timezones_enabled() | ||
| assert timezone_compat.is_extended_timezones_enabled() | ||
| # Only check timezone compatibility for Exchange Server files | ||
| if "office_3" in filename.stem: |
There was a problem hiding this comment.
Please change the text fixtures instead of doing this. That is, lets have different tests with different file globs for the test that apply. I don't want an accidental mistake related to the filename to mask this.
allenporter
left a comment
allenporter
left a comment
There was a problem hiding this comment.
Looking good, one final small change.
| if "calendar_labs" in filename.stem: | ||
| import re | ||
| new_ics = re.sub(r'DTSTAMP:\d{8}T\d{6}Z', 'DTSTAMP:20260103T212907Z', new_ics) | ||
| new_ics = re.sub(r'DTSTAMP:\d{8}T\d{6}Z', 'DTSTAMP:20260103T212907Z', new_ics) |
There was a problem hiding this comment.
Use a fixture like this to set the timestamp used for dtstamp:
@pytest.fixture(name="frozen_time", autouse=True)
def mock_frozen_time() -> Generator[FrozenDateTimeFactory, None, None]:
"""Fixture to freeze time to a specific point."""
with freeze_time("2022-09-03T09:38:05") as freeze:
with patch("ical.event.dtstamp_factory", new=freeze):
yield freeze
3 participants
No description provided.