mock: move layer mock from tracing-subscriber tests (#2369)

The `tracing-subscriber` module `tests::support` included functionality
to mock a layer (via the `Layer` 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`).

This PR is based on @hds' PR #2369, but modified to track renamings. I
also deleted all the doc comments temporarily because updating them was
a lot of work and I need to get a release of `tracing-subscriber` out
first.

Closes: #2359

Author: hawkw

tracing-mock/Cargo.toml

@@ -20,6 +20,7 @@ publish = false
 [dependencies]
 tracing = { path = "../tracing", version = "0.1.35", default-features = false }
 tracing-core = { path = "../tracing-core", version = "0.1.28", 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

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,
+}

tracing-subscriber/tests/support.rs renamed to tracing-mock/src/layer.rs

@@ -1,15 +1,13 @@
-#![allow(missing_docs, dead_code)]
-pub use tracing_mock::{event, field, span, subscriber};
-
+use crate::{
+    event::MockEvent,
+    expectation::Expect,
+    span::{MockSpan, NewSpan},
+    subscriber::MockHandle,
+};
 use tracing_core::{
     span::{Attributes, Id, Record},
     Event, Subscriber,
 };
-use tracing_mock::{
-    event::MockEvent,
-    span::{MockSpan, NewSpan},
-    subscriber::{Expect, MockHandle},
-};
 use tracing_subscriber::{
     layer::{Context, Layer},
     registry::{LookupSpan, SpanRef},
@@ -21,49 +19,34 @@ use std::{
     sync::{Arc, Mutex},
 };
 
-pub mod layer {
-    use super::ExpectLayerBuilder;
-
-    pub fn mock() -> ExpectLayerBuilder {
-        ExpectLayerBuilder {
-            expected: Default::default(),
-            name: std::thread::current()
-                .name()
-                .map(String::from)
-                .unwrap_or_default(),
-        }
+#[must_use]
+pub fn mock() -> MockLayerBuilder {
+    MockLayerBuilder {
+        expected: Default::default(),
+        name: std::thread::current()
+            .name()
+            .map(String::from)
+            .unwrap_or_default(),
     }
+}
 
-    pub fn named(name: impl std::fmt::Display) -> ExpectLayerBuilder {
-        mock().named(name)
-    }
+#[must_use]
+pub fn named(name: impl std::fmt::Display) -> MockLayerBuilder {
+    mock().named(name)
 }
 
-pub struct ExpectLayerBuilder {
+pub struct MockLayerBuilder {
     expected: VecDeque<Expect>,
     name: String,
 }
 
-pub struct ExpectLayer {
+pub struct MockLayer {
     expected: Arc<Mutex<VecDeque<Expect>>>,
     current: Mutex<Vec<Id>>,
     name: String,
 }
 
-impl ExpectLayerBuilder {
-    /// Overrides the name printed by the mock subscriber's debugging output.
-    ///
-    /// The debugging output is displayed if the test panics, or if the test is
-    /// run with `--nocapture`.
-    ///
-    /// By default, the mock subscriber's name is the  name of the test
-    /// (*technically*, the name of the thread where it was created, which is
-    /// the name of the test unless tests are run with `--test-threads=1`).
-    /// When a test has only one mock subscriber, this is sufficient. However,
-    /// some tests may include multiple subscribers, in order to test
-    /// interactions between multiple subscribers. In that case, it can be
-    /// helpful to give each subscriber a separate name to distinguish where the
-    /// debugging output comes from.
+impl MockLayerBuilder {
     pub fn named(mut self, name: impl fmt::Display) -> Self {
         use std::fmt::Write;
         if !self.name.is_empty() {
@@ -74,63 +57,55 @@ impl ExpectLayerBuilder {
         self
     }
 
-    pub fn enter(mut self, span: MockSpan) -> Self {
-        self.expected.push_back(Expect::Enter(span));
-        self
-    }
-
     pub fn event(mut self, event: MockEvent) -> Self {
         self.expected.push_back(Expect::Event(event));
         self
     }
 
-    pub fn exit(mut self, span: MockSpan) -> Self {
-        self.expected.push_back(Expect::Exit(span));
+    pub fn new_span<I>(mut self, new_span: I) -> Self
+    where
+        I: Into<NewSpan>,
+    {
+        self.expected.push_back(Expect::NewSpan(new_span.into()));
         self
     }
 
-    pub fn done(mut self) -> Self {
-        self.expected.push_back(Expect::Nothing);
+    pub fn enter(mut self, span: MockSpan) -> Self {
+        self.expected.push_back(Expect::Enter(span));
         self
     }
 
-    pub fn record<I>(mut self, span: MockSpan, fields: I) -> Self
-    where
-        I: Into<field::Expect>,
-    {
-        self.expected.push_back(Expect::Visit(span, fields.into()));
+    pub fn exit(mut self, span: MockSpan) -> Self {
+        self.expected.push_back(Expect::Exit(span));
         self
     }
 
-    pub fn new_span<I>(mut self, new_span: I) -> Self
-    where
-        I: Into<NewSpan>,
-    {
-        self.expected.push_back(Expect::NewSpan(new_span.into()));
+    pub fn done(mut self) -> Self {
+        self.expected.push_back(Expect::Nothing);
         self
     }
 
-    pub fn run(self) -> ExpectLayer {
-        ExpectLayer {
+    pub fn run(self) -> MockLayer {
+        MockLayer {
             expected: Arc::new(Mutex::new(self.expected)),
             name: self.name,
             current: Mutex::new(Vec::new()),
         }
     }
 
-    pub fn run_with_handle(self) -> (ExpectLayer, MockHandle) {
+    pub fn run_with_handle(self) -> (MockLayer, MockHandle) {
         let expected = Arc::new(Mutex::new(self.expected));
         let handle = MockHandle::new(expected.clone(), self.name.clone());
-        let layer = ExpectLayer {
+        let subscriber = MockLayer {
             expected,
             name: self.name,
             current: Mutex::new(Vec::new()),
         };
-        (layer, handle)
+        (subscriber, handle)
     }
 }
 
-impl ExpectLayer {
+impl MockLayer {
     fn check_span_ref<'spans, S>(
         &self,
         expected: &MockSpan,
@@ -191,9 +166,9 @@ impl ExpectLayer {
     }
 }
 
-impl<S> Layer<S> for ExpectLayer
+impl<C> Layer<C> for MockLayer
 where
-    S: Subscriber + for<'a> LookupSpan<'a>,
+    C: Subscriber + for<'a> LookupSpan<'a>,
 {
     fn register_callsite(
         &self,
@@ -203,15 +178,15 @@ where
         tracing_core::Interest::always()
     }
 
-    fn on_record(&self, _: &Id, _: &Record<'_>, _: Context<'_, S>) {
+    fn on_record(&self, _: &Id, _: &Record<'_>, _: Context<'_, C>) {
         unimplemented!(
             "so far, we don't have any tests that need an `on_record` \
             implementation.\nif you just wrote one that does, feel free to \
             implement it!"
         );
     }
 
-    fn on_event(&self, event: &Event<'_>, cx: Context<'_, S>) {
+    fn on_event(&self, event: &Event<'_>, cx: Context<'_, C>) {
         let name = event.metadata().name();
         println!(
             "[{}] event: {}; level: {}; target: {}",
@@ -262,11 +237,15 @@ where
         }
     }
 
-    fn on_follows_from(&self, _span: &Id, _follows: &Id, _: Context<'_, S>) {
-        // TODO: it should be possible to expect spans to follow from other spans
+    fn on_follows_from(&self, _span: &Id, _follows: &Id, _: Context<'_, C>) {
+        unimplemented!(
+            "so far, we don't have any tests that need an `on_follows_from` \
+            implementation.\nif you just wrote one that does, feel free to \
+            implement it!"
+        );
     }
 
-    fn on_new_span(&self, span: &Attributes<'_>, id: &Id, cx: Context<'_, S>) {
+    fn on_new_span(&self, span: &Attributes<'_>, id: &Id, cx: Context<'_, C>) {
         let meta = span.metadata();
         println!(
             "[{}] new_span: name={:?}; target={:?}; id={:?};",
@@ -290,7 +269,7 @@ where
         }
     }
 
-    fn on_enter(&self, id: &Id, cx: Context<'_, S>) {
+    fn on_enter(&self, id: &Id, cx: Context<'_, C>) {
         let span = cx
             .span(id)
             .unwrap_or_else(|| panic!("[{}] no span for ID {:?}", self.name, id));
@@ -305,7 +284,7 @@ where
         self.current.lock().unwrap().push(id.clone());
     }
 
-    fn on_exit(&self, id: &Id, cx: Context<'_, S>) {
+    fn on_exit(&self, id: &Id, cx: Context<'_, C>) {
         if std::thread::panicking() {
             // `exit()` can be called in `drop` impls, so we must guard against
             // double panics.
@@ -334,7 +313,7 @@ where
         };
     }
 
-    fn on_close(&self, id: Id, cx: Context<'_, S>) {
+    fn on_close(&self, id: Id, cx: Context<'_, C>) {
         if std::thread::panicking() {
             // `try_close` can be called in `drop` impls, so we must guard against
             // double panics.
@@ -380,14 +359,14 @@ where
         }
     }
 
-    fn on_id_change(&self, _old: &Id, _new: &Id, _ctx: Context<'_, S>) {
+    fn on_id_change(&self, _old: &Id, _new: &Id, _ctx: Context<'_, C>) {
         panic!("well-behaved subscribers should never do this to us, lol");
     }
 }
 
-impl fmt::Debug for ExpectLayer {
+impl fmt::Debug for MockLayer {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let mut s = f.debug_struct("ExpectLayer");
+        let mut s = f.debug_struct("MockLayer");
         s.field("name", &self.name);
 
         if let Ok(expected) = self.expected.try_lock() {

tracing-mock/src/lib.rs

@@ -4,11 +4,15 @@ use std::{
 };
 
 pub mod event;
+mod expectation;
 pub mod field;
 mod metadata;
 pub mod span;
 pub mod subscriber;
 
+#[cfg(feature = "tracing-subscriber")]
+pub mod layer;
+
 #[derive(Debug, Eq, PartialEq)]
 pub enum Parent {
     ContextualRoot,

tracing-mock/src/subscriber.rs

@@ -1,6 +1,7 @@
 #![allow(missing_docs)]
 use super::{
     event::MockEvent,
+    expectation::Expect,
     field as mock_field,
     span::{MockSpan, NewSpan},
 };
@@ -20,22 +21,6 @@ use tracing::{
     Event, Metadata, Subscriber,
 };
 
-#[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,
@@ -471,7 +456,7 @@ where
 }
 
 impl MockHandle {
-    pub fn new(expected: Arc<Mutex<VecDeque<Expect>>>, name: String) -> Self {
+    pub(crate) fn new(expected: Arc<Mutex<VecDeque<Expect>>>, name: String) -> Self {
         Self(expected, name)
     }
 
@@ -488,7 +473,7 @@ impl MockHandle {
 }
 
 impl Expect {
-    pub fn bad(&self, name: impl AsRef<str>, what: fmt::Arguments<'_>) {
+    pub(crate) fn bad(&self, name: impl AsRef<str>, what: fmt::Arguments<'_>) {
         let name = name.as_ref();
         match self {
             Expect::Event(e) => panic!(

tracing-subscriber/Cargo.toml

@@ -70,7 +70,7 @@ valuable-serde = { version = "0.1.0", optional = true, default-features = false
 
 [dev-dependencies]
 tracing = { path = "../tracing", version = "0.1.35" }
-tracing-mock = { path = "../tracing-mock" }
+tracing-mock = { path = "../tracing-mock", features = ["tracing-subscriber"] }
 log = "0.4.17"
 tracing-log = { path = "../tracing-log", version = "0.1.3" }
 criterion = { version = "0.3.6", default-features = false }

tracing-subscriber/tests/cached_layer_filters_dont_break_other_layers.rs

@@ -1,7 +1,10 @@
 #![cfg(feature = "registry")]
-mod support;
-use self::support::*;
 use tracing::Level;
+use tracing_mock::{
+    event,
+    layer::{self, MockLayer},
+    subscriber,
+};
 use tracing_subscriber::{filter::LevelFilter, prelude::*};
 
 #[test]
@@ -102,7 +105,7 @@ fn filter() -> LevelFilter {
     LevelFilter::INFO
 }
 
-fn unfiltered(name: &str) -> (ExpectLayer, subscriber::MockHandle) {
+fn unfiltered(name: &str) -> (MockLayer, subscriber::MockHandle) {
     layer::named(name)
         .event(event::mock().at_level(Level::TRACE))
         .event(event::mock().at_level(Level::DEBUG))
@@ -113,7 +116,7 @@ fn unfiltered(name: &str) -> (ExpectLayer, subscriber::MockHandle) {
         .run_with_handle()
 }
 
-fn filtered(name: &str) -> (ExpectLayer, subscriber::MockHandle) {
+fn filtered(name: &str) -> (MockLayer, subscriber::MockHandle) {
     layer::named(name)
         .event(event::mock().at_level(Level::INFO))
         .event(event::mock().at_level(Level::WARN))