From d42a261699ec97a3b8b57403f105f42d199d7075 Mon Sep 17 00:00:00 2001 From: Lucas Kent Date: Wed, 15 Dec 2021 11:51:23 +1100 Subject: [PATCH 1/2] Fix pretty formatter thread id double space --- tracing-subscriber/src/fmt/format/mod.rs | 76 ++++++++++++++++++--- tracing-subscriber/src/fmt/format/pretty.rs | 9 ++- 2 files changed, 69 insertions(+), 16 deletions(-) diff --git a/tracing-subscriber/src/fmt/format/mod.rs b/tracing-subscriber/src/fmt/format/mod.rs index 6aa83ed0b3..1374b675a6 100644 --- a/tracing-subscriber/src/fmt/format/mod.rs +++ b/tracing-subscriber/src/fmt/format/mod.rs @@ -1468,6 +1468,8 @@ pub(super) mod test { use super::{FmtSpan, TimingDisplay, Writer}; use std::fmt; + use regex::Regex; + pub(crate) struct MockTime; impl FormatTime for MockTime { fn format_time(&self, w: &mut Writer<'_>) -> fmt::Result { @@ -1488,21 +1490,31 @@ pub(super) mod test { .with_thread_names(false); #[cfg(feature = "ansi")] let subscriber = subscriber.with_ansi(false); - run_test(subscriber, make_writer, "hello\n") + assert_info_hello(subscriber, make_writer, "hello\n") } #[cfg(feature = "ansi")] #[test] fn with_ansi_true() { let expected = "\u{1b}[2mfake time\u{1b}[0m \u{1b}[32m INFO\u{1b}[0m \u{1b}[2mtracing_subscriber::fmt::format::test\u{1b}[0m\u{1b}[2m:\u{1b}[0m hello\n"; - test_ansi(true, expected); + assert_info_hello_ansi(true, expected); } #[cfg(feature = "ansi")] #[test] fn with_ansi_false() { let expected = "fake time INFO tracing_subscriber::fmt::format::test: hello\n"; - test_ansi(false, expected); + assert_info_hello_ansi(false, expected); + } + + #[cfg(feature = "ansi")] + fn assert_info_hello_ansi(is_ansi: bool, expected: &str) { + let make_writer = MockMakeWriter::default(); + let subscriber = crate::fmt::Collector::builder() + .with_writer(make_writer.clone()) + .with_ansi(is_ansi) + .with_timer(MockTime); + assert_info_hello(subscriber, make_writer, expected) } #[cfg(not(feature = "ansi"))] @@ -1513,7 +1525,7 @@ pub(super) mod test { let subscriber = crate::fmt::Collector::builder() .with_writer(make_writer) .with_timer(MockTime); - run_test(subscriber, make_writer, expected); + assert_info_hello(subscriber, make_writer, expected); } #[test] @@ -1526,23 +1538,65 @@ pub(super) mod test { .with_timer(MockTime); let expected = "fake time tracing_subscriber::fmt::format::test: hello\n"; - run_test(subscriber, make_writer, expected); + assert_info_hello(subscriber, make_writer, expected); } - #[cfg(feature = "ansi")] - fn test_ansi(is_ansi: bool, expected: &str) { + #[test] + fn with_thread_ids() { let make_writer = MockMakeWriter::default(); let subscriber = crate::fmt::Collector::builder() .with_writer(make_writer.clone()) - .with_ansi(is_ansi) + .with_thread_ids(true) + .with_ansi(false) + .with_timer(MockTime); + let expected = + "fake time INFO ThreadId(NUMERIC) tracing_subscriber::fmt::format::test: hello\n"; + + assert_info_hello_ignore_numeric(subscriber, make_writer, expected); + } + + #[test] + fn pretty_default() { + let make_writer = MockMakeWriter::default(); + let subscriber = crate::fmt::Collector::builder() + .pretty() + .with_writer(make_writer.clone()) + .with_ansi(false) .with_timer(MockTime); - run_test(subscriber, make_writer, expected) + let expected = format!( + r#" fake time INFO tracing_subscriber::fmt::format::test: hello + at {}:NUMERIC + +"#, + file!() + ); + + assert_info_hello_ignore_numeric(subscriber, make_writer, &expected) + } + + fn assert_info_hello(subscriber: impl Into, buf: MockMakeWriter, expected: &str) { + let _default = set_default(&subscriber.into()); + tracing::info!("hello"); + let result = buf.get_string(); + + assert_eq!(expected, result) } - fn run_test(subscriber: impl Into, buf: MockMakeWriter, expected: &str) { + // When numeric characters are used they often form a non-deterministic value as they usually represent things like a thread id or line number. + // This assert method should be used when non-deterministic numeric characters are present. + fn assert_info_hello_ignore_numeric( + subscriber: impl Into, + buf: MockMakeWriter, + expected: &str, + ) { let _default = set_default(&subscriber.into()); tracing::info!("hello"); - assert_eq!(expected, buf.get_string()) + + let regex = Regex::new("[0-9]+").unwrap(); + let result = buf.get_string(); + let result_cleaned = regex.replace_all(&result, "NUMERIC"); + + assert_eq!(expected, result_cleaned) } #[test] diff --git a/tracing-subscriber/src/fmt/format/pretty.rs b/tracing-subscriber/src/fmt/format/pretty.rs index d8c67fb30c..5319148912 100644 --- a/tracing-subscriber/src/fmt/format/pretty.rs +++ b/tracing-subscriber/src/fmt/format/pretty.rs @@ -174,13 +174,12 @@ where if let Some(name) = thread.name() { write!(writer, "{}", name)?; if self.display_thread_id { - write!(writer, " ({:?})", thread.id())?; + write!(writer, " ")?; } - } else if !self.display_thread_id { - write!(writer, " {:?}", thread.id())?; } - } else if self.display_thread_id { - write!(writer, " {:?}", thread.id())?; + } + if self.display_thread_id { + write!(writer, "{:?}", thread.id())?; } writer.write_char('\n')?; } From 92d08a8ea8ceffbc284255e7e7c36dd65ecf4e22 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Wed, 22 Dec 2021 09:23:10 -0800 Subject: [PATCH 2/2] Update tracing-subscriber/src/fmt/format/pretty.rs --- tracing-subscriber/src/fmt/format/pretty.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tracing-subscriber/src/fmt/format/pretty.rs b/tracing-subscriber/src/fmt/format/pretty.rs index 5319148912..f67864d6db 100644 --- a/tracing-subscriber/src/fmt/format/pretty.rs +++ b/tracing-subscriber/src/fmt/format/pretty.rs @@ -174,7 +174,7 @@ where if let Some(name) = thread.name() { write!(writer, "{}", name)?; if self.display_thread_id { - write!(writer, " ")?; + writer.write_char(' ')?; } } }