Skip to content

properly isolate FFI online tests (avoid caching) and reconnect on direct-imap failures - #8565

Merged
hpk42 merged 3 commits into
mainfrom
hpk/fix-flaky-tests-event-leackage
Aug 12, 2026
Merged

properly isolate FFI online tests (avoid caching) and reconnect on direct-imap failures#8565
hpk42 merged 3 commits into
mainfrom
hpk/fix-flaky-tests-event-leackage

Conversation

@hpk42

@hpk42 hpk42 commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

two commits to fix two test flakyness sources (EDIT: and one cleanup commit removing all caching-related logic from the FFI test plugin).

  1. (EDIT) When account addresses are reused across tests, unexpected events or un-drained messages from previous setup steps/test runs arrive can cause assertion failures. I think it's fine to just create new addresses on CI all the time. Creating 100-200 addresses per test run is worth the guaranteed isolation. Ultimately Postfix on the CI server is free to arbitrary delay messages between mailboxes, and re-using them randomly breaks test isoilation. Better to exclude this failure mode cleanly and avoid caching. RPC-tests also use no caching.

  2. under parallel test load transient socket resets during directimap SSL setup can crash tests during fixture initialization example: :

______________________ test_message_override_sender_name _______________________
[gw5] darwin -- Python 3.10.16 /Users/runner/work/core/core/python/.tox/py/bin/python

    def test_message_override_sender_name(acfactory, lp):
>       ac1, ac2 = acfactory.get_online_accounts(2)

tests/test_1_online.py:448: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
.tox/py/lib/pypy3.10/site-packages/deltachat/testplugin.py:370: in init_imap
    acc.direct_imap = DirectImap(acc)
.tox/py/lib/pypy3.10/site-packages/deltachat/direct_imap.py:45: in connect
    self.conn = MailBox(host, port, ssl_context=ssl.create_default_context())
...
../../../../hostedtoolcache/PyPy/3.10.16/arm64/lib/pypy3.10/_cffi_ssl/_stdssl/__init__.py:550:
>           raise pyssl_error(self, ret)
E           ConnectionResetError: [Errno 54] Connection reset by peer

Just retrying imap connect (like core does itself) is fine.

@link2xt

link2xt commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

First example https://github.com/chatmail/core/actions/runs/30563560871/job/90944100948#step:9:315 is in deltachat-rpc-client tests, but the commit (and the whole PR) is changing legacy python tests. There is no caching in deltachat-rpc-client already, so test_no_markseen_in_team_profile failed for some other reason. I think it was actually this already closed issue: #8446

@hpk42

hpk42 commented Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

First example https://github.com/chatmail/core/actions/runs/30563560871/job/90944100948#step:9:315 is in deltachat-rpc-client tests, but the commit (and the whole PR) is changing legacy python tests. There is no caching in deltachat-rpc-client already, so test_no_markseen_in_team_profile failed for some other reason. I think it was actually this already closed issue: #8446

fair enough. But if rpc-tests already do no caching, all the more reason the remove it from FFI based tests as well. I edited the PR description accordingly. Removing the caching actually removes >100 LOCs from the test support code.

@hpk42 hpk42 changed the title properly isolate online tests (caching is hard!) and reconnect on direct-imap failures properly isolate online tests (avoid caching) and reconnect on direct-imap failures Aug 11, 2026
@hpk42 hpk42 changed the title properly isolate online tests (avoid caching) and reconnect on direct-imap failures properly isolate FFI online tests (avoid caching) and reconnect on direct-imap failures Aug 11, 2026
Comment thread python/src/deltachat/direct_imap.py Outdated
self.conn.login(user, pw)
self.select_folder("INBOX")
return
except (OSError, ssl.SSLError, imaplib.IMAP4.error):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://docs.python.org/3/library/ssl.html#ssl.SSLError says ssl.SSLError is a subtype of OSError, so if OSError is catched, then there is no need to catch ssl.SSLError as well. Maybe even catch only ConnectionError.

IMAP errors probably should not be ignored, if we managed to connect and IMAP server says e.g. that we cannot login (because the test server has registration disabled, possibly automatically), then retrying forever will likely not help.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed it to OSError, IMAP4.abort with a comment.

@link2xt link2xt left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was also an attempt to remove caching at #5446, apparently reusing the databases saved some time on configuration (requesting autoconfig, logging in with a test session just to add transport), but all the message cleanup with key reuse is likely not worth it.

hpk42 added 3 commits August 12, 2026 21:02
As we are using a dedicated CI relay instance, we don't need to be careful with creating test addresses.
But re-using test addresses in a test running in a single worker can leak events/messages which compromises test isolation.
Instead of encountering random flaky tests because let's just not cache addresses to maximize test isolation.
…g to connect on first try

also core src/imap.rs retries connecting (2s with backoff) but direct_imap is only used in (CI) tests
and we can just keep trying every second until pytest timeout hits.
@hpk42
hpk42 force-pushed the hpk/fix-flaky-tests-event-leackage branch from dc30fd6 to 82f65ec Compare August 12, 2026 19:50
@hpk42

hpk42 commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

some measurements for running standard tox CFFI test suite:

  • fastest branch runs against fasted main: 28s - 29s, so maybe 4% slower
  • slowest branch runs against slowest mains: 49s-53s (but main had more of them)

it seems runs falls into one of the buckets. pytest durations says test_send_lots_of_offline_msgs sometimes spends 30s in teardown and that probably comes from scheduler.rs' let timeout_duration = std::time::Duration::from_secs(30); while an SMTP connection is in flight and teardown happens.

all in all the branch should not be noticably slower. running core tests against lxc containers should be noticeably faster, see #7926

@hpk42
hpk42 merged commit 3363f63 into main Aug 12, 2026
30 checks passed
@hpk42
hpk42 deleted the hpk/fix-flaky-tests-event-leackage branch August 12, 2026 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants