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

fix max_level_hint for empty cases for Option/Vec Subscribe impls #2195

Merged
merged 10 commits into from
Jul 1, 2022
Prev Previous commit
Next Next commit
cleanup tests and comments
guswynn committed Jun 30, 2022
commit 219958b86c24ca13efc59c407f814cce9f5b3d78
5 changes: 3 additions & 2 deletions tracing-subscriber/src/subscribe/mod.rs
Original file line number Diff line number Diff line change
@@ -1486,7 +1486,8 @@ where
match self {
Some(ref inner) => inner.max_level_hint(),
None => {
// There is no underlying `Subscribe` so our max level is OFF
// There is no inner subscriber, so this subscriber will
// never enable anything.
Some(LevelFilter::OFF)
}
}
@@ -1693,7 +1694,7 @@ feature! {
}

fn max_level_hint(&self) -> Option<LevelFilter> {
// Default to `OFF` if there are no underlying `Subscribe`s
// Default to `OFF` if there are no underlying subscribers
let mut max_level = LevelFilter::OFF;
for s in self {
// NOTE(eliza): this is slightly subtle: if *any* subscriber
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
use super::*;
mod support;
use self::support::*;
use tracing::Collect;
use tracing_subscriber::{filter, prelude::*, Subscribe};
use tracing::{level_filters::LevelFilter, Level};

// This test is just used to compare to the tests below
#[test]
fn just_layer() {
fn just_subscriber() {
let info = subscriber::named("info")
.run()
.with_filter(LevelFilter::INFO)
@@ -14,7 +17,7 @@ fn just_layer() {
}

#[test]
fn layer_and_option_layer() {
fn subscriber_and_option_some_subscriber() {
let info = subscriber::named("info")
.run()
.with_filter(LevelFilter::INFO)
@@ -25,21 +28,36 @@ fn layer_and_option_layer() {
.with_filter(LevelFilter::DEBUG)
.boxed();

let mut collector = tracing_subscriber::registry().with(info).with(Some(debug));
let collector = tracing_subscriber::registry().with(info).with(Some(debug));
assert_eq!(collector.max_level_hint(), Some(LevelFilter::DEBUG));
}

#[test]
fn subscriber_and_option_none_subscriber() {
let error = subscriber::named("error")
.run()
.with_filter(LevelFilter::ERROR)
.boxed();
// None means the other layer takes control
collector = tracing_subscriber::registry().with(error).with(None);
let collector = tracing_subscriber::registry()
.with(error)
.with(None::<ExpectSubscriber>);
assert_eq!(collector.max_level_hint(), Some(LevelFilter::ERROR));
}

#[test]
fn just_option_layer() {
fn just_option_some_subscriber() {
// Just a None means everything is off
let collector = tracing_subscriber::registry().with(None::<ExpectSubscriber>);
assert_eq!(collector.max_level_hint(), Some(LevelFilter::OFF));
}

#[test]
fn just_option_none_subscriber() {
let error = subscriber::named("error")
.run()
.with_filter(LevelFilter::ERROR)
.boxed();
let collector = tracing_subscriber::registry().with(Some(error));
assert_eq!(collector.max_level_hint(), Some(LevelFilter::ERROR));
}
124 changes: 0 additions & 124 deletions tracing-subscriber/tests/subscriber_filters/vec.rs

This file was deleted.

25 changes: 25 additions & 0 deletions tracing-subscriber/tests/vec.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
mod support;
use self::support::*;
use tracing::Collect;
use tracing::{level_filters::LevelFilter, Level};
use tracing_subscriber::{filter, prelude::*, Subscribe};

#[test]
fn just_empty_vec() {
// Just a None means everything is off
let collector = tracing_subscriber::registry().with(Vec::<ExpectSubscriber>::new());
assert_eq!(collector.max_level_hint(), Some(LevelFilter::OFF));
}

#[test]
fn subscriber_and_empty_vec() {
let info = subscriber::named("info")
.run()
.with_filter(LevelFilter::INFO)
.boxed();

let collector = tracing_subscriber::registry()
.with(info)
.with(Vec::<ExpectSubscriber>::new());
assert_eq!(collector.max_level_hint(), Some(LevelFilter::INFO));
}