remove _configtracker and write a tested "PendingConfigure" helper class for managing configure/started states for test accounts

python/src/deltachat/account.py

@@ -67,6 +67,9 @@ class Account(object):
         """ re-enable logging. """
         self._logging = True
 
+    def __repr__(self):
+        return "<Account path={}>".format(self.db_path)
+
     # def __del__(self):
     #    self.shutdown()
 
@@ -571,6 +574,8 @@ class Account(object):
         """ add an account plugin which implements one or more of
         the :class:`deltachat.hookspec.PerAccount` hooks.
         """
+        if name and self._pm.has_plugin(name=name):
+            self._pm.unregister(name=name)
         self._pm.register(plugin, name=name)
         self._pm.check_pending()
         return plugin

python/src/deltachat/testplugin.py

@@ -9,12 +9,13 @@ import fnmatch
 import time
 import weakref
 import tempfile
+from queue import Queue
 from typing import List, Callable
 
 import pytest
 import requests
 
-from . import Account, const
+from . import Account, const, account_hookimpl
 from .events import FFIEventLogger, FFIEventTracker
 from _pytest._code import Source
 
@@ -210,6 +211,56 @@ def data(request):
     return Data()
 
 
+class PendingConfigure:
+    CONFIGURING = "CONFIGURING"
+    CONFIGURED = "CONFIGURED"
+    POSTPROCESSED = "POSTPROCESSED"
+
+    def __init__(self):
+        self._configured_events = Queue()
+        self._account2state = {}
+
+    def add_account(self, acc, reconfigure=False):
+        class PendingTracker:
+            @account_hookimpl
+            def ac_configure_completed(this, success):
+                self._configured_events.put((acc, success))
+
+        acc.add_account_plugin(PendingTracker(), name="pending_tracker")
+        self._account2state[acc] = self.CONFIGURING
+        acc.configure(reconfigure=reconfigure)
+        print("started configure on pending", acc)
+
+    def wait_all(self, onconfigured=lambda x: None):
+        """ Wait for all accounts to finish configuration.
+        """
+        print("wait_all finds accounts=", self._account2state)
+        for acc, state in self._account2state.items():
+            if state == self.CONFIGURED:
+                onconfigured(acc)
+                self._account2state[acc] = self.POSTPROCESSED
+
+        while self.CONFIGURING in self._account2state.values():
+            acc, success = self._pop_one()
+            onconfigured(acc)
+            self._account2state[acc] = self.POSTPROCESSED
+        print("finished, account2state", self._account2state)
+
+    def wait_one(self, account):
+        if self._account2state[account] == self.CONFIGURING:
+            while 1:
+                acc, success = self._pop_one()
+                if acc == account:
+                    break
+
+    def _pop_one(self):
+        acc, success = self._configured_events.get()
+        if not success:
+            pytest.fail("configuring online account failed: {}".format(acc))
+        self._account2state[acc] = self.CONFIGURED
+        return (acc, success)
+
+
 class ACFactory:
     _finalizers: List[Callable[[], None]]
     _accounts: List[Account]
@@ -224,6 +275,8 @@ class ACFactory:
 
         self._finalizers = []
         self._accounts = []
+        self._pending_configure = PendingConfigure()
+        self._imap_cleaned = set()
         self._preconfigured_keys = ["alice", "bob", "charlie",
                                     "dom", "elena", "fiona"]
         self.set_logging_default(False)
@@ -302,11 +355,18 @@ class ACFactory:
         self._preconfigure_key(ac, addr)
         return ac
 
-    def new_online_configuring_account(self, **kwargs):
+    def new_online_configuring_account(self, cloned_from=None, **kwargs):
-        configdict = self.get_next_liveconfig()
+        if cloned_from is None:
+            configdict = self.get_next_liveconfig()
+        else:
+            # XXX we might want to transfer the key to the new account
+            configdict = dict(
+                addr=cloned_from.get_config("addr"),
+                mail_pw=cloned_from.get_config("mail_pw"),
+            )
         configdict.update(kwargs)
         ac = self.prepare_account_from_liveconfig(configdict)
-        ac._configtracker = ac.configure()
+        self._pending_configure.add_account(ac)
         return ac
 
     def prepare_account_from_liveconfig(self, configdict):
@@ -320,40 +380,32 @@ class ACFactory:
         return ac
 
     def new_cloned_configuring_account(self, account):
-        """ Clones addr, mail_pw, mvbox_move, sentbox_watch and the
+        return self.new_online_configuring_account(cloned_from=account)
-        direct_imap object of an online account. This simulates the user setting
-        up a new device without importing a backup.
-        """
-        # XXX we might want to transfer the key to the new account
-        ac = self.prepare_account_from_liveconfig(dict(
-            addr=account.get_config("addr"),
-            mail_pw=account.get_config("mail_pw"),
-        ))
-        if hasattr(account, "direct_imap"):
-            # Attach the existing direct_imap. If we did not do this, a new one would be created and
-            # delete existing messages (see dc_account_extra_configure(configure))
-            ac.direct_imap = account.direct_imap
-        ac._configtracker = ac.configure()
-        return ac
 
