Merge pull request #895 from pimutils/optimise-test-servers

Run CI storage tests against Baikal again

.builds/archlinux.yaml

@@ -1,3 +1,5 @@
+# Run tests using the packaged dependencies on ArchLinux.
+
 image: archlinux
 packages:
   - docker

.builds/tests-minimal.yaml

@@ -1,4 +1,8 @@
-# TODO: Maybe shift this job to ubuntu/debian.
+# Run tests using oldest available dependency versions.
+#
+# TODO: It might make more sense to test with an older Ubuntu or Fedora version
+# here, and consider that our "oldest suppported environment".
+
 image: archlinux
 packages:
   - docker

.builds/tests-release.yaml

@@ -1,3 +1,5 @@
+# Run tests using latest dependencies from PyPI
+
 image: archlinux
 packages:
   - docker
@@ -15,13 +17,14 @@ tasks:
   - setup: |
       cd vdirsyncer
       sudo systemctl start docker
-      DAV_SERVER="radicale xandikos" make -e install-test
+      DAV_SERVER="radicale xandikos baikal" make -e install-test
       make -e install-style
   - test: |
       cd vdirsyncer
       # Non-system python is used for packages:
       export PATH=$PATH:~/.local/bin/
       make -e ci-test
+      DAV_SERVER=baikal make -e ci-test-storage
       DAV_SERVER=radicale make -e ci-test-storage
       DAV_SERVER=xandikos make -e ci-test-storage
   - style: |

.pre-commit-config.yaml

@@ -1,6 +1,6 @@
 repos:
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v3.4.0
+    rev: v4.0.1
     hooks:
       - id: trailing-whitespace
         args: [--markdown-linebreak-ext=md]
@@ -9,12 +9,12 @@ repos:
       - id: check-added-large-files
       - id: debug-statements
   - repo: https://gitlab.com/pycqa/flake8
-    rev: "3.9.1"
+    rev: "3.9.2"
     hooks:
       - id: flake8
         additional_dependencies: [flake8-import-order, flake8-bugbear]
   - repo: https://github.com/psf/black
-    rev: "21.5b0"
+    rev: "21.6b0"
     hooks:
       - id: black
   - repo: https://github.com/asottile/reorder_python_imports

CHANGELOG.rst

@@ -18,7 +18,7 @@ Note: Version 0.17 has some alpha releases but ultimately was never finalised.
 - Support for Python 3.5 and 3.6 has been dropped. This release mostly focuses
   on keeping vdirsyncer compatible with newer environments.
 - click 8 and click-threading 0.5.0 are now required.
-- For those using ``pipx``, we now recommend using ``pipx``, it's successor.
+- For those using ``pipsi``, we now recommend using ``pipx``, it's successor.
 - Python 3.9 is now supported.
 - Our Debian/Ubuntu build scripts have been updated. New versions should be
   pushed to those repositories soon.

Makefile

@@ -40,7 +40,6 @@ PYTEST = py.test $(PYTEST_ARGS)
 export TESTSERVER_BASE := ./tests/storage/servers/
 CODECOV_PATH = /tmp/codecov.sh
 
-ifeq ($(CI), true)
 ci-test:
 	curl -s https://codecov.io/bash > $(CODECOV_PATH)
 	$(PYTEST) tests/unit/
@@ -48,14 +47,14 @@ ci-test:
 	$(PYTEST) tests/system/
 	bash $(CODECOV_PATH) -c -F system
 	[ "$(ETESYNC_TESTS)" = "false" ] || make test-storage
+
 ci-test-storage:
 	curl -s https://codecov.io/bash > $(CODECOV_PATH)
 	$(PYTEST) tests/storage/
 	bash $(CODECOV_PATH) -c -F storage
-else
+
 test:
 	$(PYTEST)
