Skip to content

Commit

Permalink
api!: remove dc_all_work_done()
Browse files Browse the repository at this point in the history
Also cleaned up test_connectivity()
which tested that state does not flicker to WORKING
when there are no messages to be fetched.
The state is expected to flicker to WORKING
when checking for new messages,
so the tests were outdated since
change 3b0b237
  • Loading branch information
link2xt committed Dec 4, 2024
1 parent 6468806 commit e694411
Show file tree
Hide file tree
Showing 6 changed files with 5 additions and 79 deletions.
6 changes: 0 additions & 6 deletions deltachat-ffi/deltachat.h
Original file line number Diff line number Diff line change
Expand Up @@ -722,12 +722,6 @@ char* dc_get_connectivity_html (dc_context_t* context);
int dc_get_push_state (dc_context_t* context);


/**
* Only used by the python tests.
*/
int dc_all_work_done (dc_context_t* context);


// connect

/**
Expand Down
10 changes: 0 additions & 10 deletions deltachat-ffi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -413,16 +413,6 @@ pub unsafe extern "C" fn dc_get_push_state(context: *const dc_context_t) -> libc
block_on(ctx.push_state()) as libc::c_int
}

#[no_mangle]
pub unsafe extern "C" fn dc_all_work_done(context: *mut dc_context_t) -> libc::c_int {
if context.is_null() {
eprintln!("ignoring careless call to dc_all_work_done()");
return 0;
}
let ctx = &*context;
block_on(async move { ctx.all_work_done().await as libc::c_int })
}

#[no_mangle]
pub unsafe extern "C" fn dc_get_oauth2_url(
context: *mut dc_context_t,
Expand Down
3 changes: 0 additions & 3 deletions python/src/deltachat/account.py
Original file line number Diff line number Diff line change
Expand Up @@ -671,9 +671,6 @@ def get_connectivity(self):
def get_connectivity_html(self) -> str:
return from_dc_charpointer(lib.dc_get_connectivity_html(self._dc_context))

def all_work_done(self):
return lib.dc_all_work_done(self._dc_context)

def start_io(self):
"""start this account's IO scheduling (Rust-core async scheduler).
Expand Down
6 changes: 0 additions & 6 deletions python/src/deltachat/events.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,12 +158,6 @@ def wait_for_connectivity_change(self, previous, expected_next):

self.get_matching("DC_EVENT_CONNECTIVITY_CHANGED")

def wait_for_all_work_done(self):
while True:
if self.account.all_work_done():
return
self.get_matching("DC_EVENT_CONNECTIVITY_CHANGED")

def ensure_event_not_queued(self, event_name_regex):
__tracebackhide__ = True
rex = re.compile(f"(?:{event_name_regex}).*")
Expand Down
55 changes: 3 additions & 52 deletions python/tests/test_1_online.py
Original file line number Diff line number Diff line change
Expand Up @@ -1900,9 +1900,10 @@ def test_connectivity(acfactory, lp):
ac1.start_io()
ac1._evtracker.wait_for_connectivity(dc.const.DC_CONNECTIVITY_CONNECTING)
ac1._evtracker.wait_for_connectivity_change(dc.const.DC_CONNECTIVITY_CONNECTING, dc.const.DC_CONNECTIVITY_WORKING)
ac1._evtracker.wait_for_connectivity_change(dc.const.DC_CONNECTIVITY_WORKING, dc.const.DC_CONNECTIVITY_CONNECTED)

lp.sec(
"Test that after calling start_io(), maybe_network() and waiting for `all_work_done()`, "
"Test that after calling start_io(), maybe_network() and waiting for `DC_CONNECTIVITY_CONNECTED`, "
"all messages are fetched",
)

Expand All @@ -1911,7 +1912,7 @@ def test_connectivity(acfactory, lp):
ac2.create_chat(ac1).send_text("Hi")
idle1.wait_for_new_message()
ac1.maybe_network()
ac1._evtracker.wait_for_all_work_done()
ac1._evtracker.wait_for_connectivity(dc.const.DC_CONNECTIVITY_CONNECTED)
msgs = ac1.create_chat(ac2).get_messages()
assert len(msgs) == 1
assert msgs[0].text == "Hi"
Expand All @@ -1927,30 +1928,6 @@ def test_connectivity(acfactory, lp):
assert len(msgs) == 2
assert msgs[1].text == "Hi 2"

lp.sec("Test that the connectivity doesn't flicker to WORKING if there are no new messages")

ac1.maybe_network()
while 1:
assert ac1.get_connectivity() == dc.const.DC_CONNECTIVITY_CONNECTED
if ac1.all_work_done():
break
ac1._evtracker.get_matching("DC_EVENT_CONNECTIVITY_CHANGED")

lp.sec("Test that the connectivity doesn't flicker to WORKING if the sender of the message is blocked")
ac1.create_contact(ac2).block()

ac1.direct_imap.select_config_folder("inbox")
with ac1.direct_imap.idle() as idle1:
ac2.create_chat(ac1).send_text("Hi")
idle1.wait_for_new_message()
ac1.maybe_network()

while 1:
assert ac1.get_connectivity() == dc.const.DC_CONNECTIVITY_CONNECTED
if ac1.all_work_done():
break
ac1._evtracker.get_matching("DC_EVENT_CONNECTIVITY_CHANGED")

lp.sec("Test that the connectivity is NOT_CONNECTED if the password is wrong")

ac1.set_config("configured_mail_pw", "abc")
Expand All @@ -1961,32 +1938,6 @@ def test_connectivity(acfactory, lp):
ac1._evtracker.wait_for_connectivity(dc.const.DC_CONNECTIVITY_NOT_CONNECTED)


def test_all_work_done(acfactory, lp):
"""
Tests that calling start_io() immediately followed by maybe_network()
and then waiting for all_work_done() reliably fetches the messages
delivered while account was offline.
In other words, connectivity should not change to a state
where all_work_done() returns true until IMAP connection goes idle.
"""
ac1, ac2 = acfactory.get_online_accounts(2)

ac1.stop_io()
ac1._evtracker.wait_for_connectivity(dc.const.DC_CONNECTIVITY_NOT_CONNECTED)

ac1.direct_imap.select_config_folder("inbox")
with ac1.direct_imap.idle() as idle1:
ac2.create_chat(ac1).send_text("Hi")
idle1.wait_for_new_message()

ac1.start_io()
ac1.maybe_network()
ac1._evtracker.wait_for_all_work_done()
msgs = ac1.create_chat(ac2).get_messages()
assert len(msgs) == 1
assert msgs[0].text == "Hi"


def test_fetch_deleted_msg(acfactory, lp):
"""This is a regression test: Messages with \\Deleted flag were downloaded again and again,
hundreds of times, because uid_next was not updated.
Expand Down
4 changes: 2 additions & 2 deletions src/scheduler/connectivity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ impl DetailedConnectivity {
DetailedConnectivity::Uninitialized => Some(Connectivity::NotConnected),
DetailedConnectivity::Connecting => Some(Connectivity::Connecting),
DetailedConnectivity::Working => Some(Connectivity::Working),
DetailedConnectivity::InterruptingIdle => Some(Connectivity::Connected),
DetailedConnectivity::InterruptingIdle => Some(Connectivity::Working),

// At this point IMAP has just connected,
// but does not know yet if there are messages to download.
Expand Down Expand Up @@ -201,7 +201,7 @@ impl ConnectivityStore {
}

/// Set all folder states to InterruptingIdle in case they were `Idle` before.
/// Called during `dc_maybe_network()` to make sure that `dc_all_work_done()`
/// Called during `dc_maybe_network()` to make sure that `all_work_done()`
/// returns false immediately after `dc_maybe_network()`.
pub(crate) async fn idle_interrupted(inbox: ConnectivityStore, oboxes: Vec<ConnectivityStore>) {
let mut connectivity_lock = inbox.0.lock().await;
Expand Down

0 comments on commit e694411

Please sign in to comment.