Conversation
* A trivial todo was fixed (I'm not so good at ReST and didn't have time checking up how to do internal links - now I asked Claude to fix it. Claude improved the language in the relevant sentence while being at it) * "3.0" was replaced with "3.x" consistently. 3.2 is out, but the doc remains relevant. * The section on deprecated things added "will be removed in 4.0" in the section header. That's or one thing incorrect (many things will be removed the earliest in 5.0) and or the other thing it doesn't need to be included in the header. prompt: The v3-migration page has a todo-comment - please fix followup=prompt: Check if there are other todo-markers under docs/source that can be dealt with easily followup-prompt: commit
…sync client When using niquests in async mode, digest auth must use AsyncHTTPDigestAuth. The sync HTTPDigestAuth.handle_401() calls r.connection.send() which returns a coroutine in async context, causing "AttributeError: 'coroutine' object has no attribute 'history'". Reported at jawah/niquests#387 (comment) prompt: Please check jawah/niquests#387 (comment), the last comment hints that the problem is in the caldav library AI Prompts: claude-sonnet-4-6: Please check jawah/niquests#387 (comment), the last comment hints that the problem is in the caldav library
tobixen
force-pushed
the
async-test-infrastructure
branch
from
2ca8eac to
5460fb2
Compare
Fixing: * Zimbra compatibility fix: omit DisplayName from MKCALENDAR body when create-calendar.set-displayname is unsupported (see below) * _async_put did not await the retry coroutine returned by _post_put. (Found when investigating Ox test breakages) * _async_get_object_by_uid was missing include_completed=True, which the sync get_object_by_uid always passes. (Found when investigating Ox test breakages) * _async_search_with_comptypes did not skip component types that the server does not support, unlike the sync _search_with_comptypes which already had this guard. (found when testing towards OX) Test code: * Add more fixtures and helpers for async tests * Add 7 async tests, mirroring the sync RepeatedFunctionalTestsBaseClass CRUD tests * Fixing up the tests to be more or less symmetric with the sync tests (and prettyfying the sync tests while being at it) * Rearranged the async testing fixtures to avoid Ox throwing errors when same event ended up in multiple calendars. prompt: An issue was created with `git bug bug new`, it has id 099db26 comment has id 009999d. Please start with Phase 1 and update the issue, but in a separate temporary worktree on a branch spun off from v3.2-preparations prompt: continue with phase 2 prompt: tests/test_async_integration.py::TestAsyncForXandikos::test_multi_get fails (...), fix multiget in collection.py to be async-aware, following the pattern in docs/design/ASYNC_DUAL_MODE.md followup-prompt: wait. calendar_multiget is deprecated and scheduled for deletion. Any test code exercising calendar_multiget can be removed or rewritten to use self.multiget instead. prompt: please help me investigate the tests/test_async_integration.py::TestAsyncForZimbra::test_lookup_event breakage. It's a known problem with zimbra that a GET towards an event URL frequently yields 404, but the .load()-method should have fallbacks to .multiget(), and the caldav-server-tester in ~/caldav-server-tester claims that the current versions of Zimbra have no problems with such GETs prompt: (lots of arguing with Claude) prompt: (trying a new claude session) With `pytest -k 'zimbra and lookup'` the sync test passes, while the async test fails. Why? There shouldn't be any difference there? (you've already investigated two hypotheses: "zimbra does not support quoted at-sign in the URL" and "the uid has been used on another calendar, therefore the GET returns 404", both hypotheses are wrong, and does not explain the asymmetry between sync and async) prompt: tests/test_async_integration.py::TestAsyncForOx::test_object_by_uid breaks followup-prompt: (when giving two options on how to fix the tests / test fixtures) Isn't it a third option here too - use a static UID, but different from well_known_1? I think we should make things as symmetric as possible and reuse the same calendar. It shouldn't be persistent, except for servers that doesn't support create/delete calendars. Mostly AI-generated, with some hands-on adjustments (AI-generating test code is fine, when done under good supervision - though I wonder a bit if I would have been able to do this faster without AI-help) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
New "features" added: * 'save-load.mutable' (replaces an old "compatibility flag", this has been observed unsupported for the old google API, long time ago) Compatibilities changed: * search.recurrences.expanded.exception was wrongly taggd as unsupported on many servers due to a bug in the checker script. Other: The old unique_calendar_ids "compatibility flag" seems not to be needed anymore by any servers I test, so it was removed (but possibly we will need it at some point in the future ...). AI-disclaimer: The unique_calendar_ids yanking job was hand-made, the other was AI-generated. The compatibility_hints.py is one of the places where AI-maintenance is considered OK. It's not considered to be a part of the sharp edge of the library, and some of the updates may be tedious. prompt: Create tests for a new feature checking if it's possible to overwrite a calendar event with new data. Should replace the old "overwrite" compatibility flag. (Perhaps this "incompatibility" was due to the caldav library not supporting etags and not updating from no sequence to SEQUENCE:1 ... i somehow doubt we'll find any servers not supporting this feature, but we should have the check anyway). load-save.mutable ? Update the compatibility_hints.py in /tmp/caldav-async-tests/ and update the tests there as well. prompt: On the main branch in the current directory, we get AssertionError: expectation is unsupported, observation is full for search.recurrences.expanded.exception from many servers now when running pytest -k compat in ~/caldav. Reproducable by doing cd ~/caldav ; status=good ; pytest tests/test_caldav.py::TestForServerRadicale::testCheckCompatibility || status=bad; cd ~/caldav-server-tester ; echo $status. Checking out tag v3.2.0, the test passes. There aren't that many differences between main and v3.2.0. Please investigate. followup-prompt: Throw it into the current async-test-infrastructure branch. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
lychee v0.24.1 was used on my laptop, and it has changed behavior: placeholder URLs like scheme://hostname:port now raise a hard parse error instead of being silently rewritten to file:// (which the old ignore pattern relied on). The 30-day cache had been masking this and other pre-existing failures. - Bump pre-commit lychee rev from v0.22.0 to v0.24.1 - Replace scheme://hostname:port placeholder URLs with concrete examples (http://proxy.example.com:8080) in both davclient.py and async_davclient.py - Update .lycheeignore: - Widen example.com pattern to cover ports - Add domain/*, credentialed localhost, oxhost2 Docker hostname - Add Google API, fsf.org, oxpedia.org, httpd.apache.org, sphinx URL template - Remove stale comment about file:// rewriting (gone in 0.24.1) AI-generated change (I'm not very clever at maintaining the CI infrastructure) prompt: `git push -f github` fails on the lychee hook in the pre-commit setup. How do I run the test? Why is it breaking just now, I can't see any recent changes causing this to break? followup-prompt: (responding to a hallucination that the problem was added in the async code monts ago) a55f9bb was added in january. Why does this break only now? I want symmetry, so the same fix should be applied to both files. I also don't understand why the existing line in .lycheeignore works for the sync davclient but not for async_davclient followup-prompt: Also update pre-commit config to use 0.24.1. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
tobixen
force-pushed
the
async-test-infrastructure
branch
from
5460fb2 to
f3f23f2
Compare
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.
There are still many more sync tests than async tests. We should try to bring some symmetry to it.
This will involve duplicated code as far as I see it - I don't care so much about it as it's tests. It may be a problem to keep things symmetric though - I need to think this through, perhaps we should introduce a "meta test" ensuring name matching between the sync and async tests.
revision 2ca8eac has some test breakages for ox and zimbra, I should look more into that before continuing the work here.
"Making async tests symmetric with sync tests" is tedious work involving quite a lot of code duplication, so in this pull request AI-generation will be heavily utilized.