Skip to content

Commit

Permalink
mock: improve ergonomics when an ExpectedSpan is needed
Browse files Browse the repository at this point in the history
Many of the methods on `MockCollector` take an `ExpectedSpan`. This
often requires significant boilerplate. For example, to expect that a
span with a specific name enters and then exits, the following code is
needed:

```rust
let span = expect::span().named("span name");

let (collector, handle) = collector::mock()
    .enter(span.clone())
    .exit(span)
    .run_with_handle();
```

In order to make using `tracing-mock` more ergonomic and also more
compact, the `MockCollector` and `MockSubscriber` methods that previous
took an `ExpectedSpan`, are now generic over `Into<ExpectedSpan>`.

There are currently 3 implementations of `From` for `ExpectedSpan` which
allow the following shorthand uses:

`T: Into<String>` - an `ExpectedSpan` will be created that expects to
have a name specified by `T`.

```rust
let (collector, handle) = collector::mock()
    .enter("span name")
    .exit("span name")
    .run_with_handle();
```

`&ExpectedId` - an `ExpectedSpan` will be created that expects to have
the expected Id. A reference is taken and cloned internally because the
caller always needs to use an `ExpectedId` in at least 2 calls to the
mock collector/subscriber.

```rust
let id = expect::id();

let (collector, handle) = collector::mock()
    .new_span(&id)
    .enter(&id)
    .run_with_handle();
```

`&ExpectedSpan` - The expected span is taken by reference and cloned.

```rust
let span = expect::span().named("span name");

let (collector, handle) = collector::mock()
    .enter(&span)
    .exit(&span)
    .run_with_handle();
```

In Rust, taking a reference to an object and immediately cloning it is
an anti-pattern. It is considered better to force the user to clone
outside the API to make the cloning explict.

However, in the case of a testing framework, it seems reasonable to
prefer a more concise API, rather than having it more explicit.

To reduce the size of this PR and to avoid unnecessary churn in other
crates, the tests within the tracing repo which use `tracing-mock` will
not be updated to use the new `Into<ExpectedSpan>` capabilities. The new
API is backwards compatible and those tests can remain as they are.
  • Loading branch information