-endif
 
 all:
 	$(error Take a look at https://vdirsyncer.pimutils.org/en/stable/tutorial.html#installation)
@@ -63,7 +62,9 @@ all:
 install-servers:
 	set -ex; \
 	for server in $(DAV_SERVER); do \
-		(cd $(TESTSERVER_BASE)$$server && sh install.sh); \
+		if [ -f $(TESTSERVER_BASE)$$server/install.sh ]; then \
+			(cd $(TESTSERVER_BASE)$$server && sh install.sh); \
+		fi \
 	done
 
 install-test: install-servers install-dev

docker/baikal/Dockerfile

@@ -1,26 +0,0 @@
-# Based on https://github.com/ckulka/baikal-docker
-# Sadly, we can't override the VOLUME it has set, and we want some static
-# config.
-FROM php:7.4-apache
-ENV VERSION 0.7.0
-
-ADD https://github.com/sabre-io/Baikal/releases/download/$VERSION/baikal-$VERSION.zip .
-RUN apt-get update && apt-get install -y sqlite3 unzip
-RUN unzip -q baikal-$VERSION.zip -d /var/www/
-
-RUN chown -R www-data:www-data /var/www/baikal && \
-  docker-php-ext-install pdo pdo_mysql
-
-COPY apache.conf /etc/apache2/sites-enabled/000-default.conf
-COPY start.sh /opt/
-RUN a2enmod rewrite
-
-COPY baikal.yaml /var/www/baikal/config/baikal.yaml
-COPY configure.sql /configure.sql
-
-RUN touch /var/www/baikal/Specific/INSTALL_DISABLED
-RUN cat /configure.sql | sqlite3 /var/www/baikal/Specific/db/db.sqlite
-
-RUN chmod -R 777 /var/www/baikal/Specific/ /var/www/baikal/config/
-
-CMD [ "sh", "/opt/start.sh" ]

docker/baikal/apache.conf

@@ -1,25 +0,0 @@
-# Shameless copied from https://github.com/ckulka/baikal-docker/blob/master/files/apache.conf
-
-<VirtualHost *:80>
-
-	# InjectedServerAlias dav.example.org dav.example.io
-	DocumentRoot /var/www/baikal/html
-
-	RewriteEngine On
-	RewriteRule /.well-known/carddav /dav.php [R,L]
-	RewriteRule /.well-known/caldav /dav.php [R,L]
-
-	<Directory "/var/www/baikal/html">
-		Options None
-		Options +FollowSymlinks
-		AllowOverride All
-
-		# Confiugration for apache-2.2:
-		Order allow,deny
-		Allow from all
-
-		# Confiugration for apache-2.4:
-		Require all granted
-	</Directory>
-
-</VirtualHost>

docker/baikal/baikal.yaml

@@ -1,18 +0,0 @@
-system:
-    configured_version: 0.7.0
-    timezone: Europe/Paris
-    card_enabled: true
-    cal_enabled: true
-    dav_auth_type: Basic
-    admin_passwordhash: 6a890c3aa185845a4bee1e1caed92e1faaf2dec6772291dca301cef6782e3bce
-    auth_realm: BaikalDAV
-    invite_from: noreply@localhost
-database:
-    sqlite_file: /var/www/baikal/Specific/db/db.sqlite
-    mysql: false
-    mysql_host: ''
-    mysql_dbname: ''
-    mysql_username: ''
-    mysql_password: ''
-    encryption_key: bdf3bec969736e122e6d5f72c282c49e
-    configured_version: ''

docker/baikal/configure.sql

@@ -1,139 +0,0 @@
-PRAGMA foreign_keys=OFF;
-BEGIN TRANSACTION;
-CREATE TABLE addressbooks (
-    id integer primary key asc NOT NULL,
-    principaluri text NOT NULL,
-    displayname text,
-    uri text NOT NULL,
-    description text,
-    synctoken integer DEFAULT 1 NOT NULL
-);
-INSERT INTO addressbooks VALUES(1,'principals/baikal','Default Address Book','default','Default Address Book for Baikal',1);
-CREATE TABLE cards (
-    id integer primary key asc NOT NULL,
-    addressbookid integer NOT NULL,
-    carddata blob,
-    uri text NOT NULL,
-    lastmodified integer,
-    etag text,
-    size integer
-);
-CREATE TABLE addressbookchanges (
-    id integer primary key asc NOT NULL,
-    uri text,
-    synctoken integer NOT NULL,
-    addressbookid integer NOT NULL,
-    operation integer NOT NULL
-);
-CREATE TABLE calendarobjects (
-    id integer primary key asc NOT NULL,
-    calendardata blob NOT NULL,
-    uri text NOT NULL,
-    calendarid integer NOT NULL,
-    lastmodified integer NOT NULL,
-    etag text NOT NULL,
-    size integer NOT NULL,
-    componenttype text,
-    firstoccurence integer,
-    lastoccurence integer,
-    uid text
-);
-CREATE TABLE calendars (
-    id integer primary key asc NOT NULL,
-    synctoken integer DEFAULT 1 NOT NULL,
-    components text NOT NULL
-);
-INSERT INTO calendars VALUES(1,1,'VEVENT,VTODO');
-CREATE TABLE calendarinstances (
-    id integer primary key asc NOT NULL,
-    calendarid integer,
-    principaluri text,
-    access integer,
-    displayname text,
-    uri text NOT NULL,
-    description text,
-    calendarorder integer,
-    calendarcolor text,
-    timezone text,
-    transparent bool,
-    share_href text,
-    share_displayname text,
-    share_invitestatus integer DEFAULT '2',
-    UNIQUE (principaluri, uri),
-    UNIQUE (calendarid, principaluri),
-    UNIQUE (calendarid, share_href)
-);
-INSERT INTO calendarinstances VALUES(1,1,'principals/baikal',NULL,'Default calendar','default','Default calendar',0,'','Europe/Paris',NULL,NULL,NULL,2);
-CREATE TABLE calendarchanges (
-    id integer primary key asc NOT NULL,
-    uri text,
-    synctoken integer NOT NULL,
-    calendarid integer NOT NULL,
-    operation integer NOT NULL
-);
-CREATE TABLE calendarsubscriptions (
-    id integer primary key asc NOT NULL,
-    uri text NOT NULL,
-    principaluri text NOT NULL,
-    source text NOT NULL,
-    displayname text,
-    refreshrate text,
-    calendarorder integer,
-    calendarcolor text,
-    striptodos bool,
-    stripalarms bool,
-    stripattachments bool,
-    lastmodified int
-);
-CREATE TABLE schedulingobjects (
-    id integer primary key asc NOT NULL,
-    principaluri text NOT NULL,
-    calendardata blob,
-    uri text NOT NULL,
-    lastmodified integer,
-    etag text NOT NULL,
-    size integer NOT NULL
-);
-CREATE TABLE locks (
-	id integer primary key asc NOT NULL,
-	owner text,
-	timeout integer,
-	created integer,
-	token text,
-	scope integer,
-	depth integer,
-	uri text
-);
-CREATE TABLE principals (
-    id INTEGER PRIMARY KEY ASC NOT NULL,
-    uri TEXT NOT NULL,
-    email TEXT,
-    displayname TEXT,
-    UNIQUE(uri)
-);
-INSERT INTO principals VALUES(1,'principals/baikal','baikal@example.com','Baikal');
-CREATE TABLE groupmembers (
-    id INTEGER PRIMARY KEY ASC NOT NULL,
-    principal_id INTEGER NOT NULL,
-    member_id INTEGER NOT NULL,
-    UNIQUE(principal_id, member_id)
-);
-CREATE TABLE propertystorage (
-    id integer primary key asc NOT NULL,
-    path text NOT NULL,
-    name text NOT NULL,
-    valuetype integer NOT NULL,
-    value string
-);
-CREATE TABLE users (
-	id integer primary key asc NOT NULL,
-	username TEXT NOT NULL,
-	digesta1 TEXT NOT NULL,
-	UNIQUE(username)
-);
-INSERT INTO users VALUES(1,'baikal','3b0845b235b7e985ce5905ab8df45e1a');
-CREATE INDEX addressbookid_synctoken ON addressbookchanges (addressbookid, synctoken);
-CREATE INDEX calendarid_synctoken ON calendarchanges (calendarid, synctoken);
-CREATE INDEX principaluri_uri ON calendarsubscriptions (principaluri, uri);
-CREATE UNIQUE INDEX path_property ON propertystorage (path, name);
-COMMIT;

docker/baikal/start.sh

@@ -1,16 +0,0 @@
-#!/bin/sh
-# Shameless copied from https://raw.githubusercontent.com/ckulka/baikal-docker/master/files/start.sh
-
-# Inject ServerName and ServerAlias if specified
-APACHE_CONFIG="/etc/apache2/sites-available/000-default.conf"
-if [ ! -z ${BAIKAL_SERVERNAME+x} ]
-then
-	sed -i "s/# InjectedServerName .*/ServerName $BAIKAL_SERVERNAME/g" $APACHE_CONFIG
-fi
-
-if [ ! -z ${BAIKAL_SERVERALIAS+x} ]
-then
-	sed -i "s/# InjectedServerAlias .*/ServerAlias $BAIKAL_SERVERALIAS/g" $APACHE_CONFIG
-fi
-
-apache2-foreground

docs/contributing.rst

@@ -102,8 +102,7 @@ initialized and documented.
 
 For example, to test xandikos, first run the server itself::
 
-    docker build -t xandikos docker/xandikos
+    docker run -p 8000:8000 whynothugo/vdirsyncer-devkit-xandikos
-    docker start -p 8000:8000 xandikos
 
 Then run the tests specifying this ``DAV_SERVER``, run::
 

setup.cfg

@@ -2,7 +2,6 @@
 universal = 1
 
 [tool:pytest]
-norecursedirs = tests/storage/servers/*
 addopts =
   --tb=short
   --cov-config .coveragerc
@@ -11,12 +10,12 @@ addopts =
   --no-cov-on-fail
 
 [flake8]
-# E731: Use a def instead of lambda expr
+application-import-names = tests,vdirsyncer
-# E743: Ambiguous function definition
+extend-ignore =
-ignore = E731, E743
+  E203,  # Black-incompatible colon spacing.
-# E503: Line break occurred before a binary operator
+  W503,  # Line jump before binary operator.
-extend-ignore = E203, W503
+  I100,
+  I202
 max-line-length = 88
-select = C,E,F,W,B,B9
+exclude = .eggs,build
-exclude = .eggs, tests/storage/servers/owncloud/, tests/storage/servers/nextcloud/, tests/storage/servers/baikal/, build/
+import-order-style = smarkets
-application-package-names = tests,vdirsyncer

tests/storage/__init__.py

@@ -210,7 +210,7 @@ class StorageTests:
         s = self.storage_class(**self.storage_class.create_collection(**args))
 
         href = s.upload(get_item())[0]
-        assert href in {href for href, etag in s.list()}
+        assert href in (href for href, etag in s.list())
 
     def test_discover_collection_arg(self, requires_collections, get_storage_args):
         args = get_storage_args(collection="test2")
@@ -239,7 +239,7 @@ class StorageTests:
         uid = str(uuid.uuid4())
         s.upload(get_item(uid=uid.upper()))
         s.upload(get_item(uid=uid.lower()))
-        items = list(href for href, etag in s.list())
+        items = [href for href, etag in s.list()]
         assert len(items) == 2
         assert len(set(items)) == 2
 

tests/storage/dav/test_caldav.py

@@ -21,6 +21,7 @@ class TestCalDAVStorage(DAVStorageTests):
     def item_type(self, request):
         return request.param
 
+    @pytest.mark.xfail(dav_server == "baikal", reason="Baikal returns 500.")
     def test_doesnt_accept_vcard(self, item_type, get_storage_args):
         s = self.storage_class(item_types=(item_type,), **get_storage_args())
 
@@ -42,6 +43,7 @@ class TestCalDAVStorage(DAVStorageTests):
             ((), 1),
         ],
     )
+    @pytest.mark.xfail(dav_server == "baikal", reason="Baikal returns 500.")
     def test_item_types_performance(
         self, get_storage_args, arg, calls_num, monkeypatch
     ):
@@ -152,6 +154,7 @@ class TestCalDAVStorage(DAVStorageTests):
     @pytest.mark.skipif(
         dav_server == "fastmail", reason="Fastmail has non-standard hadling of VTODOs."
     )
+    @pytest.mark.xfail(dav_server == "baikal", reason="Baikal returns 500.")
     def test_item_types_general(self, s):
         event = s.upload(format_item(EVENT_TEMPLATE))[0]
         task = s.upload(format_item(TASK_TEMPLATE))[0]

tests/storage/servers/baikal/install.sh

@@ -1,6 +1,3 @@
 #!/bin/sh
 
-cd $(git rev-parse --show-toplevel)
+docker run -d -p 8002:80 whynothugo/vdirsyncer-devkit-baikal
-
-docker build -t baikal docker/baikal
-docker run -d -p 8002:80 baikal

tests/storage/servers/mysteryshack/.gitignore

@@ -1 +0,0 @@
-mysteryshack

tests/storage/servers/mysteryshack/__init__.py

@@ -1,79 +0,0 @@
-import os
-import shutil
-import subprocess
-import time
-
-import pytest
-import requests
-
-testserver_repo = os.path.dirname(__file__)
-make_sh = os.path.abspath(os.path.join(testserver_repo, "make.sh"))
-
-
-def wait():
-    for i in range(100):
-        try:
-            requests.get("http://127.0.0.1:6767/", verify=False)
-        except Exception as e:
-            # Don't know exact exception class, don't care.
-            # Also, https://github.com/kennethreitz/requests/issues/2192
-            if "connection refused" not in str(e).lower():
-                raise
-            time.sleep(2 ** i)
-        else:
-            return True
-    return False
-
-
-class ServerMixin:
-    @pytest.fixture(scope="session")
-    def setup_mysteryshack_server(self, xprocess):
-        def preparefunc(cwd):
-            return wait, ["sh", make_sh, "testserver"]
-
-        subprocess.check_call(["sh", make_sh, "testserver-config"])
-        xprocess.ensure("mysteryshack_server", preparefunc)
-
-        return (
-            subprocess.check_output(
-                [
-                    os.path.join(
-                        testserver_repo, "mysteryshack/target/debug/mysteryshack"
-                    ),
-                    "-c",
-                    "/tmp/mysteryshack/config",
-                    "user",
-                    "authorize",
-                    "testuser",
-                    "https://example.com",
-                    self.storage_class.scope + ":rw",
-                ]
-            )
-            .strip()
-            .decode()
-        )
-
-    @pytest.fixture
-    def get_storage_args(self, monkeypatch, setup_mysteryshack_server):
-        from requests import Session
-
-        monkeypatch.setitem(os.environ, "OAUTHLIB_INSECURE_TRANSPORT", "true")
-
-        old_request = Session.request
-
-        def request(self, method, url, **kw):
-            url = url.replace("https://", "http://")
-            return old_request(self, method, url, **kw)
-
-        monkeypatch.setattr(Session, "request", request)
-        shutil.rmtree("/tmp/mysteryshack/testuser/data", ignore_errors=True)
-        shutil.rmtree("/tmp/mysteryshack/testuser/meta", ignore_errors=True)
-
-        def inner(**kw):
-            kw["account"] = "testuser@127.0.0.1:6767"
-            kw["access_token"] = setup_mysteryshack_server
-            if self.storage_class.fileext == ".ics":
-                kw.setdefault("collection", "test")
-            return kw
-
-        return inner

tests/storage/servers/mysteryshack/install.sh

@@ -1,18 +0,0 @@
-#!/bin/sh
-set -ex
-cd "$(dirname "$0")"
-. ./variables.sh
-
-if [ "$CI" = "true" ]; then
-    curl -sL https://static.rust-lang.org/rustup.sh -o ~/rust-installer/rustup.sh
-    sh ~/rust-installer/rustup.sh --prefix=~/rust --spec=stable -y --disable-sudo 2> /dev/null
-fi
-
-if [ ! -d mysteryshack ]; then
-    git clone https://github.com/untitaker/mysteryshack
-fi
-
-pip install pytest-xprocess
-
-cd mysteryshack
-make debug-build  # such that first test doesn't hang too long w/o output

tests/storage/servers/mysteryshack/make.sh

@@ -1,9 +0,0 @@
-#!/bin/sh
-set -e
-
-# pytest-xprocess doesn't allow us to CD into a particular directory before
-# launching a command, so we do it here.
-cd "$(dirname "$0")"
-. ./variables.sh
-cd mysteryshack
-exec make "$@"

tests/storage/servers/mysteryshack/variables.sh

@@ -1 +0,0 @@
-export PATH="$PATH:$HOME/.cargo/bin/"

vdirsyncer/http.py

@@ -138,10 +138,11 @@ def request(
     if verify_fingerprint is not None:
         _install_fingerprint_adapter(session, verify_fingerprint)
 
-    session.hooks = dict(response=_fix_redirects)
+    session.hooks = {"response": _fix_redirects}
 
     func = session.request
 
+    logger.debug("=" * 20)
     logger.debug(f"{method} {url}")
     logger.debug(kwargs.get("headers", {}))
     logger.debug(kwargs.get("data", None))

vdirsyncer/storage/google.py

@@ -42,8 +42,7 @@ class GoogleSession(dav.DAVSession):
 
         token_file = expand_path(token_file)
         ui_worker = get_ui_worker()
-        f = lambda: self._init_token(token_file, client_id, client_secret)
+        ui_worker.put(lambda: self._init_token(token_file, client_id, client_secret))
-        ui_worker.put(f)
 
     def _init_token(self, token_file, client_id, client_secret):
         token = None

vdirsyncer/sync/__init__.py

@@ -127,9 +127,9 @@ def sync(
         raise BothReadOnly()
 
     if conflict_resolution == "a wins":
-        conflict_resolution = lambda a, b: a
+        conflict_resolution = lambda a, b: a  # noqa: E731
     elif conflict_resolution == "b wins":
-        conflict_resolution = lambda a, b: b
+        conflict_resolution = lambda a, b: b  # noqa: E731
 
     status_nonempty = bool(next(status.iter_old(), None))