diff --git a/DESIGN.md b/DESIGN.md index e34c55c..66eb7d9 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -37,6 +37,7 @@ Decisions - Terse and pretty progress indicators are too nebulous to render (see their notifiers) - There is likely not enough value add in the failure message - This puts more of a burden on custom test harnesses for their implementation than is strictly needed +- Report failures separate from test-complete so we can have multiple ### Prior Art diff --git a/crates/libtest-json/event.schema.json b/crates/libtest-json/event.schema.json index 6e1a40e..efff322 100644 --- a/crates/libtest-json/event.schema.json +++ b/crates/libtest-json/event.schema.json @@ -67,6 +67,19 @@ "event" ] }, + { + "type": "object", + "properties": { + "event": { + "type": "string", + "const": "case_message" + } + }, + "$ref": "#/$defs/CaseMessage", + "required": [ + "event" + ] + }, { "type": "object", "properties": { @@ -200,35 +213,49 @@ "name" ] }, - "RunStatus": { + "MessageKind": { "type": "string", "enum": [ - "ignored", - "failed" + "error", + "ignored" ] }, - "CaseComplete": { + "CaseMessage": { "type": "object", "properties": { "name": { "type": "string" }, - "status": { - "description": "`None` means success", + "kind": { + "$ref": "#/$defs/MessageKind" + }, + "message": { + "type": [ + "string", + "null" + ] + }, + "elapsed_s": { "anyOf": [ { - "$ref": "#/$defs/RunStatus" + "$ref": "#/$defs/Elapsed" }, { "type": "null" } ] - }, - "message": { - "type": [ - "string", - "null" - ] + } + }, + "required": [ + "name", + "kind" + ] + }, + "CaseComplete": { + "type": "object", + "properties": { + "name": { + "type": "string" }, "elapsed_s": { "anyOf": [ diff --git a/crates/libtest-json/src/event.rs b/crates/libtest-json/src/event.rs index 5eb9c9c..68132f5 100644 --- a/crates/libtest-json/src/event.rs +++ b/crates/libtest-json/src/event.rs @@ -9,6 +9,7 @@ pub enum Event { DiscoverComplete(DiscoverComplete), RunStart(RunStart), CaseStart(CaseStart), + CaseMessage(CaseMessage), CaseComplete(CaseComplete), RunComplete(RunComplete), } @@ -22,6 +23,7 @@ impl Event { Self::DiscoverComplete(event) => event.to_jsonline(), Self::RunStart(event) => event.to_jsonline(), Self::CaseStart(event) => event.to_jsonline(), + Self::CaseMessage(event) => event.to_jsonline(), Self::CaseComplete(event) => event.to_jsonline(), Self::RunComplete(event) => event.to_jsonline(), } @@ -58,6 +60,12 @@ impl From for Event { } } +impl From for Event { + fn from(inner: CaseMessage) -> Self { + Self::CaseMessage(inner) + } +} + impl From for Event { fn from(inner: CaseComplete) -> Self { Self::CaseComplete(inner) @@ -296,14 +304,9 @@ impl CaseStart { #[cfg_attr(feature = "unstable-schema", derive(schemars::JsonSchema))] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[cfg_attr(feature = "serde", serde(rename_all = "snake_case"))] -pub struct CaseComplete { +pub struct CaseMessage { pub name: String, - /// `None` means success - #[cfg_attr( - feature = "serde", - serde(default, skip_serializing_if = "Option::is_none") - )] - pub status: Option, + pub kind: MessageKind, #[cfg_attr( feature = "serde", serde(default, skip_serializing_if = "Option::is_none") @@ -316,7 +319,7 @@ pub struct CaseComplete { pub elapsed_s: Option, } -impl CaseComplete { +impl CaseMessage { #[cfg(feature = "json")] pub fn to_jsonline(&self) -> String { use json_write::JsonWrite as _; @@ -326,19 +329,17 @@ impl CaseComplete { buffer.key("event").unwrap(); buffer.keyval_sep().unwrap(); - buffer.value("case_complete").unwrap(); + buffer.value("case_message").unwrap(); buffer.val_sep().unwrap(); buffer.key("name").unwrap(); buffer.keyval_sep().unwrap(); buffer.value(&self.name).unwrap(); - if let Some(status) = self.status { - buffer.val_sep().unwrap(); - buffer.key("status").unwrap(); - buffer.keyval_sep().unwrap(); - buffer.value(status.as_str()).unwrap(); - } + buffer.val_sep().unwrap(); + buffer.key("kind").unwrap(); + buffer.keyval_sep().unwrap(); + buffer.value(self.kind.as_str()).unwrap(); if let Some(message) = &self.message { buffer.val_sep().unwrap(); @@ -360,6 +361,49 @@ impl CaseComplete { } } +#[derive(Clone, Debug)] +#[cfg_attr(feature = "unstable-schema", derive(schemars::JsonSchema))] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +#[cfg_attr(feature = "serde", serde(rename_all = "snake_case"))] +pub struct CaseComplete { + pub name: String, + #[cfg_attr( + feature = "serde", + serde(default, skip_serializing_if = "Option::is_none") + )] + pub elapsed_s: Option, +} + +impl CaseComplete { + #[cfg(feature = "json")] + pub fn to_jsonline(&self) -> String { + use json_write::JsonWrite as _; + + let mut buffer = String::new(); + buffer.open_object().unwrap(); + + buffer.key("event").unwrap(); + buffer.keyval_sep().unwrap(); + buffer.value("case_complete").unwrap(); + + buffer.val_sep().unwrap(); + buffer.key("name").unwrap(); + buffer.keyval_sep().unwrap(); + buffer.value(&self.name).unwrap(); + + if let Some(elapsed_s) = self.elapsed_s { + buffer.val_sep().unwrap(); + buffer.key("elapsed_s").unwrap(); + buffer.keyval_sep().unwrap(); + buffer.value(String::from(elapsed_s)).unwrap(); + } + + buffer.close_object().unwrap(); + + buffer + } +} + #[derive(Clone, Debug)] #[cfg_attr(feature = "unstable-schema", derive(schemars::JsonSchema))] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] @@ -397,10 +441,12 @@ impl RunComplete { } } +#[cfg(feature = "serde")] fn true_default() -> bool { true } +#[cfg(feature = "serde")] fn is_true(yes: &bool) -> bool { *yes } @@ -423,25 +469,27 @@ impl RunMode { } } + #[cfg(any(feature = "serde", feature = "json"))] fn is_default(&self) -> bool { *self == Default::default() } } -#[derive(Copy, Clone, Debug, PartialEq, Eq)] +#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] #[cfg_attr(feature = "unstable-schema", derive(schemars::JsonSchema))] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[cfg_attr(feature = "serde", serde(rename_all = "snake_case"))] -pub enum RunStatus { +pub enum MessageKind { + // Highest precedent items for determining test status last + Error, Ignored, - Failed, } -impl RunStatus { +impl MessageKind { pub fn as_str(&self) -> &str { match self { + Self::Error => "error", Self::Ignored => "ignored", - Self::Failed => "failed", } } } diff --git a/crates/libtest-json/src/lib.rs b/crates/libtest-json/src/lib.rs index 2a20c9a..0075c61 100644 --- a/crates/libtest-json/src/lib.rs +++ b/crates/libtest-json/src/lib.rs @@ -9,8 +9,8 @@ pub mod event; pub use event::Elapsed; pub use event::Event; +pub use event::MessageKind; pub use event::RunMode; -pub use event::RunStatus; #[doc = include_str!("../README.md")] #[cfg(doctest)] diff --git a/crates/libtest-json/tests/roundtrip.rs b/crates/libtest-json/tests/roundtrip.rs index 4f33bbc..13381d6 100644 --- a/crates/libtest-json/tests/roundtrip.rs +++ b/crates/libtest-json/tests/roundtrip.rs @@ -102,30 +102,49 @@ fn case_start() { } #[test] -fn case_complete() { +fn case_message() { t( - libtest_json::event::CaseComplete { + libtest_json::event::CaseMessage { name: "Hello\tworld!".to_owned(), - status: None, + kind: libtest_json::MessageKind::Error, message: None, elapsed_s: None, }, - str![[r#"{"event":"case_complete","name":"Hello\tworld!"}"#]], + str![[r#"{"event":"case_message","name":"Hello\tworld!","kind":"error"}"#]], ); t( - libtest_json::event::CaseComplete { + libtest_json::event::CaseMessage { name: "Hello\tworld!".to_owned(), - status: Some(libtest_json::RunStatus::Ignored), + kind: libtest_json::MessageKind::Ignored, message: Some("This\tfailed".to_owned()), elapsed_s: Some(libtest_json::Elapsed(Default::default())), }, str![[ - r#"{"event":"case_complete","name":"Hello\tworld!","status":"ignored","message":"This\tfailed","elapsed_s":"0"}"# + r#"{"event":"case_message","name":"Hello\tworld!","kind":"ignored","message":"This\tfailed","elapsed_s":"0"}"# ]], ); } +#[test] +fn case_complete() { + t( + libtest_json::event::CaseComplete { + name: "Hello\tworld!".to_owned(), + elapsed_s: None, + }, + str![[r#"{"event":"case_complete","name":"Hello\tworld!"}"#]], + ); + + t( + libtest_json::event::CaseComplete { + name: "Hello\tworld!".to_owned(), + elapsed_s: Some(libtest_json::Elapsed(Default::default())), + }, + str![[r#"{"event":"case_complete","name":"Hello\tworld!","elapsed_s":"0"}"#]], + ); +} + #[test] fn suite_complete() { t( diff --git a/crates/libtest2-harness/src/case.rs b/crates/libtest2-harness/src/case.rs index 7dc9e94..d7639f5 100644 --- a/crates/libtest2-harness/src/case.rs +++ b/crates/libtest2-harness/src/case.rs @@ -53,14 +53,14 @@ pub type RunResult = Result<(), RunError>; #[derive(Debug)] pub struct RunError { - status: notify::RunStatus, + status: notify::MessageKind, cause: Option>, } impl RunError { pub fn with_cause(cause: impl std::error::Error + Send + Sync + 'static) -> Self { Self { - status: notify::RunStatus::Failed, + status: notify::MessageKind::Error, cause: Some(Box::new(cause)), } } @@ -71,19 +71,19 @@ impl RunError { pub(crate) fn ignore() -> Self { Self { - status: notify::RunStatus::Ignored, + status: notify::MessageKind::Ignored, cause: None, } } pub(crate) fn ignore_for(reason: String) -> Self { Self { - status: notify::RunStatus::Ignored, + status: notify::MessageKind::Ignored, cause: Some(Box::new(Message(reason))), } } - pub(crate) fn status(&self) -> notify::RunStatus { + pub(crate) fn status(&self) -> notify::MessageKind { self.status } diff --git a/crates/libtest2-harness/src/harness.rs b/crates/libtest2-harness/src/harness.rs index 49a4917..c823114 100644 --- a/crates/libtest2-harness/src/harness.rs +++ b/crates/libtest2-harness/src/harness.rs @@ -310,11 +310,26 @@ fn run( } impl RunningTest { - fn join(self, event: &mut notify::event::CaseComplete) { - if self.join_handle.join().is_err() && event.status.is_none() { - event.status = Some(notify::RunStatus::Failed); - event.message = Some("panicked after reporting success".to_owned()); + fn join( + self, + start: &std::time::Instant, + event: ¬ify::event::CaseComplete, + notifier: &mut dyn notify::Notifier, + ) -> std::io::Result<()> { + if self.join_handle.join().is_err() { + let kind = notify::MessageKind::Error; + let message = Some("panicked after reporting success".to_owned()); + notifier.notify( + notify::event::CaseMessage { + name: event.name.clone(), + kind, + message, + elapsed_s: Some(notify::Elapsed(start.elapsed())), + } + .into(), + )?; } + Ok(()) } } @@ -379,10 +394,10 @@ fn run( } } - let mut event = rx.recv().unwrap(); - if let notify::Event::CaseComplete(event) = &mut event { + let event = rx.recv().unwrap(); + if let notify::Event::CaseComplete(event) = &event { let running_test = running_tests.remove(&event.name).unwrap(); - running_test.join(event); + running_test.join(start, event, notifier)?; pending -= 1; } notifier.notify(event)?; @@ -446,20 +461,31 @@ fn run_case( Err(RunError::fail(msg)) }); - let err = outcome.as_ref().err(); - let status = err.map(|e| e.status()); - let message = err.and_then(|e| e.cause().map(|c| c.to_string())); + let mut case_status = None; + if let Some(err) = outcome.as_ref().err() { + let kind = err.status(); + case_status = Some(kind); + let message = err.cause().map(|c| c.to_string()); + notifier.notify( + notify::event::CaseMessage { + name: case.name().to_owned(), + kind, + message, + elapsed_s: Some(notify::Elapsed(start.elapsed())), + } + .into(), + )?; + } + notifier.notify( notify::event::CaseComplete { name: case.name().to_owned(), - status, - message, elapsed_s: Some(notify::Elapsed(start.elapsed())), } .into(), )?; - Ok(status != Some(notify::RunStatus::Failed)) + Ok(case_status != Some(notify::MessageKind::Error)) } /// Fixed frame used to clean the backtrace with `RUST_BACKTRACE=1`. diff --git a/crates/libtest2-harness/src/notify/pretty.rs b/crates/libtest2-harness/src/notify/pretty.rs index 2a4583a..515397b 100644 --- a/crates/libtest2-harness/src/notify/pretty.rs +++ b/crates/libtest2-harness/src/notify/pretty.rs @@ -1,5 +1,5 @@ use super::Event; -use super::RunStatus; +use super::MessageKind; use super::FAILED; use super::IGNORED; use super::OK; @@ -51,11 +51,12 @@ impl super::Notifier for PrettyRunNotifier { self.writer.flush()?; } } + Event::CaseMessage(_) => {} Event::CaseComplete(inner) => { - let status = self.summary.get_status(&inner.name); + let status = self.summary.get_kind(&inner.name); let (s, style) = match status { - Some(RunStatus::Ignored) => ("ignored", IGNORED), - Some(RunStatus::Failed) => ("FAILED", FAILED), + Some(MessageKind::Ignored) => ("ignored", IGNORED), + Some(MessageKind::Error) => ("FAILED", FAILED), None => ("ok", OK), }; diff --git a/crates/libtest2-harness/src/notify/summary.rs b/crates/libtest2-harness/src/notify/summary.rs index 0c4bf09..4f0b956 100644 --- a/crates/libtest2-harness/src/notify/summary.rs +++ b/crates/libtest2-harness/src/notify/summary.rs @@ -1,6 +1,6 @@ -use super::event::CaseComplete; +use super::event::CaseMessage; use super::Event; -use super::RunStatus; +use super::MessageKind; use super::FAILED; use super::OK; @@ -11,14 +11,14 @@ pub(crate) struct Summary { /// filter-in pattern or by `--skip` arguments). num_filtered_out: usize, - status: std::collections::HashMap, + status: std::collections::HashMap, elapsed_s: Option, } impl Summary { - pub(crate) fn get_status(&self, name: &str) -> Option { - let event = self.status.get(name)?; - event.status + pub(crate) fn get_kind(&self, name: &str) -> Option { + let status = self.status.get(name)?; + find_run_status(status) } pub(crate) fn write_start(&self, writer: &mut dyn std::io::Write) -> std::io::Result<()> { @@ -34,12 +34,27 @@ impl Summary { let mut num_failed = 0; let mut num_ignored = 0; let mut failures = std::collections::BTreeMap::new(); - for event in self.status.values() { - match event.status { - Some(RunStatus::Ignored) => num_ignored += 1, - Some(RunStatus::Failed) => { + for (name, case_status) in &self.status { + let mut status = find_run_status(case_status); + if !case_status.started { + // Even override `Ignored` + status = Some(MessageKind::Error); + failures.insert(name, Some("test found that never started")); + } + if !case_status.completed { + // Even override `Ignored` + status = Some(MessageKind::Error); + failures.insert(name, Some("test never completed")); + } + match status { + Some(MessageKind::Ignored) => num_ignored += 1, + Some(MessageKind::Error) => { num_failed += 1; - failures.insert(&event.name, &event.message); + for event in &case_status.messages { + if Some(event.kind) == status { + failures.insert(name, event.message.as_deref()); + } + } } None => num_passed += 1, } @@ -106,9 +121,18 @@ impl super::Notifier for Summary { } Event::DiscoverComplete(_) => {} Event::RunStart(_) => {} - Event::CaseStart(_) => {} + Event::CaseStart(inner) => { + self.status.entry(inner.name).or_default().started = true; + } + Event::CaseMessage(inner) => { + self.status + .entry(inner.name.clone()) + .or_default() + .messages + .push(inner); + } Event::CaseComplete(inner) => { - self.status.insert(inner.name.clone(), inner); + self.status.entry(inner.name).or_default().completed = true; } Event::RunComplete(inner) => { self.elapsed_s = inner.elapsed_s; @@ -117,3 +141,18 @@ impl super::Notifier for Summary { Ok(()) } } + +fn find_run_status(case_status: &CaseStatus) -> Option { + let mut status = None; + for event in &case_status.messages { + status = status.max(Some(event.kind)); + } + status +} + +#[derive(Default, Clone, Debug)] +struct CaseStatus { + messages: Vec, + started: bool, + completed: bool, +} diff --git a/crates/libtest2-harness/src/notify/terse.rs b/crates/libtest2-harness/src/notify/terse.rs index a901131..ac212bb 100644 --- a/crates/libtest2-harness/src/notify/terse.rs +++ b/crates/libtest2-harness/src/notify/terse.rs @@ -1,5 +1,5 @@ use super::Event; -use super::RunStatus; +use super::MessageKind; use super::FAILED; use super::IGNORED; use super::OK; @@ -35,6 +35,7 @@ impl super::Notifier for TerseListNotifier { } Event::RunStart(_) => {} Event::CaseStart(_) => {} + Event::CaseMessage(_) => {} Event::CaseComplete(_) => {} Event::RunComplete(_) => {} } @@ -68,11 +69,12 @@ impl super::Notifier for TerseRunNotifier { self.summary.write_start(&mut self.writer)?; } Event::CaseStart(_) => {} + Event::CaseMessage(_) => {} Event::CaseComplete(inner) => { - let status = self.summary.get_status(&inner.name); + let status = self.summary.get_kind(&inner.name); let (c, style) = match status { - Some(RunStatus::Ignored) => ('i', IGNORED), - Some(RunStatus::Failed) => ('F', FAILED), + Some(MessageKind::Ignored) => ('i', IGNORED), + Some(MessageKind::Error) => ('F', FAILED), None => ('.', OK), }; write!(self.writer, "{style}{c}{style:#}")?; diff --git a/crates/libtest2-mimic/tests/testsuite/mixed_bag.rs b/crates/libtest2-mimic/tests/testsuite/mixed_bag.rs index 0c26e07..7e36001 100644 --- a/crates/libtest2-mimic/tests/testsuite/mixed_bag.rs +++ b/crates/libtest2-mimic/tests/testsuite/mixed_bag.rs @@ -898,12 +898,17 @@ fn test_json() { "elapsed_s": "[..]" }, { - "event": "case_complete", + "event": "case_message", "name": "bear", - "status": "ignored", + "kind": "ignored", "message": "fails", "elapsed_s": "[..]" }, + { + "event": "case_complete", + "name": "bear", + "elapsed_s": "[..]" + }, { "event": "case_start", "name": "cat", @@ -928,13 +933,6 @@ fn test_json() { "elapsed_s": "[..]", "event": "discover_complete" }, - { - "elapsed_s": "[..]", - "event": "case_complete", - "message": "fails", - "name": "bear", - "status": "ignored" - }, { "elapsed_s": "[..]", "event": "case_complete", @@ -1007,6 +1005,18 @@ fn test_json() { "name": "owl", "selected": false, "elapsed_s": "[..]" + }, + { + "event": "case_complete", + "name": "bear", + "elapsed_s": "[..]" + }, + { + "event": "case_message", + "name": "bear", + "kind": "ignored", + "message": "fails", + "elapsed_s": "[..]" } ] "#]] @@ -1157,24 +1167,34 @@ fn fail_fast_json() { "elapsed_s": "[..]" }, { - "event": "case_complete", + "event": "case_message", "name": "bear", - "status": "ignored", + "kind": "ignored", "message": "fails", "elapsed_s": "[..]" }, + { + "event": "case_complete", + "name": "bear", + "elapsed_s": "[..]" + }, { "event": "case_start", "name": "bunny", "elapsed_s": "[..]" }, { - "event": "case_complete", + "event": "case_message", "name": "bunny", - "status": "ignored", + "kind": "ignored", "message": "fails", "elapsed_s": "[..]" }, + { + "event": "case_complete", + "name": "bunny", + "elapsed_s": "[..]" + }, { "event": "case_start", "name": "cat", @@ -1191,12 +1211,17 @@ fn fail_fast_json() { "elapsed_s": "[..]" }, { - "event": "case_complete", + "event": "case_message", "name": "dog", - "status": "failed", + "kind": "error", "message": "was not a good boy", "elapsed_s": "[..]" }, + { + "event": "case_complete", + "name": "dog", + "elapsed_s": "[..]" + }, { "event": "run_complete", "elapsed_s": "[..]" diff --git a/crates/libtest2/tests/testsuite/mixed_bag.rs b/crates/libtest2/tests/testsuite/mixed_bag.rs index 89fc58d..fc88276 100644 --- a/crates/libtest2/tests/testsuite/mixed_bag.rs +++ b/crates/libtest2/tests/testsuite/mixed_bag.rs @@ -905,12 +905,17 @@ fn test_json() { "elapsed_s": "[..]" }, { - "event": "case_complete", + "event": "case_message", "name": "bear", - "status": "ignored", + "kind": "ignored", "message": "fails", "elapsed_s": "[..]" }, + { + "event": "case_complete", + "name": "bear", + "elapsed_s": "[..]" + }, { "event": "case_start", "name": "cat", @@ -935,13 +940,6 @@ fn test_json() { "elapsed_s": "[..]", "event": "discover_complete" }, - { - "elapsed_s": "[..]", - "event": "case_complete", - "message": "fails", - "name": "bear", - "status": "ignored" - }, { "elapsed_s": "[..]", "event": "case_complete", @@ -1014,6 +1012,18 @@ fn test_json() { "name": "owl", "selected": false, "elapsed_s": "[..]" + }, + { + "event": "case_complete", + "name": "bear", + "elapsed_s": "[..]" + }, + { + "event": "case_message", + "name": "bear", + "kind": "ignored", + "message": "fails", + "elapsed_s": "[..]" } ] "#]] @@ -1164,24 +1174,34 @@ fn fail_fast_json() { "elapsed_s": "[..]" }, { - "event": "case_complete", + "event": "case_message", "name": "bear", - "status": "ignored", + "kind": "ignored", "message": "fails", "elapsed_s": "[..]" }, + { + "event": "case_complete", + "name": "bear", + "elapsed_s": "[..]" + }, { "event": "case_start", "name": "bunny", "elapsed_s": "[..]" }, { - "event": "case_complete", + "event": "case_message", "name": "bunny", - "status": "ignored", + "kind": "ignored", "message": "fails", "elapsed_s": "[..]" }, + { + "event": "case_complete", + "name": "bunny", + "elapsed_s": "[..]" + }, { "event": "case_start", "name": "cat", @@ -1198,12 +1218,17 @@ fn fail_fast_json() { "elapsed_s": "[..]" }, { - "event": "case_complete", + "event": "case_message", "name": "dog", - "status": "failed", + "kind": "error", "message": "was not a good boy", "elapsed_s": "[..]" }, + { + "event": "case_complete", + "name": "dog", + "elapsed_s": "[..]" + }, { "event": "run_complete", "elapsed_s": "[..]"