tests: async fixcalendar consolidation - #703
Merged
Merged
Conversation
`test_utf8_event` failed against one of the external test servers. `adelete_calendar_if_present()` left the calendar in place, so the following `make_calendar()` got a 405 "A collection already exists at that location". The server is configured with `delete-calendar: false`, so the calendar was wiped rather than deleted. The sync suite doesn't have this problem: everything there goes through `_fixCalendar()`, which tolerates a failed MKCALENDAR by reusing whatever is already there. The async tests had no equivalent - four fixtures and four tests each open-coded their own variant. New in `tests/fixture_helpers.py`: * `afix_calendar()` - the async counterpart of `_fixCalendar`. * `arelease_calendar()` - the matching teardown * `atry_principal()` - principal discovery, `None` when the server won't do it. Some various minor bugs and issues in the test framework was found by the review agent and fixed. Prompt: We have an issue with (...)/tests/test_async_integration.py(2365)test_utf8_event() (...) the `await principal.make_calendar(name="Yølp", cal_id=cal_id)` fails on the jeanes server (`pytest -k 'jeanes and utf8'`) saying that the calendar already exists, meaning that the above line `await adelete_calendar_if_present(principal, cal_id)` didn't work out. I think that for the sync framework we have a _fixCalendar method or something like that doing things, please check if the "create me a calendar"-logic in the async tests can be consolidated the same way. Followup-Prompt: /review-and-push and create a PR Followup-Prompt: Fix [review findings] Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Synology Calendar turns out to embed a modified DAViCal. The one real divergence from upstream DAViCal is calendar deletion: DSM manages calendars through its own UI/API and refuses a CalDAV DELETE on the collection, so delete-calendar stays False. Prompt: please check if jeanes is DAViCal or synology. Probably it's needed to do some online research to find out what synlogoy caldav support really looks like. But it could be that synlogoy is using davical. Followup-Prompt: Fix the compatibility matrix. I've already [done some work on it] Followup-Prompt: Please tell me why synology has a separate dict instead of merging the davical dict with the deviances? [Claude first suggested to do a merge, but then found some weird reasons for not doing it] Followup-Prompt: make a merge and amend commit bc62968a19de2f32ebcbcbdf8fcf27c8a50c43b5 Followup-Prompt: scheduling.mailbox.inbox-delivery should not be set to False just because it was not probed. Unless we have good reasons for assuming it's not supported on Synology, let it inheritate the parent. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Claude wrote some novels in the commit messages describing what's new in this pull request. I decided to amend the commit messages and shorten it down a bit. The gist of it is preserved in the headlines:
(also - a code review found some issues with the "give me a test calendar"-logic that was piggy-backed into the first commit)