-    def bring_accounts_online(self):
+    def _onconfigure_start_io(self, acc):
-        for acc in self._accounts:
+        acc.start_io()
-            self.wait_configure(acc)
+        print(acc._logid, "waiting for inbox IDLE to become ready")
-            acc.start_io()
+        acc._evtracker.wait_idle_inbox_ready()
-            print("waiting for inbox IDLE to become ready")
+        self.init_direct_imap_and_logging(acc)
-            acc._evtracker.wait_idle_inbox_ready()
+        acc.get_device_chat().mark_noticed()
-            logger = FFIEventLogger(acc, logid=acc._logid, init_time=self.init_time)
+        acc._evtracker.consume_events()
-            acc.add_account_plugin(logger)
+        acc.log("inbox IDLE ready")
-            acc.log("inbox IDLE ready!")
+
+    def init_direct_imap_and_logging(self, acc):
+        """ idempotent function for initializing direct_imap and logging for an account. """
+        self.init_direct_imap(acc)
+        logger = FFIEventLogger(acc, logid=acc._logid, init_time=self.init_time)
+        acc.add_account_plugin(logger, name=acc._logid)
 
     def wait_configure(self, acc):
-        if hasattr(acc, "_configtracker"):
+        self._pending_configure.wait_one(acc)
-            acc._configtracker.wait_finish()
+        self.init_direct_imap_and_logging(acc)
-            acc._evtracker.consume_events()
+        acc._evtracker.consume_events()
-            acc.get_device_chat().mark_noticed()
+
-            del acc._configtracker
+    def bring_accounts_online(self):
-            if not hasattr(acc, "direct_imap"):
+        print("bringing accounts online")
-                self.init_direct_imap(acc)
+        self._pending_configure.wait_all(onconfigured=self._onconfigure_start_io)
+        print("all accounts online")
 
     def get_online_accounts(self, num):
         # to reduce number of log events logging starts after accounts can receive
@@ -397,15 +449,19 @@ class ACFactory:
 
     def init_direct_imap(self, acc):
         from deltachat.direct_imap import DirectImap
-
         if not hasattr(acc, "direct_imap"):
-            acc.direct_imap = imap = DirectImap(acc)
+            acc.direct_imap = DirectImap(acc)
+        addr = acc.get_config("addr")
+        if addr not in self._imap_cleaned:
+            imap = acc.direct_imap
             for folder in imap.list_folders():
                 if folder.lower() == "inbox" or folder.lower() == "deltachat":
                     assert imap.select_folder(folder)
                     imap.delete("1:*", expunge=True)
                 else:
                     imap.conn.folder.delete(folder)
+            acc.log("imap cleaned for addr {}".format(addr))
+            self._imap_cleaned.add(addr)
 
     def dump_imap_summary(self, logfile):
         for ac in self._accounts:

python/tests/test_account.py

@@ -697,11 +697,10 @@ class TestOnlineAccount:
 
     def test_configure_canceled(self, acfactory):
         ac1 = acfactory.new_online_configuring_account()
-        ac1._configtracker.wait_progress()
         ac1.stop_ongoing()
         try:
-            ac1._configtracker.wait_finish()
+            acfactory._pending_configure.wait_one(ac1)
-        except Exception:
+        except pytest.fail.Exception:
             pass
 
     def test_export_import_self_keys(self, acfactory, tmpdir, lp):
@@ -2402,7 +2401,7 @@ class TestOnlineAccount:
         ac3.stop_io()
         acfactory.remove_preconfigured_keys()
         ac4 = acfactory.new_cloned_configuring_account(ac3)
-        ac4._configtracker.wait_finish()
+        acfactory._pending_configure.wait_one(ac4)
         # Create contacts to make sure incoming messages are not treated as contact requests
         chat41 = ac4.create_chat(ac1)
         chat42 = ac4.create_chat(ac2)
@@ -2736,18 +2735,19 @@ class TestOnlineAccount:
         acfactory.wait_configure(ac1)
         ac1.direct_imap.create_folder(folder)
 
-        acfactory.bring_accounts_online()
         # Wait until each folder was selected once and we are IDLEing:
+        acfactory.bring_accounts_online()
         ac1.stop_io()
+        assert folder in ac1.direct_imap.list_folders()
 
-        # Send a message to ac1 and move it to the mvbox:
+        lp.sec("Send a message to from ac2 to ac1 and manually move it to the mvbox")
         ac1.direct_imap.select_config_folder("inbox")
         ac1.direct_imap.idle_start()
         acfactory.get_accepted_chat(ac2, ac1).send_text("hello")
         ac1.direct_imap.idle_wait_for_new_message(terminate=True)
         ac1.direct_imap.conn.move(["*"], folder)  # "*" means "biggest UID in mailbox"
 
