search

cyrusimap / cassandane

Other
6 stars 11 forks source link

Add support for testing JMAP in a murder setup #151

Closed elliefm closed 2 years ago

elliefm commented 2 years ago

This is for https://github.com/cyrusimap/cyrus-imapd/pull/3564

We have a small handful of very basic tests for murder setups (Murder.pm), but the way we set things up in Cassandane meant these could only use IMAP.

This PR adjusts the infrastructure so that a test can either require an IMAP murder or a JMAP murder (but not both!), modifies the existing Murder.pm tests to use the new infrastructure, and adds a couple of basic JMAP tests too.

We can and should add more tests to this, but so far this proves that the basic premise -- connecting to the frontend, making a JMAP request, and the request being proxied to the backend containing the user's account -- seems to work.

There's probably more to do with the infrastructure yet, but I won't be sure what it needs to look like until we write and debug more tests.

elliefm commented 2 years ago

realised in the shower that I also want some kinda test_jmap_backend_commands test that verifies that connections directly to the backend don't get proxied, so I'll add that on friday

elliefm commented 2 years ago

It occurs to me that this is not really "JMAP" murder so much as it is "HTTP" murder, and we could use the same mechanism to test DAV-in-murder functionality (if that's a thing; I assume it is but don't really know either way).

So I'm tempted to rename jmapmurder (et al) to httpmurder (et al) now.

If I do that, I probably will split the Murder.pm test suite into separate IMAPMurder.pm and JMAPMurder.pm, allowing for the possibility of additional *DavMurder.pm suites later. Because I think having all those protocols tested in one suite will get real cluttered and hard to maintain.

ksmurchison commented 2 years ago

Yes. HTTP Murder makes more sense

elliefm commented 2 years ago

Mostly refactored, but I need to set up caldavtester and make sure I haven't broken it, which I might have! Next week...

elliefm commented 2 years ago

Looks like I haven't broken caldavtester, whew

elliefm commented 2 years ago

Wait, it was TesterCardDAV that was having problems, not TesterCalDAV. Checking again...

elliefm commented 2 years ago

Turns out TesterCardDAV was broken by only enabling "carddav" httpmodule, when it appears that module is dependent on "caldav" too. Have opened https://github.com/cyrusimap/cyrus-imapd/pull/3649 which updates the documentation to reflect this.

elliefm commented 2 years ago

@ksmurchison I've now refactored the murder setup stuff like we talked about. Can you review again please?

elliefm commented 2 years ago

Oh those are great test ideas, thanks. I've put them into #160 and I'll tackle them as a separate PR.

This one can be merged whenever we're about to merge https://github.com/cyrusimap/cyrus-imapd/pull/3564