Skip to content

wip: refactor log sink - #8587

Draft
j-g00da wants to merge 1 commit into
mainfrom
j-g00da/refactor-log-sink
Draft

wip: refactor log sink#8587
j-g00da wants to merge 1 commit into
mainfrom
j-g00da/refactor-log-sink

Conversation

@j-g00da

@j-g00da j-g00da commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Closes: #8555

Closes: #8555
Signed-off-by: Jagoda 艢l膮zak <jslazak@jslazak.com>
@j-g00da

j-g00da commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

This is a draft, not really sure about any of this yet.
Few major issues which I still need to go through:

  1. Removing messages from the channel using e.g. get_matching will cause some logs to not be printed or asserted.
  2. Two tests are still flaky and need a timeout... I temporarily added sleep there.

bob.assert_warn("receiving from an empty and closed channel")
.await;
// wait for logs
tokio::time::sleep(Duration::from_millis(500)).await;

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.

This can likely be fixed by join() for recently seen loop task before dropping recently_seen_loop, probably need some changes there so it can be shut down cleanly as outside the tests we currently just abort it from the scheduler.

.await;
alice.assert_warn("IMAP got rate limited").await;
// wait for logs
tokio::time::sleep(Duration::from_millis(500)).await;

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.

Not sure, but maybe this case can be fixed by inserting EventType::Test and then reading events until finding it in assert_warns_or_errors, similarly to what get_matching_ext does.

Comment thread src/test_utils.rs
}

/// Returns a reference to the event tracker.
pub fn get_evtracker(&self) -> &EventTracker {

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.

nit: getters normally don't have a get_ prefix in rust (https://rust-lang.github.io/api-guidelines/naming.html#getter-names-follow-rust-convention-c-getter). I think we have it in several places anyway, also because of C conventions around FFI and older code.

@link2xt

link2xt commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

Removing messages from the channel using e.g. get_matching will cause some logs to not be printed or asserted.

This can likely be solved by creating another broadcast channel receiver and reading from it. Not as simple as expected originally, but will avoid the task (the one that was spawned in subscribe() method) copying events between channels asynchronously.

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.

Refactor LogSink

2 participants