-        lp.sec("Everything prepared, now see if DeltaChat finds the message (" + variant + ")")
+        lp.sec("start_io() and see if DeltaChat finds the message (" + variant + ")")
         ac1.set_config("scan_all_folders_debounce_secs", "0")
         ac1.start_io()
         msg = ac1._evtracker.wait_next_incoming_message()
@@ -2778,9 +2778,7 @@ class TestOnlineAccount:
 
         ac1 = acfactory.new_online_configuring_account(mvbox_move=mvbox_move)
         ac2 = acfactory.new_online_configuring_account()
-
         acfactory.wait_configure(ac1)
-
         ac1.direct_imap.create_folder("Sent")
         ac1.set_config("sentbox_watch", "1")
 
@@ -2789,7 +2787,7 @@ class TestOnlineAccount:
         # would also find the "Sent" folder, but it would be too late:
         # The sentbox thread, started by `start_io()`, would have seen that there is no
         # ConfiguredSentboxFolder and do nothing.
-        ac1._configtracker = ac1.configure(reconfigure=True)
+        acfactory._pending_configure.add_account(ac1, reconfigure=True)
         acfactory.bring_accounts_online()
         assert_folders_configured(ac1)
 
@@ -2809,7 +2807,7 @@ class TestOnlineAccount:
         lp.sec("create a cloned ac1 and fetch contact history during configure")
         ac1_clone = acfactory.new_cloned_configuring_account(ac1)
         ac1_clone.set_config("fetch_existing_msgs", "1")
-        ac1_clone._configtracker.wait_finish()
+        acfactory.wait_configure(ac1_clone)
         ac1_clone.start_io()
         assert_folders_configured(ac1_clone)
 
@@ -2855,7 +2853,7 @@ class TestOnlineAccount:
         lp.sec("Clone online account and let it fetch the existing messages")
         ac1_clone = acfactory.new_cloned_configuring_account(ac1)
         ac1_clone.set_config("fetch_existing_msgs", "1")
-        ac1_clone._configtracker.wait_finish()
+        acfactory.wait_configure(ac1_clone)
 
         ac1_clone.start_io()
         ac1_clone._evtracker.wait_idle_inbox_ready()

python/tests/test_lowlevel.py

@@ -6,9 +6,47 @@ from deltachat import register_global_plugin
 from deltachat.hookspec import global_hookimpl
 from deltachat.capi import ffi
 from deltachat.capi import lib
+from deltachat.testplugin import PendingConfigure
 # from deltachat.account import EventLogger
 
 
+class TestPendingConfigure:
+    def test_basic_states(self, acfactory, monkeypatch):
+        pc = PendingConfigure()
+        acc = acfactory.get_unconfigured_account()
+        monkeypatch.setattr(acc, "configure", lambda **kwargs: None)
+        pc.add_account(acc)
+        assert pc._account2state[acc] == pc.CONFIGURING
+        pc._configured_events.put((acc, True))
+        pc.wait_one(acc)
+        assert pc._account2state[acc] == pc.CONFIGURED
+        accounts = []
+        pc.wait_all(onconfigured=accounts.append)
+        assert pc._account2state[acc] == pc.POSTPROCESSED
+        assert accounts == [acc]
+
+    def test_two_accounts_one_waited_all_started(self, monkeypatch, acfactory):
+        pc = PendingConfigure()
+        ac1 = acfactory.get_unconfigured_account()
+        monkeypatch.setattr(ac1, "configure", lambda **kwargs: None)
+        pc.add_account(ac1)
+        ac2 = acfactory.get_unconfigured_account()
+        monkeypatch.setattr(ac2, "configure", lambda **kwargs: None)
+        pc.add_account(ac2)
+        assert pc._account2state[ac1] == pc.CONFIGURING
+        assert pc._account2state[ac2] == pc.CONFIGURING
+        pc._configured_events.put((ac1, True))
+        pc.wait_one(ac1)
+        assert pc._account2state[ac1] == pc.CONFIGURED
+        assert pc._account2state[ac2] == pc.CONFIGURING
+        accounts = []
+        pc._configured_events.put((ac2, True))
+        pc.wait_all(onconfigured=accounts.append)
+        assert pc._account2state[ac1] == pc.POSTPROCESSED
+        assert pc._account2state[ac2] == pc.POSTPROCESSED
+        assert accounts == [ac1, ac2]
+
+
 def test_empty_context():
     ctx = capi.lib.dc_context_new(capi.ffi.NULL, capi.ffi.NULL, capi.ffi.NULL)
     capi.lib.dc_context_unref(ctx)