hds committed Oct 4, 2024
1 parent 2d30094 commit 5daa282
Show file tree
Hide file tree
Showing 3 changed files with 188 additions and 68 deletions.
74 changes: 47 additions & 27 deletions tracing-mock/src/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@
//! .named("my_span");
//! let (collector, handle) = collector::mock()
//! // Enter a matching span
//! .enter(span.clone())
//! .enter(&span)
//! // Record an event with message "collect parting message"
//! .event(expect::event().with_fields(expect::message("collect parting message")))
//! // Record a value for the field `parting` on a matching span
//! .record(span.clone(), expect::field("parting").with_value(&"goodbye world!"))
//! .record(&span, expect::field("parting").with_value(&"goodbye world!"))
//! // Exit a matching span
//! .exit(span)
//! // Expect no further messages to be recorded
Expand Down Expand Up @@ -80,9 +80,9 @@
//! let span = expect::span()
//! .named("my_span");
//! let (collector, handle) = collector::mock()
//! .enter(span.clone())
//! .enter(&span)
//! .event(expect::event().with_fields(expect::message("collect parting message")))
//! .record(span.clone(), expect::field("parting").with_value(&"goodbye world!"))
//! .record(&span, expect::field("parting").with_value(&"goodbye world!"))
//! .exit(span)
//! .only()
//! .run_with_handle();
Expand Down Expand Up @@ -225,11 +225,11 @@ pub struct MockHandle(Arc<Mutex<VecDeque<Expect>>>, String);
/// .named("my_span");
/// let (collector, handle) = collector::mock()
/// // Enter a matching span
/// .enter(span.clone())
/// .enter(&span)
/// // Record an event with message "collect parting message"
/// .event(expect::event().with_fields(expect::message("collect parting message")))
/// // Record a value for the field `parting` on a matching span
/// .record(span.clone(), expect::field("parting").with_value(&"goodbye world!"))
/// .record(&span, expect::field("parting").with_value(&"goodbye world!"))
/// // Exit a matching span
/// .exit(span)
/// // Expect no further messages to be recorded
Expand Down Expand Up @@ -472,8 +472,8 @@ where
/// .at_level(tracing::Level::INFO)
/// .named("the span we're testing");
/// let (collector, handle) = collector::mock()
/// .enter(span.clone())
/// .exit(span)
/// .enter(&span)
/// .exit(&span)
/// .only()
/// .run_with_handle();
///
Expand All @@ -495,8 +495,8 @@ where
/// .at_level(tracing::Level::INFO)
/// .named("the span we're testing");
/// let (collector, handle) = collector::mock()
/// .enter(span.clone())
/// .exit(span)
/// .enter(&span)
/// .exit(&span)
/// .only()
/// .run_with_handle();
///
Expand All @@ -511,8 +511,11 @@ where
///
/// [`exit`]: fn@Self::exit
/// [`only`]: fn@Self::only
pub fn enter(mut self, span: ExpectedSpan) -> Self {
self.expected.push_back(Expect::Enter(span));
pub fn enter<S>(mut self, span: S) -> Self
where
S: Into<ExpectedSpan>,
{
self.expected.push_back(Expect::Enter(span.into()));
self
}

Expand All @@ -536,8 +539,8 @@ where
/// .at_level(tracing::Level::INFO)
/// .named("the span we're testing");
/// let (collector, handle) = collector::mock()
/// .enter(span.clone())
/// .exit(span)
/// .enter(&span)
/// .exit(&span)
/// .run_with_handle();
///
/// tracing::collect::with_default(collector, || {
Expand All @@ -558,8 +561,8 @@ where
/// .at_level(tracing::Level::INFO)
/// .named("the span we're testing");
/// let (collector, handle) = collector::mock()
/// .enter(span.clone())
/// .exit(span)
/// .enter(&span)
/// .exit(&span)
/// .run_with_handle();
///
/// tracing::collect::with_default(collector, || {
Expand All @@ -572,8 +575,11 @@ where
/// ```
///
/// [`enter`]: fn@Self::enter
pub fn exit(mut self, span: ExpectedSpan) -> Self {
self.expected.push_back(Expect::Exit(span));
pub fn exit<S>(mut self, span: S) -> Self
where
S: Into<ExpectedSpan>,
{
self.expected.push_back(Expect::Exit(span.into()));
self
}

Expand Down Expand Up @@ -627,8 +633,11 @@ where
///
/// handle.assert_finished();
/// ```
pub fn clone_span(mut self, span: ExpectedSpan) -> Self {
self.expected.push_back(Expect::CloneSpan(span));
pub fn clone_span<S>(mut self, span: S) -> Self
where
S: Into<ExpectedSpan>,
{
self.expected.push_back(Expect::CloneSpan(span.into()));
self
}

Expand All @@ -644,8 +653,11 @@ where
///
/// [`Collect::drop_span`]: fn@tracing::Collect::drop_span
#[allow(deprecated)]
pub fn drop_span(mut self, span: ExpectedSpan) -> Self {
self.expected.push_back(Expect::DropSpan(span));
pub fn drop_span<S>(mut self, span: S) -> Self
where
S: Into<ExpectedSpan>,
{
self.expected.push_back(Expect::DropSpan(span.into()));
self
}

Expand Down Expand Up @@ -710,9 +722,15 @@ where
/// ```
///
/// [`Span::follows_from`]: fn@tracing::Span::follows_from
pub fn follows_from(mut self, consequence: ExpectedSpan, cause: ExpectedSpan) -> Self {
self.expected
.push_back(Expect::FollowsFrom { consequence, cause });
pub fn follows_from<S1, S2>(mut self, consequence: S1, cause: S2) -> Self
where
S1: Into<ExpectedSpan>,
S2: Into<ExpectedSpan>,
{
self.expected.push_back(Expect::FollowsFrom {
consequence: consequence.into(),
cause: cause.into(),
});
self
}

Expand Down Expand Up @@ -775,11 +793,13 @@ where
/// ```
///
/// [`field`]: mod@crate::field
pub fn record<I>(mut self, span: ExpectedSpan, fields: I) -> Self
pub fn record<S, I>(mut self, span: S, fields: I) -> Self
where
S: Into<ExpectedSpan>,
I: Into<ExpectedFields>,
{
self.expected.push_back(Expect::Visit(span, fields.into()));
self.expected
.push_back(Expect::Visit(span.into(), fields.into()));
self
}

Expand Down
Loading

0 comments on commit 5daa282

Please sign in to comment.