Skip to content

Conversation

@link2xt
Copy link
Collaborator

@link2xt link2xt commented Nov 9, 2025

Based on #7429

In the "multi-transport" PR #7348 I changed how folder configuration works. We no longer configure folders during the transport configuration, so folders will be configured later. mvbox_move is also disabled during first transport configuration, so enabling mvbox_move should be done after configuring the transport if needed, and then the test needs to wait for the IMAP loop to go idle again so UID validity of mvbox is known. CFFI tests that test folder handling did not do it, so I ported them to JSON-RPC and fixed the issues for all the tests that got broken. CFFI bindings are deprecated anyway and we want to move the tests to JSON-RPC eventually so CFFI bindings can be deleted.

Created new test_folders.py

Moved existing JSON-RPC tests:

  • test_reactions_for_a_reordering_move
  • test_delete_deltachat_folder

Ported tests:

  • test_move_works_on_self_sent
  • test_moved_markseen
  • test_markseen_message_and_mdn
  • test_mvbox_thread_and_trash (renamed to test_mvbox_and_trash)
  • test_scan_folders
  • test_move_works
  • test_move_avoids_loop
  • test_immediate_autodelete
  • test_trash_multiple_messages

The change also contains fixes for direct_imap fixture
needed to use IMAP IDLE in JSON-RPC tests.

@link2xt link2xt force-pushed the link2xt/test_dont_show_emails-jsonrpc branch from 969b5b7 to 609f30a Compare November 9, 2025 20:06
@link2xt link2xt marked this pull request as ready for review November 9, 2025 20:08
@link2xt link2xt mentioned this pull request Nov 9, 2025
16 tasks
@link2xt link2xt changed the title Port test_dont_show_emails test to JSON-RPC Port CFFI tests to JSON-RPC Nov 10, 2025
@link2xt link2xt marked this pull request as draft November 10, 2025 03:20
@link2xt link2xt force-pushed the link2xt/test_dont_show_emails-jsonrpc branch 5 times, most recently from dfa50b2 to 3369598 Compare November 11, 2025 17:50
def get_all_messages(self) -> list[MailMessage]:
assert not self._idling
return list(self.conn.fetch())
return list(self.conn.fetch(mark_seen=False))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

imap-tools have a weird default for the FETCH operation, it has mark_seen=True and marks all messages that we fetch with \Seen flag. This is directly in the readme: https://pypi.org/project/imap-tools/
Don't think anyone wants this even outside of tests, it is just a bad API: ikvk/imap_tools#179

class IdleManager:
def __init__(self, direct_imap) -> None:
self.direct_imap = direct_imap
self.log = direct_imap.account.log
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

account does not have log, this code got copy-pasted from CFFI bindings. I have removed log references that failed, previously this code was not working at all as nobody used IDLE on direct IMAP with JSON-RPC.

return res
return self.direct_imap.conn.idle.poll(timeout=timeout)

def wait_for_new_message(self, timeout=None) -> bytes:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We don't use timeouts anyway, arbitrary timeouts hardcoded in the tests are annoying as they fail when CI runner is paused. We have pytest timout for the whole test in case it gets stuck. Again, this was copy-pasted from CFFI and nobody used it.

@link2xt link2xt changed the title Port CFFI tests to JSON-RPC Port folder-related CFFI tests to JSON-RPC Nov 11, 2025
@link2xt link2xt marked this pull request as ready for review November 11, 2025 19:15
@link2xt link2xt requested review from Hocuri, Copilot and iequidoo and removed request for Copilot November 11, 2025 19:15
@link2xt link2xt force-pushed the link2xt/test_dont_show_emails-jsonrpc branch from 3369598 to 13ecfae Compare November 11, 2025 19:17
Copilot finished reviewing on behalf of link2xt November 11, 2025 19:17
@link2xt
Copy link
Collaborator Author

link2xt commented Nov 12, 2025

This is ready for review. Don't read the commit messages, one of them is even "WIP" inside, I am going to squash the whole thing anyway.

@link2xt link2xt force-pushed the link2xt/test_dont_show_emails-jsonrpc branch from 13ecfae to e24e218 Compare November 12, 2025 00:12
@link2xt link2xt changed the base branch from main to link2xt/flaky-test_send_receive_locations November 12, 2025 00:12
Copy link
Collaborator

@iequidoo iequidoo left a comment

Choose a reason for hiding this comment

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

Not easy to review because the code was moved, but as far as i can tell the same scenarios are covered, so looks good to me

Base automatically changed from link2xt/flaky-test_send_receive_locations to main November 12, 2025 05:50
@link2xt link2xt force-pushed the link2xt/test_dont_show_emails-jsonrpc branch from e24e218 to 7c072e6 Compare November 12, 2025 07:31
Created new test_folders.py

Moved existing JSON-RPC tests:
- test_reactions_for_a_reordering_move
- test_delete_deltachat_folder

Ported tests:
- test_move_works_on_self_sent
- test_moved_markseen
- test_markseen_message_and_mdn
- test_mvbox_thread_and_trash (renamed to test_mvbox_and_trash)
- test_scan_folders
- test_move_works
- test_move_avoids_loop
- test_immediate_autodelete
- test_trash_multiple_messages

The change also contains fixes for direct_imap fixture
needed to use IMAP IDLE in JSON-RPC tests.
@link2xt link2xt force-pushed the link2xt/test_dont_show_emails-jsonrpc branch from 7c072e6 to 84a9e6c Compare November 12, 2025 07:32
@link2xt link2xt merged commit 7b54954 into main Nov 12, 2025
29 checks passed
@link2xt link2xt deleted the link2xt/test_dont_show_emails-jsonrpc branch November 12, 2025 08:07
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.

3 participants