Improve tests

- Parse and verify XML responses
- Extract methods for common requests

radicale/tests/__init__.py

@@ -25,10 +25,11 @@ import os
 import sys
 from io import BytesIO
 
+import defusedxml.ElementTree as DefusedET
 from pytest_cov import embed
 
 import radicale
-from radicale import server
+from radicale import server, xmlutils
 
 # Measure coverage of forked processes
 finish_request = server.ParallelHTTPServer.finish_request
@@ -76,3 +77,103 @@ class BaseTest:
 
         return (int(status.split()[0]), dict(headers),
                 answer[0].decode() if answer else None)
+
+    @staticmethod
+    def parse_responses(text):
+        xml = DefusedET.fromstring(text)
+        assert xml.tag == xmlutils.make_clark("D:multistatus")
+        path_responses = {}
+        for response in xml.findall(xmlutils.make_clark("D:response")):
+            href = response.find(xmlutils.make_clark("D:href"))
+            assert href.text not in path_responses
+            prop_respones = {}
+            for propstat in response.findall(
+                    xmlutils.make_clark("D:propstat")):
+                status = propstat.find(xmlutils.make_clark("D:status"))
+                assert status.text.startswith("HTTP/1.1 ")
+                status_code = int(status.text.split(" ")[1])
+                for prop in propstat.findall(xmlutils.make_clark("D:prop")):
+                    for element in prop:
+                        human_tag = xmlutils.make_human_tag(element.tag)
+                        assert human_tag not in prop_respones
+                        prop_respones[human_tag] = (status_code, element)
+            status = response.find(xmlutils.make_clark("D:status"))
+            if status is not None:
+                assert not prop_respones
+                assert status.text.startswith("HTTP/1.1 ")
+                status_code = int(status.text.split(" ")[1])
+                path_responses[href.text] = status_code
+            else:
+                path_responses[href.text] = prop_respones
+        return path_responses
+
+    @staticmethod
+    def _check_status(status, good_status, check=True):
+        if check is not False:
+            assert status in (good_status, check)
+        return status == good_status
+
+    def get(self, path, check=True, **args):
+        status, _, answer = self.request("GET", path, **args)
+        self._check_status(status, 200, check)
+        return status, answer
+
+    def put(self, path, data, check=True, **args):
+        status, _, answer = self.request("PUT", path, data, **args)
+        self._check_status(status, 201, check)
+        return status
+
+    def propfind(self, path, data=None, check=True, **args):
+        status, _, answer = self.request("PROPFIND", path, data, **args)
+        if not self._check_status(status, 207, check):
+            return status, None
+        responses = self.parse_responses(answer)
+        if args.get("HTTP_DEPTH", 0) == 0:
+            assert len(responses) == 1 and path in responses
+        return status, responses
+
+    def proppatch(self, path, data=None, check=True, **args):
+        status, _, answer = self.request("PROPPATCH", path, data, **args)
+        if not self._check_status(status, 207, check):
+            return status, None
+        responses = self.parse_responses(answer)
+        assert len(responses) == 1 and path in responses
+        return status, responses
+
+    def report(self, path, data, check=True, **args):
+        status, _, answer = self.request("REPORT", path, data, **args)
+        if not self._check_status(status, 207, check):
+            return status, None
+        return status, self.parse_responses(answer)
+
+    def delete(self, path, check=True, **args):
+        status, _, answer = self.request("DELETE", path, **args)
+        if not self._check_status(status, 200, check):
+            return status, None
+        responses = self.parse_responses(answer)
+        assert len(responses) == 1 and path in responses
+        return status, responses
+
+    def mkcalendar(self, path, data=None, check=True, **args):
+        status, _, _ = self.request("MKCALENDAR", path, data, **args)
+        self._check_status(status, 201, check)
+        return status
+
+    def mkcol(self, path, data=None, check=True, **args):
+        status, _, _ = self.request("MKCOL", path, data, **args)
+        self._check_status(status, 201, check)
+        return status
+
+    def create_addressbook(self, path, check=True, **args):
+        return self.mkcol(path, """\
+<?xml version="1.0" encoding="UTF-8" ?>
+<create xmlns="DAV:" xmlns:CR="urn:ietf:params:xml:ns:carddav">
+    <set>
+        <prop>
+            <resourcetype>
+                <collection />
+                <CR:addressbook />
+            </resourcetype>
+        </prop>
+    </set>
+</create>""", check=check, **args)

radicale/tests/test_auth.py

@@ -28,8 +28,8 @@ import tempfile
 
 import pytest
 
