Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: Add test utilities to configure logging #1060

Merged
merged 2 commits into from
Jun 2, 2023
Merged

Conversation

flub
Copy link
Contributor

@flub flub commented Jun 1, 2023

This is useful in many tests, not just netcheck. Make it more globally available.

It also makes it better to enable by default: it just always logs all modules on the TRACE level. But it is properly captured by the test runner and only shown if the test fails.

Potentially this is too verbose for some tests. Variants can be built that allow configuring the logging level or even per-modules logging level as required. But no need for that just yet.

This is useful in many tests, not just netcheck.  Make it more
globally available.

It also makes it better to enable by default: it just always logs
all modules on the TRACE level.  But it is properly captured by the
test runner and only shown if the test fails.

Potentially this is too verbose for some tests.  Variants can be built
that allow configuring the logging level or even per-modules logging
level as required.  But no need for that just yet.
pub(crate) fn setup_logging() -> tracing::subscriber::DefaultGuard {
let log_layer = tracing_subscriber::fmt::layer()
.with_writer(|| TestWriter)
.with_filter(LevelFilter::TRACE);
Copy link
Contributor

Choose a reason for hiding this comment

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

So it is no longer possible to configure log level with RUST_LOG? I find trace to be a bit too much usually...

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, being able to use RUST_LOG for filtering would be great

@rklaehn
Copy link
Contributor

rklaehn commented Jun 1, 2023

I think this will somewhat interfere with my workflow. I like to use println!, but that will now be drowned in all the log msgs. Also, I find it useful to dial the log level using RUST_LOG=info/error/debug to get an overview.

Can we change it to pick up RUST_LOG again, with trace being the default if the env var is not set?

@flub
Copy link
Contributor Author

flub commented Jun 2, 2023

PTAL: I updated this to use RUST_LOG if it is present but fall back to logging everything on trace level if it is not set. This allows us full logs on CI but still let's us customise it when manually poking at things.

@flub flub merged commit 8448cb6 into x-hp Jun 2, 2023
@flub flub deleted the flub/test-logging branch June 2, 2023 15:55
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