Skip to content

Commit 7b2cbc6

Browse files
committed
mock: improve ergonomics when an ExpectedSpan is needed (#3097)
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.
1 parent ad260f4 commit 7b2cbc6

File tree

3 files changed

+196
-93
lines changed

3 files changed

+196
-93
lines changed

tracing-mock/src/layer.rs

Lines changed: 29 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
//! An implementation of the [`Layer`] trait which validates that
2-
//! the `tracing` data it recieves matches the expected output for a test.
2+
//! the `tracing` data it receives matches the expected output for a test.
33
//!
44
//!
55
//! The [`MockLayer`] is the central component in these tools. The
66
//! `MockLayer` has expectations set on it which are later
77
//! validated as the code under test is run.
88
//!
99
//! ```
10-
//! use tracing_mock::{expect, field, layer};
10+
//! use tracing_mock::{expect, layer};
1111
//! use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt, Layer};
1212
//!
1313
//! let (layer, handle) = layer::mock()
@@ -40,11 +40,11 @@
4040
//! .named("my_span");
4141
//! let (layer, handle) = layer::mock()
4242
//! // Enter a matching span
43-
//! .enter(span.clone())
43+
//! .enter(&span)
4444
//! // Record an event with message "collect parting message"
4545
//! .event(expect::event().with_fields(expect::message("say hello")))
4646
//! // Exit a matching span
47-
//! .exit(span)
47+
//! .exit(&span)
4848
//! // Expect no further messages to be recorded
4949
//! .only()
5050
//! // Return the layer and handle
@@ -75,18 +75,18 @@
7575
//! span before recording an event, the test will fail:
7676
//!
7777
//! ```should_panic
78-
//! use tracing_mock::{expect, field, layer};
78+
//! use tracing_mock::{expect, layer};
7979
//! use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt, Layer};
8080
//!
8181
//! let span = expect::span()
8282
//! .named("my_span");
8383
//! let (layer, handle) = layer::mock()
8484
//! // Enter a matching span
85-
//! .enter(span.clone())
85+
//! .enter(&span)
8686
//! // Record an event with message "collect parting message"
8787
//! .event(expect::event().with_fields(expect::message("say hello")))
8888
//! // Exit a matching span
89-
//! .exit(span)
89+
//! .exit(&span)
9090
//! // Expect no further messages to be recorded
9191
//! .only()
9292
//! // Return the subscriber and handle
@@ -146,18 +146,18 @@ use std::{
146146
/// # Examples
147147
///
148148
/// ```
149-
/// use tracing_mock::{expect, field, layer};
149+
/// use tracing_mock::{expect, layer};
150150
/// use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt, Layer};
151151
///
152152
/// let span = expect::span()
153153
/// .named("my_span");
154154
/// let (layer, handle) = layer::mock()
155155
/// // Enter a matching span
156-
/// .enter(span.clone())
156+
/// .enter(&span)
157157
/// // Record an event with message "collect parting message"
158158
/// .event(expect::event().with_fields(expect::message("say hello")))
159159
/// // Exit a matching span
160-
/// .exit(span)
160+
/// .exit(&span)
161161
/// // Expect no further messages to be recorded
162162
/// .only()
163163
/// // Return the subscriber and handle
@@ -414,7 +414,7 @@ impl MockLayerBuilder {
414414
///
415415
/// This function accepts `Into<NewSpan>` instead of
416416
/// [`ExpectedSpan`] directly. [`NewSpan`] can be used to test
417-
/// span fields and the span parent.
417+
/// span fields and the span ancestry.
418418
///
419419
/// The new span doesn't need to be entered for this expectation
420420
/// to succeed.
@@ -504,8 +504,8 @@ impl MockLayerBuilder {
504504
/// .at_level(tracing::Level::INFO)
505505
/// .named("the span we're testing");
506506
/// let (layer, handle) = layer::mock()
507-
/// .enter(span.clone())
508-
/// .exit(span)
507+
/// .enter(&span)
508+
/// .exit(&span)
509509
/// .only()
510510
/// .run_with_handle();
511511
///
@@ -532,8 +532,8 @@ impl MockLayerBuilder {
532532
/// .at_level(tracing::Level::INFO)
533533
/// .named("the span we're testing");
534534
/// let (layer, handle) = layer::mock()
535-
/// .enter(span.clone())
536-
/// .exit(span)
535+
/// .enter(&span)
536+
/// .exit(&span)
537537
/// .only()
538538
/// .run_with_handle();
539539
///
@@ -552,8 +552,11 @@ impl MockLayerBuilder {
552552
///
553553
/// [`exit`]: fn@Self::exit
554554
/// [`only`]: fn@Self::only
555-
pub fn enter(mut self, span: ExpectedSpan) -> Self {
556-
self.expected.push_back(Expect::Enter(span));
555+
pub fn enter<S>(mut self, span: S) -> Self
556+
where
557+
S: Into<ExpectedSpan>,
558+
{
559+
self.expected.push_back(Expect::Enter(span.into()));
557560
self
558561
}
559562

@@ -581,8 +584,8 @@ impl MockLayerBuilder {
581584
/// .at_level(tracing::Level::INFO)
582585
/// .named("the span we're testing");
583586
/// let (layer, handle) = layer::mock()
584-
/// .enter(span.clone())
585-
/// .exit(span)
587+
/// .enter(&span)
588+
/// .exit(&span)
586589
/// .only()
587590
/// .run_with_handle();
588591
///
@@ -608,8 +611,8 @@ impl MockLayerBuilder {
608611
/// .at_level(tracing::Level::INFO)
609612
/// .named("the span we're testing");
610613
/// let (layer, handle) = layer::mock()
611-
/// .enter(span.clone())
612-
/// .exit(span)
614+
/// .enter(&span)
615+
/// .exit(&span)
613616
/// .only()
614617
/// .run_with_handle();
615618
///
@@ -629,8 +632,11 @@ impl MockLayerBuilder {
629632
/// [`enter`]: fn@Self::enter
630633
/// [`MockHandle::assert_finished`]: fn@crate::subscriber::MockHandle::assert_finished
631634
/// [`Span::enter`]: fn@tracing::Span::enter
632-
pub fn exit(mut self, span: ExpectedSpan) -> Self {
633-
self.expected.push_back(Expect::Exit(span));
635+
pub fn exit<S>(mut self, span: S) -> Self
636+
where
637+
S: Into<ExpectedSpan>,
638+
{
639+
self.expected.push_back(Expect::Exit(span.into()));
634640
self
635641
}
636642

0 commit comments

Comments
 (0)