Skip to content

Conversation

@link2xt
Copy link
Collaborator

@link2xt link2xt commented Jun 4, 2024

This should fix flaky test_verified_group_vs_delete_server_after.

Fix #5201

@link2xt link2xt force-pushed the link2xt/select-with-uidvalidity branch 2 times, most recently from cdf8e30 to 3f218b0 Compare June 4, 2024 17:04
@link2xt link2xt changed the title fix: select folders using select_with_uidvalidity fix: do not miss new messages while expunging the folder Jun 4, 2024
@link2xt link2xt force-pushed the link2xt/select-with-uidvalidity branch from 3f218b0 to 003d178 Compare June 4, 2024 17:21
@link2xt
Copy link
Collaborator Author

link2xt commented Jun 4, 2024

Finally non-flaky CI, but need to think of a proper fix now.

@link2xt link2xt marked this pull request as ready for review June 4, 2024 21:27
@link2xt link2xt force-pushed the link2xt/select-with-uidvalidity branch from 01611be to e598748 Compare June 4, 2024 21:27
@link2xt
Copy link
Collaborator Author

link2xt commented Jun 4, 2024

On Windows tests failed once with "Backup provider did not start in time" message, but this is likely just because timeout is 10 seconds and GitHub Actions seem to sometimes pause/stop VMs:
https://github.com/deltachat/deltachat-core-rust/actions/runs/9374467861/job/25810847989?pr=5669
Looks like another thing we may want to fix to get non-flaky CI, but not caused by changes in this PR.
Otherwise CI is much less flaky with this change.

session.as_mut().set_read_timeout(Some(IMAP_TIMEOUT));
self.inner = session;

// Fetch mail once we exit IDLE.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do i correctly understand that this is just in case and not related to the fix itself? Can it be done under if self.server_sent_unsolicited_exists(context)? if so?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is not just in case, without this line tests will fail.

If IDLE exits because of NewData, we want session.new_mail be true next time fetch_new_messages is called. Previously fetch_new_messages called session.select_with_uidvalidity and used its return value to decide if it should try to fetch messages. session.select_with_uidvalidity returned true if the folder was already selected, so most of the time fetch happened.

In cases other than NewData it may be not necessary, but other cases are rare so better try fetching. E.g. in case of manual interrupt it seems better to fetch, because who knows what kind of broken IMAP servers are there, closing and reopening the app should not rely on correctly working IDLE to try fetching messages.

I think IDLE response does not get into unsolicited channel, so checking for unsolicited EXISTS is not enough. Unsolicited EXISTS is an EXISTS response that you get e.g. when issuing a FETCH or STORE command.

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.

Test test_verified_group_vs_delete_server_after is flaky

3 participants