Skip to content

test: Assert log warnings and errors - #8457

Merged
j-g00da merged 1 commit into
mainfrom
j-g00da/log_asserts
Aug 10, 2026
Merged

test: Assert log warnings and errors#8457
j-g00da merged 1 commit into
mainfrom
j-g00da/log_asserts

Conversation

@j-g00da

@j-g00da j-g00da commented Jul 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Adds assert_warn, assert_error and assert_warns_or_errors
methods to TestContext, that let us assert that
a certain warning or error is logged during the test.
Also asserts test logs should not contain any other
errors or warnings.

Adjusts tests accordingly.

j-g00da
j-g00da commented Jul 24, 2026
View reviewed changes
Comment thread src/receive_imf/receive_imf_tests.rs Outdated
@Hocuri

Hocuri commented Jul 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

After talking to @link2xt, we found some simplifications that are possible, and one bugfix that is needed:

  • The whole printing-logs-only-when-LogSink-is-dropped logic is historical (from before cargo nextest existed) and not needed anymore. Instead, print_logevent() can be called directly from the loop in fn subscribe. Then, we don't need the collected field.
  • With the PR as-is, calling assert_warn() on one TestContext will find also react to warnings on other TestContexts, because they share the same LogSink. Instead, each TestContext should get its own LogSink. This also is a simplification, because TestContextManager doesn't need a LogSink anymore, then.

@j-g00da j-g00da mentioned this pull request Jul 27, 2026
Merged
@j-g00da
j-g00da force-pushed the j-g00da/log_asserts branch 4 times, most recently from 8249945 to 2d7664d Compare July 30, 2026 12:24
j-g00da
j-g00da commented Jul 30, 2026
View reviewed changes
Comment thread src/test_utils.rs Outdated
@j-g00da
j-g00da force-pushed the j-g00da/log_asserts branch 7 times, most recently from 66768b3 to beedff7 Compare August 3, 2026 15:28
@j-g00da
j-g00da marked this pull request as ready for review August 3, 2026 15:28
@j-g00da j-g00da changed the title WIP: test: Assert log warnings and errors test: Assert log warnings and errors Aug 3, 2026
@j-g00da
j-g00da force-pushed the j-g00da/log_asserts branch 3 times, most recently from 71d806d to f3df7e8 Compare August 3, 2026 16:16
@j-g00da
j-g00da requested review from Hocuri and link2xt August 3, 2026 16:20
@j-g00da
j-g00da force-pushed the j-g00da/log_asserts branch 4 times, most recently from fe60a78 to bbf432d Compare August 4, 2026 08:07
Hocuri
Hocuri reviewed Aug 4, 2026
View reviewed changes
Comment thread src/test_utils.rs Outdated
Comment thread src/test_utils.rs Outdated
Comment thread src/test_utils.rs
Comment thread src/events/payload.rs Outdated
Comment thread src/test_utils.rs Outdated
Comment thread src/test_utils.rs Outdated
Comment thread src/test_utils.rs Outdated
Comment thread src/events.rs Outdated
@j-g00da
j-g00da force-pushed the j-g00da/log_asserts branch 3 times, most recently from f892b0b to 3d6f761 Compare August 4, 2026 13:33
@j-g00da
j-g00da requested a review from Hocuri August 4, 2026 13:38
@j-g00da
j-g00da force-pushed the j-g00da/log_asserts branch from 3d6f761 to 9af2008 Compare August 5, 2026 08:05
Hocuri
Hocuri reviewed Aug 7, 2026
View reviewed changes

@Hocuri Hocuri 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.

The only remaining point is the assert_many() function (see my comment there), then this is ready to be merged

Comment thread src/test_utils.rs
/// the first one takes precedence.
pub async fn assert_many(&self, mut assertions: Vec<(bool, &str, usize)>) {
'events: while let Ok(Ok(event)) =
tokio::time::timeout(Duration::from_secs(1), self.events.recv()).await

@Hocuri Hocuri Aug 7, 2026 •
edited
Loading

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 looks like it slows down every test assert_many by 1 second, because there is no break in the function body. For now that's fine because tests run in parallel anyways; in the future we should directly store the EventEmitter in InnerLogSink, rather than having a task (in the subscribe function) that takes the events out of one channel and writes them into another one. Then, we can probably use try_recv() here without the tests being flaky.

@Hocuri Hocuri Aug 10, 2026

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.

Can you open an issue or PR for this?

Comment thread src/test_utils.rs Outdated
@j-g00da
j-g00da force-pushed the j-g00da/log_asserts branch 2 times, most recently from 4ead306 to 53e14ea Compare August 10, 2026 10:10
@j-g00da
j-g00da requested a review from Hocuri August 10, 2026 10:11
Hocuri
Hocuri approved these changes Aug 10, 2026
View reviewed changes

@Hocuri Hocuri 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.

Nice 馃帀!

Comment thread src/test_utils.rs Outdated
Comment thread src/test_utils.rs
/// the first one takes precedence.
pub async fn assert_many(&self, mut assertions: Vec<(bool, &str, usize)>) {
'events: while let Ok(Ok(event)) =
tokio::time::timeout(Duration::from_secs(1), self.events.recv()).await

@Hocuri Hocuri Aug 10, 2026

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.

Can you open an issue or PR for this?

Comment thread src/test_utils.rs Outdated
Comment thread src/test_utils.rs Outdated
Comment thread src/test_utils.rs Outdated
@j-g00da
j-g00da force-pushed the j-g00da/log_asserts branch 2 times, most recently from 3d490b9 to a5e8620 Compare August 10, 2026 13:20
@j-g00da
test: Assert log warnings and errors
94b8c79 
Adds `assert_warn`, `assert_error` and `assert_warns_or_errors`
methods to `TestContext`, that let us assert that
a certain warning or error is logged during the test.
Also asserts test logs should not contain any other
errors or warnings.

Adjusts tests accordingly.

Signed-off-by: Jagoda 艢l膮zak <jslazak@jslazak.com>
@j-g00da
j-g00da force-pushed the j-g00da/log_asserts branch from a5e8620 to 94b8c79 Compare August 10, 2026 13:24
@j-g00da
j-g00da enabled auto-merge (squash) August 10, 2026 13:24
@j-g00da
j-g00da merged commit fd8c568 into main Aug 10, 2026
30 checks passed
@j-g00da
j-g00da deleted the j-g00da/log_asserts branch August 10, 2026 13:36
@j-g00da j-g00da mentioned this pull request Aug 10, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Hocuri Hocuri Hocuri approved these changes
@link2xt link2xt Awaiting requested review from link2xt

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@j-g00da @Hocuri