dbrgn / tracing-test

Access and evaluate tracing logs in async and sync tests.
Apache License 2.0
52 stars 26 forks source link

Allow overriding env_filter by setting the RUST_LOG env variable #12

Closed tgecho closed 2 years ago

tgecho commented 2 years ago

Fixes #6

I'm not sure if there's a use case or nice way to do something more programmatically configurable (especially if tests are doing logs_contain assertions), but this seems to match up well with normal logging/tracing usage and has worked well in my trivial experimentation.

It preserves the original behavior unless you set the env variable:

RUST_LOG=debug cargo test
dbrgn commented 2 years ago

Hm, this looks pretty simple and seems useful! It does not allow overriding the filter per-test (because the filter is set globally), but at least it's more configurable.

However, it means that if tests rely on the env variable being set properly to succeed, it means that they may fail if the RUST_LOG variable is set differently. I don't think that RUST_LOG should have an effect on test outcome.

Additionally, some people will have set RUST_LOG in their local env variables, so tests might fail on some people's computers even without explicitly setting RUST_LOG. That seems unexpected.

Do you have an idea for another configuration mechanism to allow overriding the filter globally? Maybe a crate-level attribute (#![traced_test_filter="debug"])?

tgecho commented 2 years ago

Yeah, I agree that the presence of an env variable potentially breaking tests is a non-starter. Of the two use cases, at this moment I'm more focused on debugging tests than asserting on log output, so the goal was to allow quickly changing the tracing output. I wonder if these are just fundamentally in mild tension?

I think I could work with a crate level attribute. Could it be done at the test level?

#[traced_test("debug")]
#[test]
fn plain_old_test() { ... }

I'm super new to tracing, but could it be plausible to have multiple subscribers? One for test assertions (strictly controlled) and one for debug logging? No idea how hard that would be. Probably just a single way to configure it should be sufficient :)

dbrgn commented 2 years ago

Could it be done at the test level?

At the moment, unfortunately not. The rationale can be found here: https://docs.rs/tracing-test/0.2.1/tracing_test/#rationale--why-you-need-this Essentially, if you launch a new thread (or spawn a task that's dispatched to another thread) in a traced scope, the subscriber will not be inherited. This means that logs from another thread will not be captured at all by a thread-local subscriber.

To avoid this, and to capture logs in all threads, I register a global subscriber. This way the logs are all captured, but the filters cannot be changed per-test, unless we add a requirement that tests may not run in parallel. (But this is something I'd like to avoid.)

Maybe there's a better solution nowadays? I don't know, but I'm open for ideas 🙂

For your situation, it seems that a crate-level attribute seems like an approach that could work.

dbrgn commented 2 years ago

Closing for now. Thanks anyways for your PR.