From b2816a2bc283d48de721bceb455bf198af9739bb Mon Sep 17 00:00:00 2001 From: Hayden Stainsby Date: Thu, 5 Jan 2023 01:55:07 +0100 Subject: [PATCH] added negative examples to most methods Also cleaned up the text and tried to apply the code review comments from #2426 to what I'd written already. --- tracing-mock/src/collector.rs | 341 ++++++++++++++++++++++++++++------ 1 file changed, 288 insertions(+), 53 deletions(-) diff --git a/tracing-mock/src/collector.rs b/tracing-mock/src/collector.rs index 6856ec52c8..9eb36274d4 100644 --- a/tracing-mock/src/collector.rs +++ b/tracing-mock/src/collector.rs @@ -107,15 +107,17 @@ where /// 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 + /// By default, the mock collector'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 + /// When a test has only one mock collector, this is sufficient. However, + /// some tests may include multiple collectors, in order to test + /// interactions between multiple collectors. In that case, it can be + /// helpful to give each collector a separate name to distinguish where the /// debugging output comes from. /// + /// # Examples + /// /// ``` /// use tracing::collect::with_default; /// use tracing_mock::{collector, expect}; @@ -138,12 +140,18 @@ where } } - /// Expects an event matching the [`ExpectedEvent`] to be traced. + /// Adds the expectation that an event matching the [`ExpectedEvent`] + /// will be recorded next. + /// + /// The `event` can be a default mock which will match any event + /// (`expect::event()`) or can include additional expectations. + /// See the [`ExpectedEvent`] documentation for more details. + /// + /// If an event is recorded that doesn't match the `ExpectedEvent`, + /// or if something else (such as entering a span) is recorded + /// first, then the expectation will fail. /// - /// The `event` can be simple a default mock which will match - /// any event (`expect::event()`) or can include - /// additional requirements. See the [`ExpectedEvent`] documentation - /// for more details. + /// # Examples /// /// ``` /// use tracing::collect::with_default; @@ -159,20 +167,47 @@ where /// /// handle.assert_finished(); /// ``` + /// + /// A span is entered before the event, causing the test to fail: + /// + /// ```should_panic + /// use tracing::collect::with_default; + /// use tracing_mock::{collector, expect}; + /// + /// let (collector, handle) = collector::mock() + /// .event(expect::event()) + /// .run_with_handle(); + /// + /// with_default(collector, || { + /// let span = tracing::info_span!("span"); + /// let _guard = span.enter(); + /// tracing::info!("a"); + /// }); + /// + /// handle.assert_finished(); + /// ``` pub fn event(mut self, event: ExpectedEvent) -> Self { self.expected.push_back(Expect::Event(event)); self } - /// Expects a span matching `new_span` to be created. + /// Adds the expectation that the creation of a span will be + /// recorded next. /// /// This function accepts `Into` instead of - /// [`ExpectedSpan`] directly. So it can be used to test + /// [`ExpectedSpan`] directly, so it can be used to test /// span fields and the span parent. This is because a /// collector only receives the span fields and parent when /// a span is created, not when it is entered. /// - /// The new span doesn't need to have been entered. + /// The new span doesn't need to be entered for this expectation + /// to succeed. + /// + /// If a span is recorded that doesn't match the `ExpectedSpan`, + /// or if something else (such as an event) is recorded first, + /// then the expectation will fail. + /// + /// # Examples /// /// ``` /// use tracing::collect::with_default; @@ -192,6 +227,29 @@ where /// /// handle.assert_finished(); /// ``` + /// + /// An event is recorded before the span is created, causing the + /// test to fail: + /// + /// ```should_panic + /// use tracing::collect::with_default; + /// use tracing_mock::{collector, expect}; + /// + /// let span = expect::span() + /// .at_level(tracing::Level::INFO) + /// .named("the span we're testing") + /// .with_field(expect::field("testing").with_value(&"yes")); + /// let (collector, handle) = collector::mock() + /// .new_span(span) + /// .run_with_handle(); + /// + /// with_default(collector, || { + /// tracing::info!("an event"); + /// _ = tracing::info_span!("the span we're testing", testing = "yes"); + /// }); + /// + /// handle.assert_finished(); + /// ``` pub fn new_span(mut self, new_span: I) -> Self where I: Into, @@ -200,12 +258,19 @@ where self } - /// Expects a span matching the [`ExpectedSpan`] to be entered. + /// Adds the expectation that entering a span matching the + /// [`ExpectedSpan`] will be recorded next. /// /// This expectation is generally accompanied by a call to /// [`exit`] as well. If used together with [`only`], this /// is necessary. /// + /// If the span that is entered doesn't match the [`ExpectedSpan`], + /// or if something else (such as an event) is recorded first, + /// then the expectation will fail. + /// + /// # Examples + /// /// ``` /// use tracing::collect::with_default; /// use tracing_mock::{collector, expect}; @@ -227,6 +292,31 @@ where /// handle.assert_finished(); /// ``` /// + /// An event is recorded before the span is entered, causing the + /// test to fail: + /// + /// ```should_panic + /// use tracing::collect::with_default; + /// use tracing_mock::{collector, expect}; + /// + /// let span = expect::span() + /// .at_level(tracing::Level::INFO) + /// .named("the span we're testing"); + /// let (collector, handle) = collector::mock() + /// .enter(span.clone()) + /// .exit(span) + /// .only() + /// .run_with_handle(); + /// + /// with_default(collector, || { + /// tracing::info!("an event"); + /// let span = tracing::info_span!("the span we're testing"); + /// let _entered = span.enter(); + /// }); + /// + /// handle.assert_finished(); + /// ``` + /// /// [`exit`]: fn@Self::exit /// [`only`]: fn@Self::only pub fn enter(mut self, span: ExpectedSpan) -> Self { @@ -234,12 +324,19 @@ where self } - /// Expects a span matching the [`ExpectedSpan`] to exit. + /// Adds the expectation that exiting a span matching the + /// [`ExpectedSpan`] will be recorded next. /// /// As a span may be entered and exited multiple times, /// this is different from the span being closed. In /// general [`enter`] and `exit` should be paired. /// + /// If the span that is exited doesn't match the [`ExpectedSpan`], + /// or if something else (such as an event) is recorded first, + /// then the expectation will fail. + /// + /// # Examples + /// /// ``` /// use tracing::collect::with_default; /// use tracing_mock::{collector, expect}; @@ -249,19 +346,36 @@ where /// .named("the span we're testing"); /// let (collector, handle) = collector::mock() /// .enter(span.clone()) - /// .exit(span.clone()) + /// .exit(span) + /// .run_with_handle(); + /// + /// with_default(collector, || { + /// let span = tracing::info_span!("the span we're testing"); + /// let _entered = span.enter(); + /// }); + /// + /// handle.assert_finished(); + /// ``` + /// + /// An event is recorded before the span is exited, causing the + /// test to fail: + /// + /// ```should_panic + /// use tracing::collect::with_default; + /// use tracing_mock::{collector, expect}; + /// + /// let span = expect::span() + /// .at_level(tracing::Level::INFO) + /// .named("the span we're testing"); + /// let (collector, handle) = collector::mock() /// .enter(span.clone()) /// .exit(span) /// .run_with_handle(); /// /// with_default(collector, || { /// let span = tracing::info_span!("the span we're testing"); - /// { - /// let _entered = span.enter(); - /// } - /// { - /// let _entered = span.enter(); - /// } + /// let _entered = span.enter(); + /// tracing::info!("an event"); /// }); /// /// handle.assert_finished(); @@ -273,11 +387,40 @@ where self } - /// Expects a span matching the [`ExpectedSpan`] to be cloned. + /// Adds the expectation that cloning a span matching the + /// [`ExpectedSpan`] will be recorded next. + /// + /// The cloned span does need to be entered. + /// + /// If the span that is cloned doesn't match the [`ExpectedSpan`], + /// or if something else (such as an event) is recorded first, + /// then the expectation will fail. + /// + /// # Examples + /// + /// ``` + /// use tracing::collect::with_default; + /// use tracing_mock::{collector, expect}; /// - /// The cloned span does need to have been entered. + /// let span = expect::span() + /// .at_level(tracing::Level::INFO) + /// .named("the span we're testing"); + /// let (collector, handle) = collector::mock() + /// .clone_span(span) + /// .run_with_handle(); + /// + /// with_default(collector, || { + /// let span = tracing::info_span!("the span we're testing"); + /// _ = span.clone(); + /// }); /// + /// handle.assert_finished(); /// ``` + /// + /// An event is recorded before the span is cloned, causing the + /// test to fail: + /// + /// ```should_panic /// use tracing::collect::with_default; /// use tracing_mock::{collector, expect}; /// @@ -290,6 +433,7 @@ where /// /// with_default(collector, || { /// let span = tracing::info_span!("the span we're testing"); + /// tracing::info!("an event"); /// _ = span.clone(); /// }); /// @@ -302,8 +446,9 @@ where /// **This method is deprecated.** /// - /// Expects a span matching the [`ExpectedSpan`] to be dropped via - /// the deprecated function [`Collect::drop_span`]. + /// Adds the expectation that a span matching the [`ExpectedSpan`] + /// getting dropped` via the deprecated function + /// [`Collect::drop_span`] will be recorded next. /// /// Instead [`Collect::try_close`] should be used on the collector /// and should be asserted with `close_span` (which hasn't been @@ -316,22 +461,31 @@ where self } - /// Expects that a span matching `consequence` follows from a span matching `cause`. + /// Adds the expectation that a `follows_from` relationship will be + /// recorded next. Specifically that a span matching `consequence` + /// follows from a span matching `cause`. /// /// For further details on what this causal relationship means, see /// [`Span::follows_from`]. /// + /// If either of the 2 spans don't match their respective + /// [`ExpectedSpan`] or if something else (such as an event) is + /// recorded first, then the expectation will fail. + /// + /// **Note**: The 2 spans, `consequence` and `cause` are matched + /// by `name `only. + /// + /// # Examples + /// /// ``` /// use tracing::collect::with_default; /// use tracing_mock::{collector, expect}; /// - /// let span_1 = expect::span().named("cause"); - /// let span_2 = expect::span().named("consequence"); + /// let cause = expect::span().named("cause"); + /// let consequence = expect::span().named("consequence"); /// /// let (collector, handle) = collector::mock() - /// .new_span(span_1.clone()) - /// .new_span(span_2.clone()) - /// .follows_from(span_2, span_1) + /// .follows_from(consequence, cause) /// .run_with_handle(); /// /// with_default(collector, || { @@ -344,6 +498,31 @@ where /// handle.assert_finished(); /// ``` /// + /// The `cause` span doesn't match, it is actually recorded at + /// `Level::WARN` instead of the expected `Level::INFO`, causing + /// this test to fail: + /// + /// ```should_panic + /// use tracing::collect::with_default; + /// use tracing_mock::{collector, expect}; + /// + /// let cause = expect::span().named("cause"); + /// let consequence = expect::span().named("consequence"); + /// + /// let (collector, handle) = collector::mock() + /// .follows_from(consequence, cause) + /// .run_with_handle(); + /// + /// with_default(collector, || { + /// let cause = tracing::info_span!("another cause"); + /// let consequence = tracing::info_span!("consequence"); + /// + /// consequence.follows_from(&cause); + /// }); + /// + /// handle.assert_finished(); + /// ``` + /// /// [`Span::follows_from`]: fn@tracing::Span::follows_from pub fn follows_from(mut self, consequence: ExpectedSpan, cause: ExpectedSpan) -> Self { self.expected @@ -351,8 +530,18 @@ where self } - /// Expect that `fields` are recorded on a span matching the - /// [`ExpectedSpan`]. + /// Adds the expectation that `fields` are recorded on a span + /// matching the [`ExpectedSpan`] will be recorded next. + /// + /// For further information on how to specify the expected + /// fields, see the documentation on the [`field`] module. + /// + /// If either the span doesn't match the [`ExpectedSpan`], the + /// fields don't match the expected fields, or if something else + /// (such as an event) is recorded first, then the expectation + /// will fail. + /// + /// # Examples /// /// ``` /// use tracing::collect::with_default; @@ -361,7 +550,6 @@ where /// let span = expect::span() /// .named("my_span"); /// let (collector, handle) = collector::mock() - /// .new_span(span.clone()) /// .record(span, expect::field("parting").with_value(&"goodbye world!")) /// .run_with_handle(); /// @@ -376,6 +564,33 @@ where /// /// handle.assert_finished(); /// ``` + /// + /// The value of the recorded field doesn't match the expectation, + /// causing the test to fail: + /// + /// ```should_panic + /// use tracing::collect::with_default; + /// use tracing_mock::{collector, expect}; + /// + /// let span = expect::span() + /// .named("my_span"); + /// let (collector, handle) = collector::mock() + /// .record(span, expect::field("parting").with_value(&"goodbye world!")) + /// .run_with_handle(); + /// + /// with_default(collector, || { + /// let span = tracing::trace_span!( + /// "my_span", + /// greeting = "hello world", + /// parting = tracing::field::Empty + /// ); + /// span.record("parting", "goodbye universe!"); + /// }); + /// + /// handle.assert_finished(); + /// ``` + /// + /// [`field`]: mod@crate::field pub fn record(mut self, span: ExpectedSpan, fields: I) -> Self where I: Into, @@ -387,8 +602,11 @@ where /// Filter the traces evaluated by the `MockCollector`. /// /// The filter will be applied to all traces received before - /// any verification occurs - so its position in the call chain - /// is not important. + /// any validation occurs - so its position in the call chain + /// is not important. The filter does not perform any validation + /// itself. + /// + /// # Examples /// /// ``` /// use tracing::collect::with_default; @@ -419,11 +637,21 @@ where } } - /// Sets the max level that will be provided to the `tracing` system. + /// Sets the max level that will be provided to the `tracing` + /// system. /// - /// This method can be used to test the internals of `tracing`, but it - /// is also useful to filter out traces on more verbose levels if you - /// only want to verify above a certain level. + /// This method can be used to test the internals of `tracing`, + /// but it is also useful to filter out traces on more verbose + /// levels if you only want to verify above a certain level. + /// + /// + /// **Note**: this value determines a global filter, if + /// `with_max_level_hint` is called on multiple collectors, the + /// global filter will be the least restrictive of all collectors. + /// To filter the events evaluated by a specific `MockCollector`, + /// use [`with_filter`] instead. + /// + /// # Examples /// /// ``` /// use tracing::collect::with_default; @@ -437,19 +665,12 @@ where /// /// with_default(collector, || { /// tracing::debug!("a message we don't care about"); - /// tracing::info!("a message we want to verify"); + /// tracing::info!("a message we want to validate"); /// }); /// /// handle.assert_finished(); /// ``` /// - /// Note that this value determines a global filter, if `with_max_level_hint` - /// is called on multiple collectors, the global filter will be the least - /// restrictive of all collectors. - /// - /// To filter the events evaluated by a specific `MockCollector`, use - /// [`with_filter`] instead. - /// /// [`with_filter`]: fn@Self::with_filter pub fn with_max_level_hint(self, hint: impl Into) -> Self { Self { @@ -460,8 +681,14 @@ where /// Expects that no further traces are received. /// + /// The call to `only` should appear immediately before the final + /// call to `run` or `run_with_handle`, as any expectations which + /// are added after `only` will not be considered. + /// + /// # Examples + /// /// Consider this simple test. It passes even though we only - /// expect a single event, but receive three. + /// expect a single event, but receive three: /// /// ``` /// use tracing::collect::with_default; @@ -480,7 +707,7 @@ where /// handle.assert_finished(); /// ``` /// - /// Now we include `only` and this test will fail. + /// After including `only`, the test will fail: /// /// ```should_panic /// use tracing::collect::with_default; @@ -511,8 +738,10 @@ where /// return a [`MockHandle`]. This is useful if the desired /// assertions can be checked externally to the collector. /// - /// For example, the following test is used within the `tracing` - /// codebase. + /// # Examples + /// + /// The following test is used within the `tracing` + /// codebase: /// /// ``` /// use tracing::collect::with_default; @@ -537,6 +766,8 @@ where /// be set as the default collector and a [`MockHandle`] which can /// be used to validate the provided expectations. /// + /// # Examples + /// /// ``` /// use tracing::collect::with_default; /// use tracing_mock::{collector, expect}; @@ -654,9 +885,11 @@ where cause: ref expected_cause, }) => { if let Some(name) = expected_consequence.name() { + // TODO(hds): Write proper assertion text. assert_eq!(name, consequence_span.name); } if let Some(name) = expected_cause.name() { + // TODO(hds): Write proper assertion text. assert_eq!(name, cause_span.name); } } @@ -864,6 +1097,8 @@ impl MockHandle { /// This method will panic if any of the provided expectations are /// not met. /// + /// # Examples + /// /// ``` /// use tracing::collect::with_default; /// use tracing_mock::{collector, expect};