From dc06b18121334b46227135e24a4a04bed897f36c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Sat, 27 Jan 2024 19:19:15 +0100 Subject: [PATCH] Revert "refactor(cli): use new sanitizer for resources (#22125)" This reverts commit 84fb2ad71b94ce773cefcbc868b160a73c52ab47. --- cli/js/40_test.js | 183 +++++++++++++++++- cli/lsp/testing/execution.rs | 4 +- cli/ops/testing.rs | 4 - .../testdata/test/resource_sanitizer.out | 6 +- cli/tools/test/fmt.rs | 139 ------------- cli/tools/test/mod.rs | 83 +------- cli/tools/test/reporters/common.rs | 12 +- cli/tools/test/reporters/dot.rs | 6 +- cli/tools/test/reporters/pretty.rs | 6 +- 9 files changed, 204 insertions(+), 239 deletions(-) diff --git a/cli/js/40_test.js b/cli/js/40_test.js index 146163fdd1ae29..b6f966f2003a40 100644 --- a/cli/js/40_test.js +++ b/cli/js/40_test.js @@ -15,8 +15,10 @@ const { MapPrototypeGet, MapPrototypeHas, MapPrototypeSet, + ObjectKeys, Promise, SafeArrayIterator, + Set, SymbolToStringTag, TypeError, } = primordials; @@ -340,6 +342,182 @@ function assertOps(fn) { }; } +function prettyResourceNames(name) { + switch (name) { + case "fsFile": + return ["A file", "opened", "closed"]; + case "fetchRequest": + return ["A fetch request", "started", "finished"]; + case "fetchRequestBody": + return ["A fetch request body", "created", "closed"]; + case "fetchResponse": + return ["A fetch response body", "created", "consumed"]; + case "httpClient": + return ["An HTTP client", "created", "closed"]; + case "dynamicLibrary": + return ["A dynamic library", "loaded", "unloaded"]; + case "httpConn": + return ["An inbound HTTP connection", "accepted", "closed"]; + case "httpStream": + return ["An inbound HTTP request", "accepted", "closed"]; + case "tcpStream": + return ["A TCP connection", "opened/accepted", "closed"]; + case "unixStream": + return ["A Unix connection", "opened/accepted", "closed"]; + case "tlsStream": + return ["A TLS connection", "opened/accepted", "closed"]; + case "tlsListener": + return ["A TLS listener", "opened", "closed"]; + case "unixListener": + return ["A Unix listener", "opened", "closed"]; + case "unixDatagram": + return ["A Unix datagram", "opened", "closed"]; + case "tcpListener": + return ["A TCP listener", "opened", "closed"]; + case "udpSocket": + return ["A UDP socket", "opened", "closed"]; + case "timer": + return ["A timer", "started", "fired/cleared"]; + case "textDecoder": + return ["A text decoder", "created", "finished"]; + case "messagePort": + return ["A message port", "created", "closed"]; + case "webSocketStream": + return ["A WebSocket", "opened", "closed"]; + case "fsEvents": + return ["A file system watcher", "created", "closed"]; + case "childStdin": + return ["A child process stdin", "opened", "closed"]; + case "childStdout": + return ["A child process stdout", "opened", "closed"]; + case "childStderr": + return ["A child process stderr", "opened", "closed"]; + case "child": + return ["A child process", "started", "closed"]; + case "signal": + return ["A signal listener", "created", "fired/cleared"]; + case "stdin": + return ["The stdin pipe", "opened", "closed"]; + case "stdout": + return ["The stdout pipe", "opened", "closed"]; + case "stderr": + return ["The stderr pipe", "opened", "closed"]; + case "compression": + return ["A CompressionStream", "created", "closed"]; + default: + return [`A "${name}" resource`, "created", "cleaned up"]; + } +} + +function resourceCloseHint(name) { + switch (name) { + case "fsFile": + return "Close the file handle by calling `file.close()`."; + case "fetchRequest": + return "Await the promise returned from `fetch()` or abort the fetch with an abort signal."; + case "fetchRequestBody": + return "Terminate the request body `ReadableStream` by closing or erroring it."; + case "fetchResponse": + return "Consume or close the response body `ReadableStream`, e.g `await resp.text()` or `await resp.body.cancel()`."; + case "httpClient": + return "Close the HTTP client by calling `httpClient.close()`."; + case "dynamicLibrary": + return "Unload the dynamic library by calling `dynamicLibrary.close()`."; + case "httpConn": + return "Close the inbound HTTP connection by calling `httpConn.close()`."; + case "httpStream": + return "Close the inbound HTTP request by responding with `e.respondWith()` or closing the HTTP connection."; + case "tcpStream": + return "Close the TCP connection by calling `tcpConn.close()`."; + case "unixStream": + return "Close the Unix socket connection by calling `unixConn.close()`."; + case "tlsStream": + return "Close the TLS connection by calling `tlsConn.close()`."; + case "tlsListener": + return "Close the TLS listener by calling `tlsListener.close()`."; + case "unixListener": + return "Close the Unix socket listener by calling `unixListener.close()`."; + case "unixDatagram": + return "Close the Unix datagram socket by calling `unixDatagram.close()`."; + case "tcpListener": + return "Close the TCP listener by calling `tcpListener.close()`."; + case "udpSocket": + return "Close the UDP socket by calling `udpSocket.close()`."; + case "timer": + return "Clear the timer by calling `clearInterval` or `clearTimeout`."; + case "textDecoder": + return "Close the text decoder by calling `textDecoder.decode('')` or `await textDecoderStream.readable.cancel()`."; + case "messagePort": + return "Close the message port by calling `messagePort.close()`."; + case "webSocketStream": + return "Close the WebSocket by calling `webSocket.close()`."; + case "fsEvents": + return "Close the file system watcher by calling `watcher.close()`."; + case "childStdin": + return "Close the child process stdin by calling `proc.stdin.close()`."; + case "childStdout": + return "Close the child process stdout by calling `proc.stdout.close()` or `await child.stdout.cancel()`."; + case "childStderr": + return "Close the child process stderr by calling `proc.stderr.close()` or `await child.stderr.cancel()`."; + case "child": + return "Close the child process by calling `proc.kill()` or `proc.close()`."; + case "signal": + return "Clear the signal listener by calling `Deno.removeSignalListener`."; + case "stdin": + return "Close the stdin pipe by calling `Deno.stdin.close()`."; + case "stdout": + return "Close the stdout pipe by calling `Deno.stdout.close()`."; + case "stderr": + return "Close the stderr pipe by calling `Deno.stderr.close()`."; + case "compression": + return "Close the compression stream by calling `await stream.writable.close()`."; + default: + return "Close the resource before the end of the test."; + } +} + +// Wrap test function in additional assertion that makes sure +// the test case does not "leak" resources - ie. resource table after +// the test has exactly the same contents as before the test. +function assertResources(fn) { + /** @param desc {TestDescription | TestStepDescription} */ + return async function resourceSanitizer(desc) { + const pre = core.resources(); + const innerResult = await fn(desc); + if (innerResult) return innerResult; + const post = core.resources(); + + const allResources = new Set([ + ...new SafeArrayIterator(ObjectKeys(pre)), + ...new SafeArrayIterator(ObjectKeys(post)), + ]); + + const details = []; + for (const resource of allResources) { + const preResource = pre[resource]; + const postResource = post[resource]; + if (preResource === postResource) continue; + + if (preResource === undefined) { + const [name, action1, action2] = prettyResourceNames(postResource); + const hint = resourceCloseHint(postResource); + const detail = + `${name} (rid ${resource}) was ${action1} during the test, but not ${action2} during the test. ${hint}`; + ArrayPrototypePush(details, detail); + } else { + const [name, action1, action2] = prettyResourceNames(preResource); + const detail = + `${name} (rid ${resource}) was ${action1} before the test started, but was ${action2} during the test. Do not close resources in a test that were not created during that test.`; + ArrayPrototypePush(details, detail); + } + } + if (details.length == 0) { + return null; + } + return { failed: { leakedResources: details } }; + }; +} + // Wrap test function in additional assertion that makes sure // that the test case does not accidentally exit prematurely. function assertExit(fn, isTest) { @@ -542,8 +720,6 @@ function testInner( testDesc.name, testDesc.ignore, testDesc.only, - false, /*testDesc.sanitizeOps*/ - testDesc.sanitizeResources, testDesc.location.fileName, testDesc.location.lineNumber, testDesc.location.columnNumber, @@ -734,6 +910,9 @@ function wrapTest(desc) { if (desc.sanitizeOps) { testFn = assertOps(testFn); } + if (desc.sanitizeResources) { + testFn = assertResources(testFn); + } if (desc.sanitizeExit) { testFn = assertExit(testFn, true); } diff --git a/cli/lsp/testing/execution.rs b/cli/lsp/testing/execution.rs index 11882e6af3dbad..69f218be851a1a 100644 --- a/cli/lsp/testing/execution.rs +++ b/cli/lsp/testing/execution.rs @@ -363,9 +363,7 @@ impl TestRun { test::TestResult::Ignored => summary.ignored += 1, test::TestResult::Failed(error) => { summary.failed += 1; - summary - .failures - .push(((&description).into(), error.clone())); + summary.failures.push((description.clone(), error.clone())); } test::TestResult::Cancelled => { summary.failed += 1; diff --git a/cli/ops/testing.rs b/cli/ops/testing.rs index 4a2f4818141865..28c4145ea4b3e2 100644 --- a/cli/ops/testing.rs +++ b/cli/ops/testing.rs @@ -123,8 +123,6 @@ fn op_register_test( #[string] name: String, ignore: bool, only: bool, - sanitize_ops: bool, - sanitize_resources: bool, #[string] file_name: String, #[smi] line_number: u32, #[smi] column_number: u32, @@ -143,8 +141,6 @@ fn op_register_test( name, ignore, only, - sanitize_ops, - sanitize_resources, origin: origin.clone(), location: TestLocation { file_name, diff --git a/cli/tests/testdata/test/resource_sanitizer.out b/cli/tests/testdata/test/resource_sanitizer.out index 655dd2a17157d4..a0e91665414a5b 100644 --- a/cli/tests/testdata/test/resource_sanitizer.out +++ b/cli/tests/testdata/test/resource_sanitizer.out @@ -6,10 +6,8 @@ leak ... FAILED ([WILDCARD]) leak => ./test/resource_sanitizer.ts:[WILDCARD] error: Leaking resources: -[UNORDERED_START] - - The stdin pipe was opened before the test started, but was closed during the test. Do not close resources in a test that were not created during that test. - - A file was opened during the test, but not closed during the test. Close the file handle by calling `file.close()`. -[UNORDERED_END] + - The stdin pipe (rid 0) was opened before the test started, but was closed during the test. Do not close resources in a test that were not created during that test. + - A file (rid 3) was opened during the test, but not closed during the test. Close the file handle by calling `file.close()`. FAILURES diff --git a/cli/tools/test/fmt.rs b/cli/tools/test/fmt.rs index 68adcbef196025..2b6defeac619cd 100644 --- a/cli/tools/test/fmt.rs +++ b/cli/tools/test/fmt.rs @@ -1,11 +1,5 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. -use deno_core::stats::RuntimeActivity; -use deno_core::stats::RuntimeActivityDiff; -use deno_core::stats::RuntimeActivityType; -use std::borrow::Cow; -use std::ops::AddAssign; - use super::*; pub fn to_relative_path_or_remote_url(cwd: &Url, path_or_url: &str) -> String { @@ -80,136 +74,3 @@ pub fn format_test_error(js_error: &JsError) -> String { .to_string(); format_js_error(&js_error) } - -pub fn format_sanitizer_diff(diff: RuntimeActivityDiff) -> Vec { - let mut output = format_sanitizer_accum(diff.appeared, true); - output.extend(format_sanitizer_accum(diff.disappeared, false)); - output.sort(); - output -} - -fn format_sanitizer_accum( - activities: Vec, - appeared: bool, -) -> Vec { - let mut accum = HashMap::new(); - for activity in activities { - let item = format_sanitizer_accum_item(activity); - accum.entry(item).or_insert(0).add_assign(1); - } - - let mut output = vec![]; - for ((item_type, item_name), count) in accum.into_iter() { - if item_type == RuntimeActivityType::Resource { - // TODO(mmastrac): until we implement the new timers and op sanitization, these must be ignored in this path - if item_name == "timer" { - continue; - } - let (name, action1, action2) = pretty_resource_name(&item_name); - let hint = resource_close_hint(&item_name); - - if appeared { - output.push(format!("{name} was {action1} during the test, but not {action2} during the test. {hint}")); - } else { - output.push(format!("{name} was {action1} before the test started, but was {action2} during the test. \ - Do not close resources in a test that were not created during that test.")); - } - } else { - // TODO(mmastrac): this will be done in a later PR - unimplemented!( - "Unhandled diff: {appeared} {} {:?} {}", - count, - item_type, - item_name - ); - } - } - output -} - -fn format_sanitizer_accum_item( - activity: RuntimeActivity, -) -> (RuntimeActivityType, Cow<'static, str>) { - let activity_type = activity.activity(); - match activity { - RuntimeActivity::AsyncOp(_, name) => (activity_type, name.into()), - RuntimeActivity::Interval(_) => (activity_type, "".into()), - RuntimeActivity::Resource(_, name) => (activity_type, name.into()), - RuntimeActivity::Timer(_) => (activity_type, "".into()), - } -} - -fn pretty_resource_name( - name: &str, -) -> (Cow<'static, str>, &'static str, &'static str) { - let (name, action1, action2) = match name { - "fsFile" => ("A file", "opened", "closed"), - "fetchRequest" => ("A fetch request", "started", "finished"), - "fetchRequestBody" => ("A fetch request body", "created", "closed"), - "fetchResponse" => ("A fetch response body", "created", "consumed"), - "httpClient" => ("An HTTP client", "created", "closed"), - "dynamicLibrary" => ("A dynamic library", "loaded", "unloaded"), - "httpConn" => ("An inbound HTTP connection", "accepted", "closed"), - "httpStream" => ("An inbound HTTP request", "accepted", "closed"), - "tcpStream" => ("A TCP connection", "opened/accepted", "closed"), - "unixStream" => ("A Unix connection", "opened/accepted", "closed"), - "tlsStream" => ("A TLS connection", "opened/accepted", "closed"), - "tlsListener" => ("A TLS listener", "opened", "closed"), - "unixListener" => ("A Unix listener", "opened", "closed"), - "unixDatagram" => ("A Unix datagram", "opened", "closed"), - "tcpListener" => ("A TCP listener", "opened", "closed"), - "udpSocket" => ("A UDP socket", "opened", "closed"), - "timer" => ("A timer", "started", "fired/cleared"), - "textDecoder" => ("A text decoder", "created", "finished"), - "messagePort" => ("A message port", "created", "closed"), - "webSocketStream" => ("A WebSocket", "opened", "closed"), - "fsEvents" => ("A file system watcher", "created", "closed"), - "childStdin" => ("A child process stdin", "opened", "closed"), - "childStdout" => ("A child process stdout", "opened", "closed"), - "childStderr" => ("A child process stderr", "opened", "closed"), - "child" => ("A child process", "started", "closed"), - "signal" => ("A signal listener", "created", "fired/cleared"), - "stdin" => ("The stdin pipe", "opened", "closed"), - "stdout" => ("The stdout pipe", "opened", "closed"), - "stderr" => ("The stderr pipe", "opened", "closed"), - "compression" => ("A CompressionStream", "created", "closed"), - _ => return (format!("\"{name}\"").into(), "created", "cleaned up"), - }; - (name.into(), action1, action2) -} - -fn resource_close_hint(name: &str) -> &'static str { - match name { - "fsFile" => "Close the file handle by calling `file.close()`.", - "fetchRequest" => "Await the promise returned from `fetch()` or abort the fetch with an abort signal.", - "fetchRequestBody" => "Terminate the request body `ReadableStream` by closing or erroring it.", - "fetchResponse" => "Consume or close the response body `ReadableStream`, e.g `await resp.text()` or `await resp.body.cancel()`.", - "httpClient" => "Close the HTTP client by calling `httpClient.close()`.", - "dynamicLibrary" => "Unload the dynamic library by calling `dynamicLibrary.close()`.", - "httpConn" => "Close the inbound HTTP connection by calling `httpConn.close()`.", - "httpStream" => "Close the inbound HTTP request by responding with `e.respondWith()` or closing the HTTP connection.", - "tcpStream" => "Close the TCP connection by calling `tcpConn.close()`.", - "unixStream" => "Close the Unix socket connection by calling `unixConn.close()`.", - "tlsStream" => "Close the TLS connection by calling `tlsConn.close()`.", - "tlsListener" => "Close the TLS listener by calling `tlsListener.close()`.", - "unixListener" => "Close the Unix socket listener by calling `unixListener.close()`.", - "unixDatagram" => "Close the Unix datagram socket by calling `unixDatagram.close()`.", - "tcpListener" => "Close the TCP listener by calling `tcpListener.close()`.", - "udpSocket" => "Close the UDP socket by calling `udpSocket.close()`.", - "timer" => "Clear the timer by calling `clearInterval` or `clearTimeout`.", - "textDecoder" => "Close the text decoder by calling `textDecoder.decode('')` or `await textDecoderStream.readable.cancel()`.", - "messagePort" => "Close the message port by calling `messagePort.close()`.", - "webSocketStream" => "Close the WebSocket by calling `webSocket.close()`.", - "fsEvents" => "Close the file system watcher by calling `watcher.close()`.", - "childStdin" => "Close the child process stdin by calling `proc.stdin.close()`.", - "childStdout" => "Close the child process stdout by calling `proc.stdout.close()` or `await child.stdout.cancel()`.", - "childStderr" => "Close the child process stderr by calling `proc.stderr.close()` or `await child.stderr.cancel()`.", - "child" => "Close the child process by calling `proc.kill()` or `proc.close()`.", - "signal" => "Clear the signal listener by calling `Deno.removeSignalListener`.", - "stdin" => "Close the stdin pipe by calling `Deno.stdin.close()`.", - "stdout" => "Close the stdout pipe by calling `Deno.stdout.close()`.", - "stderr" => "Close the stderr pipe by calling `Deno.stderr.close()`.", - "compression" => "Close the compression stream by calling `await stream.writable.close()`.", - _ => "Close the resource before the end of the test.", - } -} diff --git a/cli/tools/test/mod.rs b/cli/tools/test/mod.rs index e62582af3010c6..d1dc76028ac2e3 100644 --- a/cli/tools/test/mod.rs +++ b/cli/tools/test/mod.rs @@ -40,8 +40,6 @@ use deno_core::futures::StreamExt; use deno_core::located_script_name; use deno_core::parking_lot::Mutex; use deno_core::serde_v8; -use deno_core::stats::RuntimeActivityStats; -use deno_core::stats::RuntimeActivityStatsFilter; use deno_core::unsync::spawn; use deno_core::unsync::spawn_blocking; use deno_core::url::Url; @@ -89,7 +87,6 @@ use tokio::sync::mpsc::WeakUnboundedSender; pub mod fmt; pub mod reporters; -use fmt::format_sanitizer_diff; pub use fmt::format_test_error; use reporters::CompoundTestReporter; use reporters::DotTestReporter; @@ -178,29 +175,6 @@ pub struct TestDescription { pub only: bool, pub origin: String, pub location: TestLocation, - pub sanitize_ops: bool, - pub sanitize_resources: bool, -} - -/// May represent a failure of a test or test step. -#[derive(Debug, Clone, PartialEq, Deserialize, Eq, Hash)] -#[serde(rename_all = "camelCase")] -pub struct TestFailureDescription { - pub id: usize, - pub name: String, - pub origin: String, - pub location: TestLocation, -} - -impl From<&TestDescription> for TestFailureDescription { - fn from(value: &TestDescription) -> Self { - Self { - id: value.id, - name: value.name.clone(), - origin: value.origin.clone(), - location: value.location.clone(), - } - } } #[allow(clippy::derive_partial_eq_without_eq)] @@ -358,7 +332,7 @@ pub struct TestSummary { pub ignored_steps: usize, pub filtered_out: usize, pub measured: usize, - pub failures: Vec<(TestFailureDescription, TestFailure)>, + pub failures: Vec<(TestDescription, TestFailure)>, pub uncaught_errors: Vec<(String, Box)>, } @@ -573,17 +547,6 @@ pub async fn run_tests_for_worker( used_only, }))?; let mut had_uncaught_error = false; - let stats = worker.js_runtime.runtime_activity_stats_factory(); - let ops = worker.js_runtime.op_names(); - let op_id_host_recv_message = ops - .iter() - .position(|op| *op == "op_host_recv_message") - .unwrap(); - let op_id_host_recv_ctrl = ops - .iter() - .position(|op| *op == "op_host_recv_ctrl") - .unwrap(); - for (desc, function) in tests { if fail_fast_tracker.should_stop() { break; @@ -598,11 +561,15 @@ pub async fn run_tests_for_worker( } sender.send(TestEvent::Wait(desc.id))?; - // Poll event loop once, to allow all ops that are already resolved, but haven't - // responded to settle. - // TODO(mmastrac): we should provide an API to poll the event loop until no futher - // progress is made. + // TODO(bartlomieju): this is a nasty (beautiful) hack, that was required + // when switching `JsRuntime` from `FuturesUnordered` to `JoinSet`. With + // `JoinSet` all pending ops are immediately polled and that caused a problem + // when some async ops were fired and canceled before running tests (giving + // false positives in the ops sanitizer). We should probably rewrite sanitizers + // to be done in Rust instead of in JS (40_testing.js). { + // Poll event loop once, this will allow all ops that are already resolved, + // but haven't responded to settle. let waker = noop_waker(); let mut cx = Context::from_waker(&waker); let _ = worker @@ -610,22 +577,6 @@ pub async fn run_tests_for_worker( .poll_event_loop(&mut cx, PollEventLoopOptions::default()); } - let mut filter = RuntimeActivityStatsFilter::default(); - if desc.sanitize_ops { - filter = filter.with_ops().with_timers(); - filter = filter.omit_op(op_id_host_recv_ctrl as _); - filter = filter.omit_op(op_id_host_recv_message as _); - } - if desc.sanitize_resources { - filter = filter.with_resources(); - } - - let before = if !filter.is_empty() { - Some(stats.clone().capture(&filter)) - } else { - None - }; - let earlier = SystemTime::now(); let call = worker.js_runtime.call(&function); let result = match worker @@ -649,22 +600,6 @@ pub async fn run_tests_for_worker( } } }; - if let Some(before) = before { - let after = stats.clone().capture(&filter); - let diff = RuntimeActivityStats::diff(&before, &after); - let formatted = format_sanitizer_diff(diff); - if !formatted.is_empty() { - let failure = TestFailure::LeakedResources(formatted); - let elapsed = SystemTime::now().duration_since(earlier)?.as_millis(); - sender.send(TestEvent::Result( - desc.id, - TestResult::Failed(failure), - elapsed as u64, - ))?; - continue; - } - } - let scope = &mut worker.js_runtime.handle_scope(); let result = v8::Local::new(scope, result); let result = serde_v8::from_v8::(scope, result)?; diff --git a/cli/tools/test/reporters/common.rs b/cli/tools/test/reporters/common.rs index 1dc8796670a929..3d9cdba4682e71 100644 --- a/cli/tools/test/reporters/common.rs +++ b/cli/tools/test/reporters/common.rs @@ -33,10 +33,7 @@ pub(super) fn format_test_step_ancestry( result } -pub fn format_test_for_summary( - cwd: &Url, - desc: &TestFailureDescription, -) -> String { +pub fn format_test_for_summary(cwd: &Url, desc: &TestDescription) -> String { format!( "{} {}", &desc.name, @@ -81,7 +78,7 @@ pub(super) fn report_sigint( let mut formatted_pending = BTreeSet::new(); for id in tests_pending { if let Some(desc) = tests.get(id) { - formatted_pending.insert(format_test_for_summary(cwd, &desc.into())); + formatted_pending.insert(format_test_for_summary(cwd, desc)); } if let Some(desc) = test_steps.get(id) { formatted_pending @@ -110,10 +107,7 @@ pub(super) fn report_summary( #[allow(clippy::type_complexity)] // Type alias doesn't look better here let mut failures_by_origin: BTreeMap< String, - ( - Vec<(&TestFailureDescription, &TestFailure)>, - Option<&JsError>, - ), + (Vec<(&TestDescription, &TestFailure)>, Option<&JsError>), > = BTreeMap::default(); let mut failure_titles = vec![]; for (description, failure) in &summary.failures { diff --git a/cli/tools/test/reporters/dot.rs b/cli/tools/test/reporters/dot.rs index 39a4e44d984fd6..0df000dad26fea 100644 --- a/cli/tools/test/reporters/dot.rs +++ b/cli/tools/test/reporters/dot.rs @@ -113,7 +113,7 @@ impl TestReporter for DotTestReporter { self .summary .failures - .push((description.into(), failure.clone())); + .push((description.clone(), failure.clone())); } TestResult::Cancelled => { self.summary.failed += 1; @@ -162,9 +162,11 @@ impl TestReporter for DotTestReporter { TestStepResult::Failed(failure) => { self.summary.failed_steps += 1; self.summary.failures.push(( - TestFailureDescription { + TestDescription { id: desc.id, name: common::format_test_step_ancestry(desc, tests, test_steps), + ignore: false, + only: false, origin: desc.origin.clone(), location: desc.location.clone(), }, diff --git a/cli/tools/test/reporters/pretty.rs b/cli/tools/test/reporters/pretty.rs index c49081dd664c95..4a96192e6d8ab4 100644 --- a/cli/tools/test/reporters/pretty.rs +++ b/cli/tools/test/reporters/pretty.rs @@ -233,7 +233,7 @@ impl TestReporter for PrettyTestReporter { self .summary .failures - .push((description.into(), failure.clone())); + .push((description.clone(), failure.clone())); } TestResult::Cancelled => { self.summary.failed += 1; @@ -318,9 +318,11 @@ impl TestReporter for PrettyTestReporter { TestStepResult::Failed(failure) => { self.summary.failed_steps += 1; self.summary.failures.push(( - TestFailureDescription { + TestDescription { id: desc.id, name: common::format_test_step_ancestry(desc, tests, test_steps), + ignore: false, + only: false, origin: desc.origin.clone(), location: desc.location.clone(), },