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

Move tracing-subscriber test/support module into tracing-mock #2359

Closed
hds opened this issue Oct 28, 2022 · 0 comments · Fixed by #2369
Closed

Move tracing-subscriber test/support module into tracing-mock #2359

hds opened this issue Oct 28, 2022 · 0 comments · Fixed by #2369

Comments

@hds
Copy link
Contributor

hds commented Oct 28, 2022

Feature Request

Refactor tracing-subscriber::tests::support and tracing-mock so that some items currently defined as public in tracing-mock can be made not-public (specifically pub(crate)).

Crates

  • tracing-subscriber
  • tracing-mock

Motivation

While documenting tracing-mock, I found a couple of things in the collector module which are public, but don't need to be except that they're used in the module tracing-subscriber::tests::support.

In the context of #539, this is potentially exposing parts of tracing-mock that it would be better not to expose. For example the num collector::Expect is currenlty public, leaving it this way would make adding new expectations a breaking change.

Proposal

Move everything needed by tracing-subscriber tests (that uses potential tracing-mock internals) into tracing-mock itself and put it behind a feature flag.

Alternatives

Another solution would be to duplicate the required parts of tracing-mock in the support module - which I think is how it was set up before.

Or these public items could be reworked to ensure forward comaptibility, for example by adding #[non_exhaustive] to the previously mentioned Expect enum.

hds added a commit that referenced this issue Nov 4, 2022
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 pushed a commit that referenced this issue Nov 8, 2022
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 added a commit that referenced this issue Apr 21, 2023
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 added a commit that referenced this issue Apr 21, 2023
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
hawkw added a commit that referenced this issue Apr 21, 2023
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
kaffarell pushed a commit to kaffarell/tracing that referenced this issue May 22, 2024
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant