mock: move subscriber mock from tracing-subscriber tests #2369

hds · 2022-11-04T22:36:17Z

Motivation

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.

Solution

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

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

hawkw

overall, this is great --- it's definitely much nicer to have everything all in the same place! i commented on a couple very small nits.

tracing-mock/Cargo.toml

tracing-mock/src/lib.rs

The subscriber feature is included directly by using the `tracing-subscriber` feature (for the optional depedency).

hawkw

looks good to me, thanks!

hawkw · 2022-11-08T18:10:42Z

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")]


we should probably also globally enable the doc_cfg feature flag for this crate so that the feature flags show up in RustDoc, but we can do that separately when we actually...add documentation...before releasing.

There has been interest around publishing `tracing-mock` to crates.io for some time. In order to make this possible, documentation and some code clean up is needed. This change adds documentation to the collector module itself and to all the public APIs in the module. This includes doctests on all the methods that serve as examples. Additionally the implementation for the `Expect` struct has been moved into the module with the definition, this was missed in #2369.

There has been interest around publishing `tracing-mock` to crates.io for some time. In order to make this possible, documentation and some code clean up is needed. This change adds documentation to the collector module itself and to all the public APIs in the module. This includes doctests on all the methods that serve as examples. Additionally the implementation for the `Expect` struct has been moved into the module with the definition, this was missed in #2369. Refs: #539

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

@hds

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

@hds

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

There has been interest around publishing `tracing-mock` to crates.io for some time. In order to make this possible, documentation and some code clean up is needed. This change adds documentation to the collector module itself and to all the public APIs in the module. This includes doctests on all the methods that serve as examples. Additionally the implementation for the `Expect` struct has been moved into the module with the definition, this was missed in #2369. Refs: #539

@hds

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 tokio-rs#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: tokio-rs#2359

There has been interest around publishing `tracing-mock` to crates.io for some time. In order to make this possible, documentation and some code clean up is needed. This change adds documentation to the collector module itself and to all the public APIs in the module. This includes doctests on all the methods that serve as examples. Additionally the implementation for the `Expect` struct has been moved into the module with the definition, this was missed in tokio-rs#2369. Refs: tokio-rs#539

hds requested review from hawkw, davidbarsky and a team as code owners November 4, 2022 22:36

hawkw reviewed Nov 7, 2022

View reviewed changes

tracing-mock/Cargo.toml Outdated Show resolved Hide resolved

tracing-mock/src/lib.rs Outdated Show resolved Hide resolved

remove separate subscriber feature from tracing-mock

334b50d

The subscriber feature is included directly by using the `tracing-subscriber` feature (for the optional depedency).

hds requested a review from hawkw November 8, 2022 14:35

hawkw approved these changes Nov 8, 2022

View reviewed changes

hawkw merged commit b86ea89 into master Nov 8, 2022

hawkw deleted the hds/mock-include-subscriber branch November 8, 2022 18:14

hds mentioned this pull request Nov 16, 2022

mock: document public API in collector module #2389

Merged

hawkw mentioned this pull request Apr 21, 2023

backport a gigantic pile of stuff #2570

Merged

davidbarsky mentioned this pull request Sep 26, 2023

chore: backport roughly a year's worth of changes #2728

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mock: move subscriber mock from tracing-subscriber tests #2369

mock: move subscriber mock from tracing-subscriber tests #2369

hds commented Nov 4, 2022

hawkw left a comment

hawkw left a comment

hawkw Nov 8, 2022

mock: move subscriber mock from tracing-subscriber tests #2369

mock: move subscriber mock from tracing-subscriber tests #2369

Conversation

hds commented Nov 4, 2022

Motivation

Solution

hawkw left a comment

Choose a reason for hiding this comment

hawkw left a comment

Choose a reason for hiding this comment

hawkw Nov 8, 2022

Choose a reason for hiding this comment