-from radicale import Application, config
+from radicale import Application, config, xmlutils
-from radicale.tests.test_base import BaseTest
+from radicale.tests import BaseTest
 
 
 class TestBaseAuthRequests(BaseTest):
@@ -84,11 +84,10 @@ class TestBaseAuthRequests(BaseTest):
                            ("😁", "🔑", False), ("😀", "", False),
                            ("", "🔑", False), ("", "", False))
         for user, password, valid in test_matrix:
-            status, _, _ = self.request(
+            status, _ = self.propfind(
-                "PROPFIND", "/",
+                "/", check=207 if valid else 401, HTTP_AUTHORIZATION=(
-                HTTP_AUTHORIZATION="Basic %s" % base64.b64encode(
+                    "Basic %s" % base64.b64encode(
-                    ("%s:%s" % (user, password)).encode()).decode())
+                        ("%s:%s" % (user, password)).encode()).decode()))
-            assert status == (207 if valid else 401)
 
     def test_htpasswd_plain(self):
         self._test_htpasswd("plain", "tmp:bepo")
@@ -136,38 +135,36 @@ class TestBaseAuthRequests(BaseTest):
     def test_remote_user(self):
         self.configuration.update({"auth": {"type": "remote_user"}}, "test")
         self.application = Application(self.configuration)
-        status, _, answer = self.request(
+        _, responses = self.propfind("/", """\
-            "PROPFIND", "/",
+<?xml version="1.0" encoding="utf-8"?>
-            """<?xml version="1.0" encoding="utf-8"?>
+<propfind xmlns="DAV:">
-               <propfind xmlns="DAV:">
+    <prop>
-                 <prop>
+        <current-user-principal />
-                   <current-user-principal />
+    </prop>
-                 </prop>
+</propfind>""", REMOTE_USER="test")
-               </propfind>""", REMOTE_USER="test")
+        status, prop = responses["/"]["D:current-user-principal"]
-        assert status == 207
+        assert status == 200
-        assert ">/test/<" in answer
+        assert prop.find(xmlutils.make_clark("D:href")).text == "/test/"
 
     def test_http_x_remote_user(self):
         self.configuration.update(
             {"auth": {"type": "http_x_remote_user"}}, "test")
         self.application = Application(self.configuration)
-        status, _, answer = self.request(
+        _, responses = self.propfind("/", """\
-            "PROPFIND", "/",
+<?xml version="1.0" encoding="utf-8"?>
-            """<?xml version="1.0" encoding="utf-8"?>
+<propfind xmlns="DAV:">
-               <propfind xmlns="DAV:">
+    <prop>
-                 <prop>
+        <current-user-principal />
-                   <current-user-principal />
+    </prop>
-                 </prop>
+</propfind>""", HTTP_X_REMOTE_USER="test")
-               </propfind>""", HTTP_X_REMOTE_USER="test")
+        status, prop = responses["/"]["D:current-user-principal"]
-        assert status == 207
+        assert status == 200
-        assert ">/test/<" in answer
+        assert prop.find(xmlutils.make_clark("D:href")).text == "/test/"
 
     def test_custom(self):
         """Custom authentication."""
         self.configuration.update(
             {"auth": {"type": "radicale.tests.custom.auth"}}, "test")
         self.application = Application(self.configuration)
-        status, _, _ = self.request(
+        self.propfind("/tmp/", HTTP_AUTHORIZATION="Basic %s" %
-            "PROPFIND", "/tmp", HTTP_AUTHORIZATION="Basic %s" %
+                      base64.b64encode(("tmp:").encode()).decode())
-            base64.b64encode(("tmp:").encode()).decode())
-        assert status == 207

radicale/tests/test_rights.py

@@ -24,8 +24,8 @@ import shutil
 import tempfile
 
 from radicale import Application, config
+from radicale.tests import BaseTest
 from radicale.tests.helpers import get_file_content
-from radicale.tests.test_base import BaseTest
 
 
 class TestBaseRightsRequests(BaseTest):
@@ -56,69 +56,67 @@ class TestBaseRightsRequests(BaseTest):
                      "htpasswd_encryption": "plain"}}, "test")
         self.application = Application(self.configuration)
         for u in ("tmp", "other"):
-            status, _, _ = self.request(
+            status, _ = self.propfind(
-                "PROPFIND", "/%s" % u, HTTP_AUTHORIZATION="Basic %s" %
+                "/%s/" % u, HTTP_AUTHORIZATION="Basic %s" %
                 base64.b64encode(("%s:bepo" % u).encode()).decode())
-            assert status == 207
+        status, _ = (self.propfind if mode == "r" else self.proppatch)(
-        status, _, _ = self.request(
+            path, check=False, HTTP_AUTHORIZATION="Basic %s" %
-            "PROPFIND" if mode == "r" else "PROPPATCH", path,
+            base64.b64encode(("tmp:bepo").encode()).decode() if user else "")
-            HTTP_AUTHORIZATION="Basic %s" % base64.b64encode(
-                ("tmp:bepo").encode()).decode() if user else "")
         assert status == expected_status
 
     def test_owner_only(self):
         self._test_rights("owner_only", "", "/", "r", 401)
         self._test_rights("owner_only", "", "/", "w", 401)
-        self._test_rights("owner_only", "", "/tmp", "r", 401)
+        self._test_rights("owner_only", "", "/tmp/", "r", 401)
-        self._test_rights("owner_only", "", "/tmp", "w", 401)
+        self._test_rights("owner_only", "", "/tmp/", "w", 401)
         self._test_rights("owner_only", "tmp", "/", "r", 207)
         self._test_rights("owner_only", "tmp", "/", "w", 403)
-        self._test_rights("owner_only", "tmp", "/tmp", "r", 207)
+        self._test_rights("owner_only", "tmp", "/tmp/", "r", 207)
-        self._test_rights("owner_only", "tmp", "/tmp", "w", 207)
+        self._test_rights("owner_only", "tmp", "/tmp/", "w", 207)
-        self._test_rights("owner_only", "tmp", "/other", "r", 403)
+        self._test_rights("owner_only", "tmp", "/other/", "r", 403)
-        self._test_rights("owner_only", "tmp", "/other", "w", 403)
+        self._test_rights("owner_only", "tmp", "/other/", "w", 403)
 
     def test_owner_only_without_auth(self):
         self._test_rights("owner_only", "", "/", "r", 207, False)
         self._test_rights("owner_only", "", "/", "w", 401, False)
-        self._test_rights("owner_only", "", "/tmp", "r", 207, False)
+        self._test_rights("owner_only", "", "/tmp/", "r", 207, False)
-        self._test_rights("owner_only", "", "/tmp", "w", 207, False)
+        self._test_rights("owner_only", "", "/tmp/", "w", 207, False)
 
     def test_owner_write(self):
         self._test_rights("owner_write", "", "/", "r", 401)
         self._test_rights("owner_write", "", "/", "w", 401)
-        self._test_rights("owner_write", "", "/tmp", "r", 401)
+        self._test_rights("owner_write", "", "/tmp/", "r", 401)
-        self._test_rights("owner_write", "", "/tmp", "w", 401)
+        self._test_rights("owner_write", "", "/tmp/", "w", 401)
         self._test_rights("owner_write", "tmp", "/", "r", 207)
         self._test_rights("owner_write", "tmp", "/", "w", 403)
-        self._test_rights("owner_write", "tmp", "/tmp", "r", 207)
+        self._test_rights("owner_write", "tmp", "/tmp/", "r", 207)
-        self._test_rights("owner_write", "tmp", "/tmp", "w", 207)
+        self._test_rights("owner_write", "tmp", "/tmp/", "w", 207)
-        self._test_rights("owner_write", "tmp", "/other", "r", 207)
+        self._test_rights("owner_write", "tmp", "/other/", "r", 207)
-        self._test_rights("owner_write", "tmp", "/other", "w", 403)
+        self._test_rights("owner_write", "tmp", "/other/", "w", 403)
 
     def test_owner_write_without_auth(self):
         self._test_rights("owner_write", "", "/", "r", 207, False)
         self._test_rights("owner_write", "", "/", "w", 401, False)
-        self._test_rights("owner_write", "", "/tmp", "r", 207, False)
+        self._test_rights("owner_write", "", "/tmp/", "r", 207, False)
-        self._test_rights("owner_write", "", "/tmp", "w", 207, False)
+        self._test_rights("owner_write", "", "/tmp/", "w", 207, False)
 
     def test_authenticated(self):
         self._test_rights("authenticated", "", "/", "r", 401)
         self._test_rights("authenticated", "", "/", "w", 401)
-        self._test_rights("authenticated", "", "/tmp", "r", 401)
+        self._test_rights("authenticated", "", "/tmp/", "r", 401)
-        self._test_rights("authenticated", "", "/tmp", "w", 401)
+        self._test_rights("authenticated", "", "/tmp/", "w", 401)
         self._test_rights("authenticated", "tmp", "/", "r", 207)
         self._test_rights("authenticated", "tmp", "/", "w", 207)
-        self._test_rights("authenticated", "tmp", "/tmp", "r", 207)
+        self._test_rights("authenticated", "tmp", "/tmp/", "r", 207)
-        self._test_rights("authenticated", "tmp", "/tmp", "w", 207)
+        self._test_rights("authenticated", "tmp", "/tmp/", "w", 207)
-        self._test_rights("authenticated", "tmp", "/other", "r", 207)
+        self._test_rights("authenticated", "tmp", "/other/", "r", 207)
-        self._test_rights("authenticated", "tmp", "/other", "w", 207)
+        self._test_rights("authenticated", "tmp", "/other/", "w", 207)
 
     def test_authenticated_without_auth(self):
         self._test_rights("authenticated", "", "/", "r", 207, False)
         self._test_rights("authenticated", "", "/", "w", 207, False)
-        self._test_rights("authenticated", "", "/tmp", "r", 207, False)
+        self._test_rights("authenticated", "", "/tmp/", "r", 207, False)
-        self._test_rights("authenticated", "", "/tmp", "w", 207, False)
+        self._test_rights("authenticated", "", "/tmp/", "w", 207, False)
 
     def test_from_file(self):
         rights_file_path = os.path.join(self.colpath, "rights")
@@ -134,8 +132,8 @@ collection: custom(/.*)?
 permissions: Rr""")
         self.configuration.update(
             {"rights": {"file": rights_file_path}}, "test")
-        self._test_rights("from_file", "", "/other", "r", 401)
+        self._test_rights("from_file", "", "/other/", "r", 401)
-        self._test_rights("from_file", "tmp", "/other", "r", 403)
+        self._test_rights("from_file", "tmp", "/other/", "r", 403)
         self._test_rights("from_file", "", "/custom/sub", "r", 404)
         self._test_rights("from_file", "tmp", "/custom/sub", "r", 404)
         self._test_rights("from_file", "", "/custom/sub", "w", 401)
@@ -144,7 +142,8 @@ permissions: Rr""")
     def test_custom(self):
         """Custom rights management."""
         self._test_rights("radicale.tests.custom.rights", "", "/", "r", 401)
