From 5a2d6853a48ccc273b4cf521883657555298ab11 Mon Sep 17 00:00:00 2001 From: Hayden Stainsby Date: Thu, 12 Jan 2023 18:04:19 +0100 Subject: [PATCH 1/9] mock: document public APIs in the `field` module This change adds documentation to the `field` module and all the public APIs within it. This includes doctests on all the methods which serve as examples. Additionally, the `field::msg` function (which constructs a field with name "message" and the provided value) was moved to `expect::message`. This is part of a unification of all expectation constructors inside the `expect` module. Refs: #539 --- tracing-mock/README.md | 10 +- tracing-mock/src/event.rs | 2 +- tracing-mock/src/expect.rs | 9 + tracing-mock/src/field.rs | 382 +++++++++++++++++++++++++++++++++++-- tracing/tests/event.rs | 4 +- 5 files changed, 386 insertions(+), 21 deletions(-) diff --git a/tracing-mock/README.md b/tracing-mock/README.md index b508db00cb..5267eb6b6d 100644 --- a/tracing-mock/README.md +++ b/tracing-mock/README.md @@ -71,14 +71,14 @@ Below is an example that checks that an event contains a message: ```rust use tracing::collect::with_default; -use tracing_mock::{collector, expect, field}; +use tracing_mock::{collector, expect}; fn yak_shaving() { tracing::info!("preparing to shave yaks"); } let (collector, handle) = collector::mock() - .event(expect::event().with_fields(field::msg("preparing to shave yaks"))) + .event(expect::event().with_fields(expect::message("preparing to shave yaks"))) .only() .run_with_handle(); @@ -102,7 +102,7 @@ Below is a slightly more complex example. `tracing-mock` asserts that, in order: ```rust use tracing::collect::with_default; -use tracing_mock::{collector, expect, field}; +use tracing_mock::{collector, expect}; #[tracing::instrument] fn yak_shaving(number_of_yaks: u32) { @@ -128,7 +128,7 @@ let (collector, handle) = collector::mock() expect::event().with_fields( expect::field("number_of_yaks") .with_value(&yak_count) - .and(field::msg("preparing to shave yaks")) + .and(expect::message("preparing to shave yaks")) .only(), ), ) @@ -136,7 +136,7 @@ let (collector, handle) = collector::mock() expect::event().with_fields( expect::field("all_yaks_shaved") .with_value(&true) - .and(field::msg("yak shaving completed.")) + .and(expect::message("yak shaving completed.")) .only(), ), ) diff --git a/tracing-mock/src/event.rs b/tracing-mock/src/event.rs index 34ed9288c5..5de1db777b 100644 --- a/tracing-mock/src/event.rs +++ b/tracing-mock/src/event.rs @@ -48,7 +48,7 @@ pub struct ExpectedEvent { } pub fn msg(message: impl fmt::Display) -> ExpectedEvent { - expect::event().with_fields(field::msg(message)) + expect::event().with_fields(expect::message(message)) } impl ExpectedEvent { diff --git a/tracing-mock/src/expect.rs b/tracing-mock/src/expect.rs index a21f260e8d..e46c2f735e 100644 --- a/tracing-mock/src/expect.rs +++ b/tracing-mock/src/expect.rs @@ -4,6 +4,8 @@ use crate::{ span::{ExpectedSpan, NewSpan}, }; +use std::fmt; + #[derive(Debug, Eq, PartialEq)] pub(crate) enum Expect { Event(ExpectedEvent), @@ -36,6 +38,13 @@ where } } +pub fn message(message: impl fmt::Display) -> ExpectedField { + ExpectedField { + name: "message".to_string(), + value: ExpectedValue::Debug(message.to_string()), + } +} + pub fn span() -> ExpectedSpan { ExpectedSpan { ..Default::default() diff --git a/tracing-mock/src/field.rs b/tracing-mock/src/field.rs index fa956c7946..95b4ea6846 100644 --- a/tracing-mock/src/field.rs +++ b/tracing-mock/src/field.rs @@ -1,3 +1,86 @@ +//! Define expectations to validate fields on events and spans. +//! +//! The [`ExpectedField`] struct define expected values for fields in +//! order to match events and spans via the mock collector API in the +//! [`collector`] module. +//! +//! Expected fields should be created with [`expect::field`] and a +//! chain of method calls to specify the field value and additional +//! fields as necessary. +//! +//! # Examples +//! +//! The simplest case is to expect that an event has a field with a +//! specific name, without any expectation about the value: +//! +//! ``` +//! use tracing_mock::{collector, expect}; +//! +//! let event = expect::event() +//! .with_fields(expect::field("field_name")); +//! +//! let (collector, handle) = collector::mock() +//! .event(event) +//! .run_with_handle(); +//! +//! tracing::collect::with_default(collector, || { +//! tracing::info!(field_name = "value"); +//! }); +//! +//! handle.assert_finished(); +//! ``` +//! +//! It is possible to expect multiple fields and specify the value for +//! each of them: +//! +//! ``` +//! use tracing_mock::{collector, expect}; +//! +//! let event = expect::event().with_fields( +//! expect::field("string_field") +//! .with_value(&"field_value") +//! .and(expect::field("integer_field").with_value(&54_i64)) +//! .and(expect::field("bool_field").with_value(&true)), +//! ); +//! +//! let (collector, handle) = collector::mock() +//! .event(event) +//! .run_with_handle(); +//! +//! tracing::collect::with_default(collector, || { +//! tracing::info!( +//! string_field = "field_value", +//! integer_field = 54_i64, +//! bool_field = true, +//! ); +//! }); +//! +//! handle.assert_finished(); +//! ``` +//! +//! If an expected field is not present, or if the value of the field +//! is different, the test will fail. In this example, the value is +//! different: +//! +//! ```should_panic +//! use tracing_mock::{collector, expect}; +//! +//! let event = expect::event() +//! .with_fields(expect::field("field_name").with_value(&"value")); +//! +//! let (collector, handle) = collector::mock() +//! .event(event) +//! .run_with_handle(); +//! +//! tracing::collect::with_default(collector, || { +//! tracing::info!(field_name = "different value"); +//! }); +//! +//! handle.assert_finished(); +//! ``` +//! +//! [`collector`]: mod@crate::collector +//! [`expect::field`]: fn@crate::expect::field use tracing::{ callsite, callsite::Callsite, @@ -7,12 +90,24 @@ use tracing::{ use std::{collections::HashMap, fmt}; +/// An expectation for multiple fields. +/// +/// For a detailed description and examples, see the documentation for +/// the methods and the [`field`] module. +/// +/// [`field`]: mod@crate::field #[derive(Default, Debug, Eq, PartialEq)] pub struct ExpectedFields { fields: HashMap, only: bool, } +/// An expected field. +/// +/// For a detailed description and examples, see the documentation for +/// the methods and the [`field`] module. +/// +/// [`field`]: mod@crate::field #[derive(Debug)] pub struct ExpectedField { pub(super) name: String, @@ -20,7 +115,7 @@ pub struct ExpectedField { } #[derive(Debug)] -pub enum ExpectedValue { +pub(crate) enum ExpectedValue { F64(f64), I64(i64), U64(u64), @@ -55,15 +150,49 @@ impl PartialEq for ExpectedValue { } } -pub fn msg(message: impl fmt::Display) -> ExpectedField { - ExpectedField { - name: "message".to_string(), - value: ExpectedValue::Debug(message.to_string()), - } -} - impl ExpectedField { - /// Expect a field with the given name and value. + /// Sets the value to expect when matching this field. + /// + /// If the recorded value for this field is different, the + /// expectation will fail. + /// + /// # Examples + /// + /// ``` + /// use tracing_mock::{collector, expect}; + /// + /// let event = expect::event() + /// .with_fields(expect::field("field_name").with_value(&"value")); + /// + /// let (collector, handle) = collector::mock() + /// .event(event) + /// .run_with_handle(); + /// + /// tracing::collect::with_default(collector, || { + /// tracing::info!(field_name = "value"); + /// }); + /// + /// handle.assert_finished(); + /// ``` + /// + /// A different value will cause the test to fail: + /// + /// ```should_panic + /// use tracing_mock::{collector, expect}; + /// + /// let event = expect::event() + /// .with_fields(expect::field("field_name").with_value(&"value")); + /// + /// let (collector, handle) = collector::mock() + /// .event(event) + /// .run_with_handle(); + /// + /// tracing::collect::with_default(collector, || { + /// tracing::info!(field_name = "different value"); + /// }); + /// + /// handle.assert_finished(); + /// ``` pub fn with_value(self, value: &dyn Value) -> Self { Self { value: ExpectedValue::from(value), @@ -71,6 +200,58 @@ impl ExpectedField { } } + /// Adds an additional [`ExpectedField`] to be matched. + /// + /// Both fields must match, if either of them are not present, or + /// if the value for either field is different, the expectation + /// will fail. + /// + /// # Examples + /// + /// ``` + /// use tracing_mock::{collector, expect}; + /// + /// let event = expect::event().with_fields( + /// expect::field("field") + /// .with_value(&"value") + /// .and(expect::field("another_field").with_value(&42)), + /// ); + /// + /// let (collector, handle) = collector::mock() + /// .event(event) + /// .run_with_handle(); + /// + /// tracing::collect::with_default(collector, || { + /// tracing::info!( + /// field = "value", + /// another_field = 42, + /// ); + /// }); + /// + /// handle.assert_finished(); + /// ``` + /// + /// If the second field is not present, the test will fail: + /// + /// ```should_panic + /// use tracing_mock::{collector, expect}; + /// + /// let event = expect::event().with_fields( + /// expect::field("field") + /// .with_value(&"value") + /// .and(expect::field("another_field").with_value(&42)), + /// ); + /// + /// let (collector, handle) = collector::mock() + /// .event(event) + /// .run_with_handle(); + /// + /// tracing::collect::with_default(collector, || { + /// tracing::info!(field = "value"); + /// }); + /// + /// handle.assert_finished(); + /// ``` pub fn and(self, other: ExpectedField) -> ExpectedFields { ExpectedFields { fields: HashMap::new(), @@ -80,6 +261,50 @@ impl ExpectedField { .and(other) } + /// Indicates that no fields other than those specified should be + /// expected. + /// + /// If additional fields are present on the recorded event or span, + /// the expectation will fail. + /// + /// # Examples + /// + /// The following test passes despite the recorded event having + /// fields that were not expected because `only` was not + /// used: + /// + /// ``` + /// use tracing_mock::{collector, expect}; + /// + /// let event = expect::event() + /// .with_fields(expect::field("field").with_value(&"value")); + /// + /// let (collector, handle) = collector::mock().event(event).run_with_handle(); + /// + /// tracing::collect::with_default(collector, || { + /// tracing::info!(field = "value", another_field = 42,); + /// }); + /// + /// handle.assert_finished(); + /// ``` + /// + /// If we include `only` on the `ExpectedField` then the test + /// will fail: + /// + /// ```should_panic + /// use tracing_mock::{collector, expect}; + /// + /// let event = expect::event() + /// .with_fields(expect::field("field").with_value(&"value").only()); + /// + /// let (collector, handle) = collector::mock().event(event).run_with_handle(); + /// + /// tracing::collect::with_default(collector, || { + /// tracing::info!(field = "value", another_field = 42,); + /// }); + /// + /// handle.assert_finished(); + /// ``` pub fn only(self) -> ExpectedFields { ExpectedFields { fields: HashMap::new(), @@ -100,12 +325,139 @@ impl From for ExpectedFields { } impl ExpectedFields { + /// Adds an additional [`ExpectedField`] to be matched. + /// + /// All fields must match, if any of them are not present, or if + /// the value for any field is different, the expectation will + /// fail. + /// + /// This method performs the same function as + /// [`ExpectedField::and`], but applies in the case where there are + /// already multiple fields expected. + /// + /// # Examples + /// + /// ``` + /// use tracing_mock::{collector, expect}; + /// + /// let event = expect::event().with_fields( + /// expect::field("field") + /// .with_value(&"value") + /// .and(expect::field("another_field").with_value(&42)) + /// .and(expect::field("a_third_field").with_value(&true)), + /// ); + /// + /// let (collector, handle) = collector::mock() + /// .event(event) + /// .run_with_handle(); + /// + /// tracing::collect::with_default(collector, || { + /// tracing::info!( + /// field = "value", + /// another_field = 42, + /// a_third_field = true, + /// ); + /// }); + /// + /// handle.assert_finished(); + /// ``` + /// + /// If any of the expected fields are not present on the recorded + /// event, the test will fail: + /// + /// ```should_panic + /// use tracing_mock::{collector, expect}; + /// + /// let event = expect::event().with_fields( + /// expect::field("field") + /// .with_value(&"value") + /// .and(expect::field("another_field").with_value(&42)) + /// .and(expect::field("a_third_field").with_value(&true)), + /// ); + /// + /// let (collector, handle) = collector::mock() + /// .event(event) + /// .run_with_handle(); + /// + /// tracing::collect::with_default(collector, || { + /// tracing::info!( + /// field = "value", + /// a_third_field = true, + /// ); + /// }); + /// + /// handle.assert_finished(); + /// ``` + /// + /// [`ExpectedField::and`]: fn@crate::field::ExpectedField::and pub fn and(mut self, field: ExpectedField) -> Self { self.fields.insert(field.name, field.value); self } - /// Indicates that no fields other than those specified should be expected. + /// Indicates that no fields other than those specified should be + /// expected. + /// + /// This method performs the same function as + /// [`ExpectedField::only`], but applies in the case where there are + /// multiple fields expected. + /// + /// # Examples + /// + /// The following test will pass, even though additional fields are + /// recorded on the event. + /// + /// ``` + /// use tracing_mock::{collector, expect}; + /// + /// let event = expect::event().with_fields( + /// expect::field("field") + /// .with_value(&"value") + /// .and(expect::field("another_field").with_value(&42)), + /// ); + /// + /// let (collector, handle) = collector::mock() + /// .event(event) + /// .run_with_handle(); + /// + /// tracing::collect::with_default(collector, || { + /// tracing::info!( + /// field = "value", + /// another_field = 42, + /// a_third_field = true, + /// ); + /// }); + /// + /// handle.assert_finished(); + /// ``` + /// + /// If we include `only` on the `ExpectedFields` then the test + /// will fail: + /// + /// ```should_panic + /// use tracing_mock::{collector, expect}; + /// + /// let event = expect::event().with_fields( + /// expect::field("field") + /// .with_value(&"value") + /// .and(expect::field("another_field").with_value(&42)) + /// .only(), + /// ); + /// + /// let (collector, handle) = collector::mock() + /// .event(event) + /// .run_with_handle(); + /// + /// tracing::collect::with_default(collector, || { + /// tracing::info!( + /// field = "value", + /// another_field = 42, + /// a_third_field = true, + /// ); + /// }); + /// + /// handle.assert_finished(); + /// ``` pub fn only(self) -> Self { Self { only: true, ..self } } @@ -132,7 +484,11 @@ impl ExpectedFields { } } - pub fn checker<'a>(&'a mut self, ctx: &'a str, collector_name: &'a str) -> CheckVisitor<'a> { + pub(crate) fn checker<'a>( + &'a mut self, + ctx: &'a str, + collector_name: &'a str, + ) -> CheckVisitor<'a> { CheckVisitor { expect: self, ctx, @@ -140,7 +496,7 @@ impl ExpectedFields { } } - pub fn is_empty(&self) -> bool { + pub(crate) fn is_empty(&self) -> bool { self.fields.is_empty() } } @@ -159,7 +515,7 @@ impl fmt::Display for ExpectedValue { } } -pub struct CheckVisitor<'a> { +pub(crate) struct CheckVisitor<'a> { expect: &'a mut ExpectedFields, ctx: &'a str, collector_name: &'a str, diff --git a/tracing/tests/event.rs b/tracing/tests/event.rs index 7e4f87c6d3..8efb42b5f3 100644 --- a/tracing/tests/event.rs +++ b/tracing/tests/event.rs @@ -85,7 +85,7 @@ fn message_without_delims() { .and( expect::field("question").with_value(&"life, the universe, and everything"), ) - .and(field::msg(format_args!( + .and(expect::message(format_args!( "hello from my event! tricky? {:?}!", true ))) @@ -114,7 +114,7 @@ fn string_message_without_delims() { .and( expect::field("question").with_value(&"life, the universe, and everything"), ) - .and(field::msg(format_args!("hello from my event"))) + .and(expect::message(format_args!("hello from my event"))) .only(), ), ) From 9041a289b73a5903197d0e6e31838acbac87cfd4 Mon Sep 17 00:00:00 2001 From: Hayden Stainsby Date: Mon, 30 Jan 2023 12:06:26 +0100 Subject: [PATCH 2/9] Update subscriber tests with new `expect::message` function Was previously `field::msg`. --- tracing-mock/src/subscriber.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tracing-mock/src/subscriber.rs b/tracing-mock/src/subscriber.rs index e923f56863..0f9c31c7cc 100644 --- a/tracing-mock/src/subscriber.rs +++ b/tracing-mock/src/subscriber.rs @@ -7,12 +7,12 @@ //! validated as the code under test is run. //! //! ``` -//! use tracing_mock::{expect, field, subscriber}; +//! use tracing_mock::{expect, subscriber}; //! use tracing_subscriber::{subscribe::CollectExt, util::SubscriberInitExt, Subscribe}; //! //! let (subscriber, handle) = subscriber::mock() //! // Expect a single event with a specified message -//! .event(expect::event().with_fields(field::msg("droids"))) +//! .event(expect::event().with_fields(expect::message("droids"))) //! .run_with_handle(); //! //! // Use `set_default` to apply the `MockSubscriber` until the end @@ -33,7 +33,7 @@ //! their respective fields: //! //! ``` -//! use tracing_mock::{expect, field, subscriber}; +//! use tracing_mock::{expect, subscriber}; //! use tracing_subscriber::{subscribe::CollectExt, util::SubscriberInitExt, Subscribe}; //! //! let span = expect::span() @@ -42,7 +42,7 @@ //! // Enter a matching span //! .enter(span.clone()) //! // Record an event with message "collect parting message" -//! .event(expect::event().with_fields(field::msg("say hello"))) +//! .event(expect::event().with_fields(expect::message("say hello"))) //! // Exit a matching span //! .exit(span) //! // Expect no further messages to be recorded @@ -75,7 +75,7 @@ //! span before recording an event, the test will fail: //! //! ```should_panic -//! use tracing_mock::{expect, field, subscriber}; +//! use tracing_mock::{expect, subscriber}; //! use tracing_subscriber::{subscribe::CollectExt, util::SubscriberInitExt, Subscribe}; //! //! let span = expect::span() @@ -84,7 +84,7 @@ //! // Enter a matching span //! .enter(span.clone()) //! // Record an event with message "collect parting message" -//! .event(expect::event().with_fields(field::msg("say hello"))) +//! .event(expect::event().with_fields(expect::message("say hello"))) //! // Exit a matching span //! .exit(span) //! // Expect no further messages to be recorded @@ -145,7 +145,7 @@ use std::{ /// # Examples /// /// ``` -/// use tracing_mock::{expect, field, subscriber}; +/// use tracing_mock::{expect, subscriber}; /// use tracing_subscriber::{subscribe::CollectExt, util::SubscriberInitExt, Subscribe}; /// /// let span = expect::span() @@ -154,7 +154,7 @@ use std::{ /// // Enter a matching span /// .enter(span.clone()) /// // Record an event with message "collect parting message" -/// .event(expect::event().with_fields(field::msg("say hello"))) +/// .event(expect::event().with_fields(expect::message("say hello"))) /// // Exit a matching span /// .exit(span) /// // Expect no further messages to be recorded From 49cd29ec7e9432312207b95e583678b4d4e8b479 Mon Sep 17 00:00:00 2001 From: Hayden Stainsby Date: Wed, 25 Oct 2023 14:21:41 +0200 Subject: [PATCH 3/9] remove extra `std::fmt` import --- tracing-mock/src/expect.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/tracing-mock/src/expect.rs b/tracing-mock/src/expect.rs index 45a40d1767..353bc52f5f 100644 --- a/tracing-mock/src/expect.rs +++ b/tracing-mock/src/expect.rs @@ -6,8 +6,6 @@ use crate::{ span::{ExpectedSpan, NewSpan}, }; -use std::fmt; - #[derive(Debug, Eq, PartialEq)] pub(crate) enum Expect { Event(ExpectedEvent), From 3ff2edfb2f562d443b26b872dbac9affaff1a81b Mon Sep 17 00:00:00 2001 From: Hayden Stainsby Date: Thu, 26 Oct 2023 11:11:52 +0200 Subject: [PATCH 4/9] fix collector tests to use `expect::message` --- tracing-mock/src/collector.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tracing-mock/src/collector.rs b/tracing-mock/src/collector.rs index 3298bfb22f..f1c14f17fb 100644 --- a/tracing-mock/src/collector.rs +++ b/tracing-mock/src/collector.rs @@ -12,7 +12,7 @@ //! //! let (collector, handle) = collector::mock() //! // Expect a single event with a specified message -//! .event(expect::event().with_fields(field::msg("droids"))) +//! .event(expect::event().with_fields(expect::message("droids"))) //! .only() //! .run_with_handle(); //! @@ -40,7 +40,7 @@ //! // Enter a matching span //! .enter(span.clone()) //! // Record an event with message "collect parting message" -//! .event(expect::event().with_fields(field::msg("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!")) //! // Exit a matching span @@ -81,7 +81,7 @@ //! .named("my_span"); //! let (collector, handle) = collector::mock() //! .enter(span.clone()) -//! .event(expect::event().with_fields(field::msg("collect parting message"))) +//! .event(expect::event().with_fields(expect::message("collect parting message"))) //! .record(span.clone(), expect::field("parting").with_value(&"goodbye world!")) //! .exit(span) //! .only() @@ -215,7 +215,7 @@ pub struct MockHandle(Arc>>, String); /// // Enter a matching span /// .enter(span.clone()) /// // Record an event with message "collect parting message" -/// .event(expect::event().with_fields(field::msg("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!")) /// // Exit a matching span From 0bf4e4d735d56af035127ede4a181dad2efa3034 Mon Sep 17 00:00:00 2001 From: Hayden Stainsby Date: Tue, 31 Oct 2023 11:13:18 +0100 Subject: [PATCH 5/9] Apply suggestions from code review Co-authored-by: David Barsky --- tracing-mock/src/field.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tracing-mock/src/field.rs b/tracing-mock/src/field.rs index 95b4ea6846..6723426803 100644 --- a/tracing-mock/src/field.rs +++ b/tracing-mock/src/field.rs @@ -153,8 +153,7 @@ impl PartialEq for ExpectedValue { impl ExpectedField { /// Sets the value to expect when matching this field. /// - /// If the recorded value for this field is different, the - /// expectation will fail. + /// If the recorded value for this field diffs, the expectation will fail. /// /// # Examples /// @@ -327,9 +326,9 @@ impl From for ExpectedFields { impl ExpectedFields { /// Adds an additional [`ExpectedField`] to be matched. /// - /// All fields must match, if any of them are not present, or if - /// the value for any field is different, the expectation will - /// fail. + /// _All_ fields must match for the expectation to pass. If any of + /// them are not present, if any of the values differs, the + /// expectation will fail. /// /// This method performs the same function as /// [`ExpectedField::and`], but applies in the case where there are @@ -405,7 +404,7 @@ impl ExpectedFields { /// # Examples /// /// The following test will pass, even though additional fields are - /// recorded on the event. + /// recorded on the event: /// /// ``` /// use tracing_mock::{collector, expect}; From 167aa318ef5efdf5d9ffee8f7ebc8349249f5027 Mon Sep 17 00:00:00 2001 From: Hayden Stainsby Date: Tue, 31 Oct 2023 18:30:24 +0100 Subject: [PATCH 6/9] Apply suggestions from code review Co-authored-by: David Barsky --- tracing-mock/src/field.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tracing-mock/src/field.rs b/tracing-mock/src/field.rs index 6723426803..96cac50af8 100644 --- a/tracing-mock/src/field.rs +++ b/tracing-mock/src/field.rs @@ -394,7 +394,7 @@ impl ExpectedFields { self } - /// Indicates that no fields other than those specified should be + /// Asserts that no fields other than those specified should be /// expected. /// /// This method performs the same function as From 7da0a27dfb1a6969ecf5caf5765b576efe906ef2 Mon Sep 17 00:00:00 2001 From: Hayden Stainsby Date: Tue, 31 Oct 2023 18:26:45 +0100 Subject: [PATCH 7/9] make the first test for `only()` be a positive test --- tracing-mock/src/field.rs | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/tracing-mock/src/field.rs b/tracing-mock/src/field.rs index 96cac50af8..a235bcc33b 100644 --- a/tracing-mock/src/field.rs +++ b/tracing-mock/src/field.rs @@ -268,27 +268,24 @@ impl ExpectedField { /// /// # Examples /// - /// The following test passes despite the recorded event having - /// fields that were not expected because `only` was not - /// used: + /// Check that only a single field is recorded. /// /// ``` /// use tracing_mock::{collector, expect}; /// /// let event = expect::event() - /// .with_fields(expect::field("field").with_value(&"value")); + /// .with_fields(expect::field("field").with_value(&"value").only()); /// /// let (collector, handle) = collector::mock().event(event).run_with_handle(); /// /// tracing::collect::with_default(collector, || { - /// tracing::info!(field = "value", another_field = 42,); + /// tracing::info!(field = "value"); /// }); /// /// handle.assert_finished(); /// ``` /// - /// If we include `only` on the `ExpectedField` then the test - /// will fail: + /// The following example fails because a second field is recorded. /// /// ```should_panic /// use tracing_mock::{collector, expect}; @@ -403,8 +400,7 @@ impl ExpectedFields { /// /// # Examples /// - /// The following test will pass, even though additional fields are - /// recorded on the event: + /// Check that only two fields are recorded on the event. /// /// ``` /// use tracing_mock::{collector, expect}; @@ -412,7 +408,8 @@ impl ExpectedFields { /// let event = expect::event().with_fields( /// expect::field("field") /// .with_value(&"value") - /// .and(expect::field("another_field").with_value(&42)), + /// .and(expect::field("another_field").with_value(&42)) + /// .only(), /// ); /// /// let (collector, handle) = collector::mock() @@ -423,15 +420,13 @@ impl ExpectedFields { /// tracing::info!( /// field = "value", /// another_field = 42, - /// a_third_field = true, /// ); /// }); /// /// handle.assert_finished(); /// ``` /// - /// If we include `only` on the `ExpectedFields` then the test - /// will fail: + /// The following example fails because a third field is recorded. /// /// ```should_panic /// use tracing_mock::{collector, expect}; From 3a8e2244746eec7721c7310625f8cc216abecd67 Mon Sep 17 00:00:00 2001 From: Hayden Stainsby Date: Tue, 31 Oct 2023 18:35:20 +0100 Subject: [PATCH 8/9] fixed formatting --- tracing-mock/src/field.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tracing-mock/src/field.rs b/tracing-mock/src/field.rs index a235bcc33b..b89e8f5703 100644 --- a/tracing-mock/src/field.rs +++ b/tracing-mock/src/field.rs @@ -324,7 +324,7 @@ impl ExpectedFields { /// Adds an additional [`ExpectedField`] to be matched. /// /// _All_ fields must match for the expectation to pass. If any of - /// them are not present, if any of the values differs, the + /// them are not present, if any of the values differs, the /// expectation will fail. /// /// This method performs the same function as From 1732ac7a2d37ae6d58bd8ffbd554cc49823f7bf4 Mon Sep 17 00:00:00 2001 From: Hayden Stainsby Date: Tue, 14 Nov 2023 11:12:37 +0100 Subject: [PATCH 9/9] reword `ExpectedField::and` description as per review It's better this way. --- tracing-mock/src/field.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tracing-mock/src/field.rs b/tracing-mock/src/field.rs index b89e8f5703..a374461375 100644 --- a/tracing-mock/src/field.rs +++ b/tracing-mock/src/field.rs @@ -201,9 +201,9 @@ impl ExpectedField { /// Adds an additional [`ExpectedField`] to be matched. /// - /// Both fields must match, if either of them are not present, or - /// if the value for either field is different, the expectation - /// will fail. + /// Any fields introduced by `.and` must also match. If any fields + /// are not present, or if the value for any field is different, + /// then the expectation will fail. /// /// # Examples ///