From 9f01a9d6fb12ed7c61f273d41f01c1cb658979b8 Mon Sep 17 00:00:00 2001 From: Hayden Stainsby Date: Fri, 4 Nov 2022 17:11:53 +0100 Subject: [PATCH 1/2] mock: move subscriber mock from tracing-subscriber tests The `tracing-subscriber` module `tests::support` included functionality to mock a subscriber (via the `Subscribe` trait). This code depends on some items from `tracing_mock::collector` which should otherwise not be public. This change moves the mocking functionality inside `tracing-mock` behind a feature flag. Allowing the `Expect` enum and `MockHandle::new` from `tracing_mock::collector` to be made `pub(crate)` instead of `pub`. Since it's now used from two different modules, the `Expect` enum has been moved to its own module. This requires a lot of modifications to imports so that we're not doing wildcard imports from another crate (i.e. in `tracing-subscriber` importing wildcards from `tracing-mock`). Closes: #2359 --- tracing-mock/Cargo.toml | 5 +++ tracing-mock/src/collector.rs | 21 ++---------- tracing-mock/src/expectation.rs | 21 ++++++++++++ tracing-mock/src/lib.rs | 5 +++ .../src/subscriber.rs | 32 ++++++++----------- tracing-subscriber/Cargo.toml | 2 +- ...er_filters_dont_break_other_subscribers.rs | 6 ++-- tracing-subscriber/tests/env_filter/main.rs | 5 +-- .../tests/env_filter/per_subscriber.rs | 1 + ...er_filters_dont_break_other_subscribers.rs | 6 ++-- ...iple_subscriber_filter_interests_cached.rs | 4 +-- .../subscriber_filter_interests_are_cached.rs | 4 +-- .../tests/subscriber_filters/filter_scopes.rs | 1 + .../tests/subscriber_filters/main.rs | 4 +-- .../tests/subscriber_filters/per_event.rs | 2 +- .../tests/subscriber_filters/trees.rs | 1 + .../tests/subscriber_filters/vec.rs | 1 + ...er_filters_dont_break_other_subscribers.rs | 6 ++-- .../vec_subscriber_filter_interests_cached.rs | 7 ++-- 19 files changed, 74 insertions(+), 60 deletions(-) create mode 100644 tracing-mock/src/expectation.rs rename tracing-subscriber/tests/support.rs => tracing-mock/src/subscriber.rs (96%) diff --git a/tracing-mock/Cargo.toml b/tracing-mock/Cargo.toml index 92a9818ff9..a9bf030a29 100644 --- a/tracing-mock/Cargo.toml +++ b/tracing-mock/Cargo.toml @@ -17,9 +17,14 @@ edition = "2018" rust-version = "1.49.0" publish = false +[features] +default = [] +subscriber = ["tracing-subscriber"] + [dependencies] tracing = { path = "../tracing", version = "0.2", default-features = false } tracing-core = { path = "../tracing-core", version = "0.2", default-features = false } +tracing-subscriber = { path = "../tracing-subscriber", version = "0.3", default-features = false, optional = true } tokio-test = { version = "0.4.2", optional = true } # Fix minimal-versions; tokio-test fails with otherwise acceptable 0.1.0 diff --git a/tracing-mock/src/collector.rs b/tracing-mock/src/collector.rs index 9c61f2c8b5..f60918f7d0 100644 --- a/tracing-mock/src/collector.rs +++ b/tracing-mock/src/collector.rs @@ -1,6 +1,7 @@ #![allow(missing_docs)] -use super::{ +use crate::{ event::MockEvent, + expectation::Expect, field as mock_field, span::{MockSpan, NewSpan}, }; @@ -20,22 +21,6 @@ use tracing::{ Collect, Event, Metadata, }; -#[derive(Debug, Eq, PartialEq)] -pub enum Expect { - Event(MockEvent), - FollowsFrom { - consequence: MockSpan, - cause: MockSpan, - }, - Enter(MockSpan), - Exit(MockSpan), - CloneSpan(MockSpan), - DropSpan(MockSpan), - Visit(MockSpan, mock_field::Expect), - NewSpan(NewSpan), - Nothing, -} - struct SpanState { name: &'static str, refs: usize, @@ -469,7 +454,7 @@ where } impl MockHandle { - pub fn new(expected: Arc>>, name: String) -> Self { + pub(crate) fn new(expected: Arc>>, name: String) -> Self { Self(expected, name) } diff --git a/tracing-mock/src/expectation.rs b/tracing-mock/src/expectation.rs new file mode 100644 index 0000000000..0328754fc8 --- /dev/null +++ b/tracing-mock/src/expectation.rs @@ -0,0 +1,21 @@ +use crate::{ + event::MockEvent, + field, + span::{MockSpan, NewSpan}, +}; + +#[derive(Debug, Eq, PartialEq)] +pub(crate) enum Expect { + Event(MockEvent), + FollowsFrom { + consequence: MockSpan, + cause: MockSpan, + }, + Enter(MockSpan), + Exit(MockSpan), + CloneSpan(MockSpan), + DropSpan(MockSpan), + Visit(MockSpan, field::Expect), + NewSpan(NewSpan), + Nothing, +} diff --git a/tracing-mock/src/lib.rs b/tracing-mock/src/lib.rs index eefc409ea0..49a2375748 100644 --- a/tracing-mock/src/lib.rs +++ b/tracing-mock/src/lib.rs @@ -5,10 +5,15 @@ use std::{ pub mod collector; pub mod event; +mod expectation; pub mod field; mod metadata; pub mod span; +#[cfg(feature = "subscriber")] +#[doc(hidden)] +pub mod subscriber; + #[derive(Debug, Eq, PartialEq)] pub enum Parent { ContextualRoot, diff --git a/tracing-subscriber/tests/support.rs b/tracing-mock/src/subscriber.rs similarity index 96% rename from tracing-subscriber/tests/support.rs rename to tracing-mock/src/subscriber.rs index fab742b68c..d744cc6a02 100644 --- a/tracing-subscriber/tests/support.rs +++ b/tracing-mock/src/subscriber.rs @@ -1,9 +1,9 @@ #![allow(missing_docs, dead_code)] -pub use tracing_mock::{collector, event, field, span}; - -use self::{ - collector::{Expect, MockHandle}, +use crate::{ + collector::MockHandle, event::MockEvent, + expectation::Expect, + field, span::{MockSpan, NewSpan}, }; use tracing_core::{ @@ -21,22 +21,18 @@ use std::{ sync::{Arc, Mutex}, }; -pub mod subscriber { - use super::ExpectSubscriberBuilder; - - pub fn mock() -> ExpectSubscriberBuilder { - ExpectSubscriberBuilder { - expected: Default::default(), - name: std::thread::current() - .name() - .map(String::from) - .unwrap_or_default(), - } +pub fn mock() -> ExpectSubscriberBuilder { + ExpectSubscriberBuilder { + expected: Default::default(), + name: std::thread::current() + .name() + .map(String::from) + .unwrap_or_default(), } +} - pub fn named(name: impl std::fmt::Display) -> ExpectSubscriberBuilder { - mock().named(name) - } +pub fn named(name: impl std::fmt::Display) -> ExpectSubscriberBuilder { + mock().named(name) } pub struct ExpectSubscriberBuilder { diff --git a/tracing-subscriber/Cargo.toml b/tracing-subscriber/Cargo.toml index de90c43000..c2febbe484 100644 --- a/tracing-subscriber/Cargo.toml +++ b/tracing-subscriber/Cargo.toml @@ -65,7 +65,7 @@ thread_local = { version = "1.1.4", optional = true } [dev-dependencies] tracing = { path = "../tracing", version = "0.2" } -tracing-mock = { path = "../tracing-mock" } +tracing-mock = { path = "../tracing-mock", features = ["subscriber"] } log = "0.4.17" tracing-log = { path = "../tracing-log", version = "0.2" } criterion = { version = "0.3.6", default_features = false } diff --git a/tracing-subscriber/tests/cached_subscriber_filters_dont_break_other_subscribers.rs b/tracing-subscriber/tests/cached_subscriber_filters_dont_break_other_subscribers.rs index 3c538fa6f4..dcb32d119b 100644 --- a/tracing-subscriber/tests/cached_subscriber_filters_dont_break_other_subscribers.rs +++ b/tracing-subscriber/tests/cached_subscriber_filters_dont_break_other_subscribers.rs @@ -1,7 +1,9 @@ #![cfg(feature = "registry")] -mod support; -use self::support::*; use tracing::Level; +use tracing_mock::{ + collector, event, + subscriber::{self, ExpectSubscriber}, +}; use tracing_subscriber::{filter::LevelFilter, prelude::*}; #[test] diff --git a/tracing-subscriber/tests/env_filter/main.rs b/tracing-subscriber/tests/env_filter/main.rs index d47c57e17e..2246d92ebd 100644 --- a/tracing-subscriber/tests/env_filter/main.rs +++ b/tracing-subscriber/tests/env_filter/main.rs @@ -1,12 +1,9 @@ #![cfg(feature = "env-filter")] -#[path = "../support.rs"] -mod support; -use self::support::*; - mod per_subscriber; use tracing::{self, collect::with_default, Level}; +use tracing_mock::{collector, event, field, span}; use tracing_subscriber::{ filter::{EnvFilter, LevelFilter}, prelude::*, diff --git a/tracing-subscriber/tests/env_filter/per_subscriber.rs b/tracing-subscriber/tests/env_filter/per_subscriber.rs index 65ac1a195b..6110dd632c 100644 --- a/tracing-subscriber/tests/env_filter/per_subscriber.rs +++ b/tracing-subscriber/tests/env_filter/per_subscriber.rs @@ -2,6 +2,7 @@ //! `subscriber` filter). #![cfg(feature = "registry")] use super::*; +use tracing_mock::{event, field, span, subscriber}; #[test] fn level_filter_event() { diff --git a/tracing-subscriber/tests/hinted_subscriber_filters_dont_break_other_subscribers.rs b/tracing-subscriber/tests/hinted_subscriber_filters_dont_break_other_subscribers.rs index e1c3c849eb..3a0b9e30d2 100644 --- a/tracing-subscriber/tests/hinted_subscriber_filters_dont_break_other_subscribers.rs +++ b/tracing-subscriber/tests/hinted_subscriber_filters_dont_break_other_subscribers.rs @@ -1,7 +1,9 @@ #![cfg(feature = "registry")] -mod support; -use self::support::*; use tracing::{Collect, Level, Metadata}; +use tracing_mock::{ + collector, event, + subscriber::{self, ExpectSubscriber}, +}; use tracing_subscriber::{filter::DynFilterFn, prelude::*, subscribe::Context}; #[test] diff --git a/tracing-subscriber/tests/multiple_subscriber_filter_interests_cached.rs b/tracing-subscriber/tests/multiple_subscriber_filter_interests_cached.rs index 8088fbb8d6..b7660176e9 100644 --- a/tracing-subscriber/tests/multiple_subscriber_filter_interests_cached.rs +++ b/tracing-subscriber/tests/multiple_subscriber_filter_interests_cached.rs @@ -1,12 +1,10 @@ #![cfg(feature = "registry")] -mod support; -use self::support::*; - use std::{ collections::HashMap, sync::{Arc, Mutex}, }; use tracing::{Collect, Level}; +use tracing_mock::{event, subscriber}; use tracing_subscriber::{filter, prelude::*}; #[test] diff --git a/tracing-subscriber/tests/subscriber_filter_interests_are_cached.rs b/tracing-subscriber/tests/subscriber_filter_interests_are_cached.rs index 647d170b1e..88ea93d5c5 100644 --- a/tracing-subscriber/tests/subscriber_filter_interests_are_cached.rs +++ b/tracing-subscriber/tests/subscriber_filter_interests_are_cached.rs @@ -1,12 +1,10 @@ #![cfg(feature = "registry")] -mod support; -use self::support::*; - use std::{ collections::HashMap, sync::{Arc, Mutex}, }; use tracing::{Collect, Level}; +use tracing_mock::{event, subscriber}; use tracing_subscriber::{filter, prelude::*}; #[test] diff --git a/tracing-subscriber/tests/subscriber_filters/filter_scopes.rs b/tracing-subscriber/tests/subscriber_filters/filter_scopes.rs index 710d267b86..0de398b2e8 100644 --- a/tracing-subscriber/tests/subscriber_filters/filter_scopes.rs +++ b/tracing-subscriber/tests/subscriber_filters/filter_scopes.rs @@ -1,4 +1,5 @@ use super::*; +use tracing_mock::subscriber::ExpectSubscriber; #[test] fn filters_span_scopes() { diff --git a/tracing-subscriber/tests/subscriber_filters/main.rs b/tracing-subscriber/tests/subscriber_filters/main.rs index 2b80e3bc57..897175cfc8 100644 --- a/tracing-subscriber/tests/subscriber_filters/main.rs +++ b/tracing-subscriber/tests/subscriber_filters/main.rs @@ -1,7 +1,4 @@ #![cfg(feature = "registry")] -#[path = "../support.rs"] -mod support; -use self::support::*; mod filter_scopes; mod per_event; mod targets; @@ -9,6 +6,7 @@ mod trees; mod vec; use tracing::{level_filters::LevelFilter, Level}; +use tracing_mock::{collector, event, span, subscriber}; use tracing_subscriber::{filter, prelude::*, Subscribe}; #[test] diff --git a/tracing-subscriber/tests/subscriber_filters/per_event.rs b/tracing-subscriber/tests/subscriber_filters/per_event.rs index f5b90203c9..f4d3febc09 100644 --- a/tracing-subscriber/tests/subscriber_filters/per_event.rs +++ b/tracing-subscriber/tests/subscriber_filters/per_event.rs @@ -1,5 +1,5 @@ -use crate::support::*; use tracing::Level; +use tracing_mock::{event, subscriber}; use tracing_subscriber::{field::Visit, prelude::*, subscribe::Filter}; struct FilterEvent; diff --git a/tracing-subscriber/tests/subscriber_filters/trees.rs b/tracing-subscriber/tests/subscriber_filters/trees.rs index 3332045e1a..3227e69a5e 100644 --- a/tracing-subscriber/tests/subscriber_filters/trees.rs +++ b/tracing-subscriber/tests/subscriber_filters/trees.rs @@ -1,4 +1,5 @@ use super::*; +use tracing_mock::subscriber::ExpectSubscriber; #[test] fn basic_trees() { diff --git a/tracing-subscriber/tests/subscriber_filters/vec.rs b/tracing-subscriber/tests/subscriber_filters/vec.rs index 4823a5e4a0..b10ecf8ab9 100644 --- a/tracing-subscriber/tests/subscriber_filters/vec.rs +++ b/tracing-subscriber/tests/subscriber_filters/vec.rs @@ -1,5 +1,6 @@ use super::*; use tracing::Collect; +use tracing_mock::subscriber::ExpectSubscriber; #[test] fn with_filters_unboxed() { diff --git a/tracing-subscriber/tests/unhinted_subscriber_filters_dont_break_other_subscribers.rs b/tracing-subscriber/tests/unhinted_subscriber_filters_dont_break_other_subscribers.rs index 725bd4cde4..87fc803e65 100644 --- a/tracing-subscriber/tests/unhinted_subscriber_filters_dont_break_other_subscribers.rs +++ b/tracing-subscriber/tests/unhinted_subscriber_filters_dont_break_other_subscribers.rs @@ -1,7 +1,9 @@ #![cfg(feature = "registry")] -mod support; -use self::support::*; use tracing::Level; +use tracing_mock::{ + collector, event, + subscriber::{self, ExpectSubscriber}, +}; use tracing_subscriber::{filter::DynFilterFn, prelude::*}; #[test] diff --git a/tracing-subscriber/tests/vec_subscriber_filter_interests_cached.rs b/tracing-subscriber/tests/vec_subscriber_filter_interests_cached.rs index bf8525a38b..6e6b01e75e 100644 --- a/tracing-subscriber/tests/vec_subscriber_filter_interests_cached.rs +++ b/tracing-subscriber/tests/vec_subscriber_filter_interests_cached.rs @@ -1,12 +1,13 @@ #![cfg(feature = "registry")] -mod support; -use self::support::*; - use std::{ collections::HashMap, sync::{Arc, Mutex}, }; use tracing::{Collect, Level}; +use tracing_mock::{ + event, + subscriber::{self, ExpectSubscriber}, +}; use tracing_subscriber::{filter, prelude::*}; #[test] From 334b50d60d4223e0dc545f198bf09096f045e0f2 Mon Sep 17 00:00:00 2001 From: Hayden Stainsby Date: Tue, 8 Nov 2022 14:04:14 +0100 Subject: [PATCH 2/2] remove separate subscriber feature from tracing-mock The subscriber feature is included directly by using the `tracing-subscriber` feature (for the optional depedency). --- tracing-mock/Cargo.toml | 4 ---- tracing-mock/src/lib.rs | 3 +-- tracing-subscriber/Cargo.toml | 2 +- 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/tracing-mock/Cargo.toml b/tracing-mock/Cargo.toml index a9bf030a29..eb4bbe0fcf 100644 --- a/tracing-mock/Cargo.toml +++ b/tracing-mock/Cargo.toml @@ -17,10 +17,6 @@ edition = "2018" rust-version = "1.49.0" publish = false -[features] -default = [] -subscriber = ["tracing-subscriber"] - [dependencies] tracing = { path = "../tracing", version = "0.2", default-features = false } tracing-core = { path = "../tracing-core", version = "0.2", default-features = false } diff --git a/tracing-mock/src/lib.rs b/tracing-mock/src/lib.rs index 49a2375748..dc3803bba6 100644 --- a/tracing-mock/src/lib.rs +++ b/tracing-mock/src/lib.rs @@ -10,8 +10,7 @@ pub mod field; mod metadata; pub mod span; -#[cfg(feature = "subscriber")] -#[doc(hidden)] +#[cfg(feature = "tracing-subscriber")] pub mod subscriber; #[derive(Debug, Eq, PartialEq)] diff --git a/tracing-subscriber/Cargo.toml b/tracing-subscriber/Cargo.toml index c2febbe484..a93d231dd1 100644 --- a/tracing-subscriber/Cargo.toml +++ b/tracing-subscriber/Cargo.toml @@ -65,7 +65,7 @@ thread_local = { version = "1.1.4", optional = true } [dev-dependencies] tracing = { path = "../tracing", version = "0.2" } -tracing-mock = { path = "../tracing-mock", features = ["subscriber"] } +tracing-mock = { path = "../tracing-mock", features = ["tracing-subscriber"] } log = "0.4.17" tracing-log = { path = "../tracing-log", version = "0.2" } criterion = { version = "0.3.6", default_features = false }