-        self._test_rights("radicale.tests.custom.rights", "", "/tmp", "r", 207)
+        self._test_rights(
+            "radicale.tests.custom.rights", "", "/tmp/", "r", 207)
 
     def test_collections_and_items(self):
         """Test rights for creation of collections, calendars and items.
@@ -155,33 +154,19 @@ permissions: Rr""")
 
         """
         self.application = Application(self.configuration)
-        status, _, _ = self.request("MKCALENDAR", "/")
+        self.mkcalendar("/", check=401)
-        assert status == 401
+        self.mkcalendar("/user/", check=401)
-        status, _, _ = self.request("MKCALENDAR", "/user/")
+        self.mkcol("/user/")
-        assert status == 401
+        self.mkcol("/user/calendar/", check=401)
-        status, _, _ = self.request("MKCOL", "/user/")
+        self.mkcalendar("/user/calendar/")
-        assert status == 201
+        self.mkcol("/user/calendar/item", check=401)
-        status, _, _ = self.request("MKCOL", "/user/calendar/")
+        self.mkcalendar("/user/calendar/item", check=401)
-        assert status == 401
-        status, _, _ = self.request("MKCALENDAR", "/user/calendar/")
-        assert status == 201
-        status, _, _ = self.request("MKCOL", "/user/calendar/item")
-        assert status == 401
-        status, _, _ = self.request("MKCALENDAR", "/user/calendar/item")
-        assert status == 401
 
     def test_put_collections_and_items(self):
         """Test rights for creation of calendars and items with PUT."""
         self.application = Application(self.configuration)
-        status, _, _ = self.request(
+        self.put("/user/", "BEGIN:VCALENDAR\r\nEND:VCALENDAR", check=401)
-            "PUT", "/user/", "BEGIN:VCALENDAR\r\nEND:VCALENDAR")
+        self.mkcol("/user/")
-        assert status == 401
+        self.put("/user/calendar/", "BEGIN:VCALENDAR\r\nEND:VCALENDAR")
-        status, _, _ = self.request("MKCOL", "/user/")
-        assert status == 201
-        status, _, _ = self.request(
-            "PUT", "/user/calendar/", "BEGIN:VCALENDAR\r\nEND:VCALENDAR")
-        assert status == 201
         event1 = get_file_content("event1.ics")
-        status, _, _ = self.request(
+        self.put("/user/calendar/event1.ics", event1)
-            "PUT", "/user/calendar/event1.ics", event1)
-        assert status == 201

radicale/tests/test_server.py

@@ -35,6 +35,7 @@ from urllib.error import HTTPError, URLError
 import pytest
 
 from radicale import config, server
+from radicale.tests import BaseTest
 from radicale.tests.helpers import configuration_to_dict, get_file_path
 
 try:
@@ -50,7 +51,7 @@ class DisabledRedirectHandler(request.HTTPRedirectHandler):
     http_error_301 = http_error_303 = http_error_307 = http_error_302
 
 
-class TestBaseServerRequests:
+class TestBaseServerRequests(BaseTest):
     """Test the internal server."""
 
     def setup(self):
@@ -108,8 +109,7 @@ class TestBaseServerRequests:
 
     def test_root(self):
         self.thread.start()
-        status, _, _ = self.request("GET", "/")
+        self.get("/", check=302)
-        assert status == 302
 
     def test_ssl(self):
         self.configuration.update({
@@ -117,8 +117,7 @@ class TestBaseServerRequests:
                        "certificate": get_file_path("cert.pem"),
                        "key": get_file_path("key.pem")}}, "test")
         self.thread.start()
-        status, _, _ = self.request("GET", "/")
+        self.get("/", check=302)
-        assert status == 302
 
     @pytest.mark.skipif(not server.HAS_IPV6, reason="IPv6 not supported")
     def test_ipv6(self):
@@ -132,16 +131,8 @@ class TestBaseServerRequests:
             self.sockname = sock.getsockname()[:2]
         self.configuration.update({
             "server": {"hosts": "[%s]:%d" % self.sockname}}, "test")
-        original_eai_addrfamily = server.EAI_ADDRFAMILY
+        self.thread.start()
-        if os.name == "nt" and server.EAI_ADDRFAMILY is None:
+        self.get("/", check=302)
-            # HACK: incomplete errno conversion in WINE
-            server.EAI_ADDRFAMILY = -9
-        try:
-            self.thread.start()
-            status, _, _ = self.request("GET", "/")
-        finally:
-            server.EAI_ADDRFAMILY = original_eai_addrfamily
-        assert status == 302
 
     def test_command_line_interface(self):
         config_args = []
@@ -165,9 +156,7 @@ class TestBaseServerRequests:
         p = subprocess.Popen(
             [sys.executable, "-m", "radicale"] + config_args, env=env)
         try:
-            status, _, _ = self.request(
+            self.get("/", is_alive_fn=lambda: p.poll() is None, check=302)
-                "GET", "/", is_alive_fn=lambda: p.poll() is None)
-            assert status == 302
         finally:
             p.terminate()
             p.wait()
@@ -189,9 +178,7 @@ class TestBaseServerRequests:
             "--bind", self.configuration.get_raw("server", "hosts"),
             "--env", "RADICALE_CONFIG=%s" % config_path, "radicale"], env=env)
         try:
-            status, _, _ = self.request(
+            self.get("/", is_alive_fn=lambda: p.poll() is None, check=302)
-                "GET", "/", is_alive_fn=lambda: p.poll() is None)
-            assert status == 302
         finally:
             p.terminate()
             p.wait()

radicale/tests/test_web.py

@@ -23,7 +23,7 @@ import shutil
 import tempfile
 
 from radicale import Application, config
-from radicale.tests.test_base import BaseTest
+from radicale.tests import BaseTest
 
 
 class TestBaseWebRequests(BaseTest):
@@ -45,24 +45,20 @@ class TestBaseWebRequests(BaseTest):
         status, headers, _ = self.request("GET", "/.web")
         assert status == 302
         assert headers.get("Location") == ".web/"
-        status, _, answer = self.request("GET", "/.web/")
+        _, answer = self.get("/.web/")
-        assert status == 200
         assert answer
 
     def test_none(self):
         self.configuration.update({"web": {"type": "none"}}, "test")
         self.application = Application(self.configuration)
-        status, _, answer = self.request("GET", "/.web")
+        _, answer = self.get("/.web")
-        assert status == 200
         assert answer
-        status, _, answer = self.request("GET", "/.web/")
+        self.get("/.web/", check=404)
-        assert status == 404
 
     def test_custom(self):
         """Custom web plugin."""
         self.configuration.update({
             "web": {"type": "radicale.tests.custom.web"}}, "test")
         self.application = Application(self.configuration)
-        status, _, answer = self.request("GET", "/.web")
+        _, answer = self.get("/.web")
-        assert status == 200
         assert answer == "custom"