From 113896f710310c0d35064d7e22b4da7178c65911 Mon Sep 17 00:00:00 2001 From: stefnotch Date: Fri, 3 Jan 2025 11:34:57 +0800 Subject: [PATCH 01/10] Compile without cmake --- Cargo.toml | 2 +- cli/Cargo.toml | 2 +- resolvers/npm_cache/Cargo.toml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 0c11ff9a693471..61f3640b44abd9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -153,7 +153,7 @@ ipnet = "2.3" jsonc-parser = { version = "=0.26.2", features = ["serde"] } lazy-regex = "3" libc = "0.2.168" -libz-sys = { version = "1.1.20", default-features = false } +libz-sys = { version = "1.1.20", default-features = false, features = ["zlib-ng-no-cmake-experimental-community-maintained"] } log = { version = "0.4.20", features = ["kv"] } lsp-types = "=0.97.0" # used by tower-lsp and "proposed" feature is unstable in patch releases memmem = "0.1.1" diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 4525a1bab4469a..9cc060474bf4d6 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -37,7 +37,7 @@ harness = false path = "./bench/lsp_bench_standalone.rs" [features] -default = ["upgrade", "__vendored_zlib_ng"] +default = ["upgrade"] # A feature that enables heap profiling with dhat on Linux. # 1. Compile with `cargo build --profile=release-with-debug --features=dhat-heap` # 2. Run the executable. It will output a dhat-heap.json file. diff --git a/resolvers/npm_cache/Cargo.toml b/resolvers/npm_cache/Cargo.toml index d73cee9cd63cef..b6f647ff645e09 100644 --- a/resolvers/npm_cache/Cargo.toml +++ b/resolvers/npm_cache/Cargo.toml @@ -24,7 +24,7 @@ deno_path_util.workspace = true deno_semver.workspace = true deno_unsync = { workspace = true, features = ["tokio"] } faster-hex.workspace = true -flate2 = { workspace = true, features = ["zlib-ng-compat"] } +flate2 = { workspace = true, features = ["default"] } futures.workspace = true http.workspace = true log.workspace = true From 7ba1f318140bab3118ca0ade67ea69cb1761b61a Mon Sep 17 00:00:00 2001 From: stefnotch Date: Wed, 15 Jan 2025 15:42:44 +0100 Subject: [PATCH 02/10] Precise error message reporting --- cli/js/40_test.js | 27 ++++++++++++++++- cli/lsp/testing/execution.rs | 57 +++++++++++++++++++++++++++++------- cli/tools/test/mod.rs | 40 +++++++++++++++++++++++++ 3 files changed, 113 insertions(+), 11 deletions(-) diff --git a/cli/js/40_test.js b/cli/js/40_test.js index 3dbb7ec3402f72..cd4150cb4caaec 100644 --- a/cli/js/40_test.js +++ b/cli/js/40_test.js @@ -125,14 +125,39 @@ function assertExit(fn, isTest) { }; } +// From @std/internal/1.0.5/format.ts +function format(v) { + return Deno.inspect(v, { + depth: Infinity, + sorted: true, + trailingComma: true, + compact: true, + iterableLimit: Infinity, + // getters should be true in assertEquals. + getters: true, + strAbbreviateSize: Infinity, + }); +} + function wrapOuter(fn, desc) { return async function outerWrapped() { try { if (desc.ignore) { return "ignored"; } - return await fn(desc) ?? "ok"; + return (await fn(desc)) ?? "ok"; } catch (error) { + if ("expected" in error && "actual" in error) { + return { + failed: { + expected: { + jsError: core.destructureError(error), + expected: format(error.expected), + actual: format(error.actual), + }, + }, + }; + } return { failed: { jsError: core.destructureError(error) } }; } finally { const state = MapPrototypeGet(testStates, desc.id); diff --git a/cli/lsp/testing/execution.rs b/cli/lsp/testing/execution.rs index 3d6d6f6b734a5c..1e33c46330aa83 100644 --- a/cli/lsp/testing/execution.rs +++ b/cli/lsp/testing/execution.rs @@ -42,6 +42,7 @@ use crate::lsp::urls::url_to_uri; use crate::tools::test; use crate::tools::test::create_test_event_channel; use crate::tools::test::FailFastTracker; +use crate::tools::test::TestFailure; use crate::tools::test::TestFailureFormatOptions; /// Logic to convert a test request into a set of test modules to be tested and @@ -122,6 +123,44 @@ fn as_test_messages>( }] } +fn failure_to_test_message( + test_uri: lsp::Uri, + failure: &TestFailure, +) -> lsp_custom::TestMessage { + let message = lsp::MarkupContent { + kind: lsp::MarkupKind::PlainText, + value: failure + .format(&TestFailureFormatOptions::default()) + .to_string(), + }; + let (expected_output, actual_output) = + if let TestFailure::Expected { + expected, actual, .. + } = failure + { + (Some(expected.to_string()), Some(actual.to_string())) + } else { + (None, None) + }; + let location = failure.error_location().map(|v| { + let pos = lsp::Position { + line: v.line_number, + // TODO: The column number might be wrong if Unicode is involved + character: v.column_number, + }; + lsp::Location { + uri: test_uri, + range: lsp::Range::new(pos, pos), + } + }); + lsp_custom::TestMessage { + message, + expected_output, + actual_output, + location, + } +} + #[derive(Debug, Clone, Default, PartialEq)] struct LspTestFilter { include: Option>, @@ -667,12 +706,11 @@ impl LspTestReporter { } test::TestResult::Failed(failure) => { let desc = self.tests.get(&desc.id).unwrap(); + let test = desc.as_test_identifier(&self.tests); + let uri = test.text_document.uri.clone(); self.progress(lsp_custom::TestRunProgressMessage::Failed { - test: desc.as_test_identifier(&self.tests), - messages: as_test_messages( - failure.format(&TestFailureFormatOptions::default()), - false, - ), + test, + messages: vec![failure_to_test_message(uri, &failure)], duration: Some(elapsed as u32), }) } @@ -767,12 +805,11 @@ impl LspTestReporter { }) } test::TestStepResult::Failed(failure) => { + let test = desc.as_test_identifier(&self.tests); + let uri = test.text_document.uri.clone(); self.progress(lsp_custom::TestRunProgressMessage::Failed { - test: desc.as_test_identifier(&self.tests), - messages: as_test_messages( - failure.format(&TestFailureFormatOptions::default()), - false, - ), + test, + messages: vec![failure_to_test_message(uri, &failure)], duration: Some(elapsed as u32), }) } diff --git a/cli/tools/test/mod.rs b/cli/tools/test/mod.rs index 21ee7e152d159b..de81be7edd5867 100644 --- a/cli/tools/test/mod.rs +++ b/cli/tools/test/mod.rs @@ -314,6 +314,12 @@ pub struct TestFailureFormatOptions { #[serde(rename_all = "camelCase")] pub enum TestFailure { JsError(Box), + #[serde(rename_all = "camelCase")] + Expected { + js_error: Box, + expected: String, + actual: String, + }, FailedSteps(usize), IncompleteSteps, Leaked(Vec, Vec), // Details, trailer notes @@ -332,6 +338,9 @@ impl TestFailure { TestFailure::JsError(js_error) => { Cow::Owned(format_test_error(js_error, options)) } + TestFailure::Expected { js_error, .. } => { + Cow::Owned(format_test_error(js_error, options)) + } TestFailure::FailedSteps(1) => Cow::Borrowed("1 test step failed."), TestFailure::FailedSteps(n) => { Cow::Owned(format!("{} test steps failed.", n)) @@ -375,6 +384,9 @@ impl TestFailure { pub fn overview(&self) -> String { match self { TestFailure::JsError(js_error) => js_error.exception_message.clone(), + TestFailure::Expected { js_error, .. } => { + js_error.exception_message.clone() + } TestFailure::FailedSteps(1) => "1 test step failed".to_string(), TestFailure::FailedSteps(n) => format!("{n} test steps failed"), TestFailure::IncompleteSteps => { @@ -393,6 +405,34 @@ impl TestFailure { } } + pub fn error_location(&self) -> Option { + match self { + // TODO: What happens if not enough call stack frames were recorded? + // TODO: Could anything else trigger the ext:cli/40_test.js file? Maybe we should also test for the method name + TestFailure::JsError(js_error) + | TestFailure::Expected { js_error, .. } => js_error + .frames + .iter() + .position(|v| v.file_name.as_deref() == Some("ext:cli/40_test.js")) + // Go one up in the stack frame, this is where the user code was + .and_then(|index| index.checked_sub(1)) + .and_then(|index| { + let user_frame = &js_error.frames[index]; + let file_name = user_frame.file_name.as_ref()?.to_string(); + // Turn into zero based indices + let line_number = user_frame.line_number.map(|v| v - 1)? as u32; + let column_number = + user_frame.column_number.map(|v| v - 1).unwrap_or(0) as u32; + Some(TestLocation { + file_name, + line_number, + column_number, + }) + }), + _ => None, + } + } + fn format_label(&self) -> String { match self { TestFailure::Incomplete => colors::gray("INCOMPLETE").to_string(), From c5d9abc52b2b47f711e1ee181ed937bbda0605b8 Mon Sep 17 00:00:00 2001 From: stefnotch Date: Wed, 15 Jan 2025 15:43:02 +0100 Subject: [PATCH 03/10] Prep for test explorer watch mode --- cli/lsp/testing/execution.rs | 6 ++++++ cli/lsp/testing/lsp_custom.rs | 2 ++ 2 files changed, 8 insertions(+) diff --git a/cli/lsp/testing/execution.rs b/cli/lsp/testing/execution.rs index 1e33c46330aa83..09b64b7b2b1b8c 100644 --- a/cli/lsp/testing/execution.rs +++ b/cli/lsp/testing/execution.rs @@ -190,6 +190,7 @@ impl LspTestFilter { pub struct TestRun { id: u32, kind: lsp_custom::TestRunKind, + is_continuous: bool, filters: HashMap, queue: HashSet, tests: TestServerTests, @@ -211,6 +212,7 @@ impl TestRun { Self { id: params.id, kind: params.kind.clone(), + is_continuous: params.is_continuous, filters, queue, tests, @@ -529,6 +531,9 @@ impl TestRun { { args.push(Cow::Borrowed("--inspect")); } + if self.is_continuous && !args.contains(&Cow::Borrowed("--watch")) { + args.push(Cow::Borrowed("--watch")); + } args } } @@ -845,6 +850,7 @@ mod tests { let params = lsp_custom::TestRunRequestParams { id: 1, kind: lsp_custom::TestRunKind::Run, + is_continuous: false, include: Some(vec![ lsp_custom::TestIdentifier { text_document: lsp::TextDocumentIdentifier { diff --git a/cli/lsp/testing/lsp_custom.rs b/cli/lsp/testing/lsp_custom.rs index 5ee6e84f52b876..10711a9118c5c3 100644 --- a/cli/lsp/testing/lsp_custom.rs +++ b/cli/lsp/testing/lsp_custom.rs @@ -93,6 +93,8 @@ pub enum TestRunKind { pub struct TestRunRequestParams { pub id: u32, pub kind: TestRunKind, + /// Corresponds to --watch + pub is_continuous: bool, #[serde(skip_serializing_if = "Vec::is_empty")] #[serde(default)] pub exclude: Vec, From 4d3c3f29db02f75854f3acb60047a543726280cb Mon Sep 17 00:00:00 2001 From: stefnotch Date: Wed, 15 Jan 2025 15:59:42 +0100 Subject: [PATCH 04/10] Build with newer libuv --- Cargo.lock | 5 ++--- ext/napi/Cargo.toml | 8 ++++---- tests/napi/Cargo.toml | 10 ++++++---- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7ec72aa3cdafde..c62533e1acaabd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4804,12 +4804,11 @@ dependencies = [ [[package]] name = "libuv-sys-lite" -version = "1.48.2" +version = "1.48.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ca8dfd1a173826d193e3b955e07c22765829890f62c677a59c4a410cb4f47c01" +checksum = "10de9b449ef5865c757dc2a0c1b1dde1578adf466db4bb0074e5d83b530164f1" dependencies = [ "bindgen", - "libloading 0.8.5", ] [[package]] diff --git a/ext/napi/Cargo.toml b/ext/napi/Cargo.toml index ff69d6a47e064f..a5fcc8a3bd2c62 100644 --- a/ext/napi/Cargo.toml +++ b/ext/napi/Cargo.toml @@ -1,14 +1,14 @@ # Copyright 2018-2025 the Deno authors. MIT license. [package] -name = "deno_napi" -version = "0.116.0" authors.workspace = true +description = "NAPI implementation for Deno" edition.workspace = true license.workspace = true +name = "deno_napi" readme = "README.md" repository.workspace = true -description = "NAPI implementation for Deno" +version = "0.116.0" [lib] path = "lib.rs" @@ -27,4 +27,4 @@ thiserror.workspace = true windows-sys.workspace = true [dev-dependencies] -libuv-sys-lite = "=1.48.2" +libuv-sys-lite = "=1.48.3" diff --git a/tests/napi/Cargo.toml b/tests/napi/Cargo.toml index e8a39a0cda825e..5a52902cbe38b5 100644 --- a/tests/napi/Cargo.toml +++ b/tests/napi/Cargo.toml @@ -1,20 +1,22 @@ # Copyright 2018-2025 the Deno authors. MIT license. [package] -name = "test_napi" -version = "0.1.0" authors.workspace = true edition.workspace = true license.workspace = true +name = "test_napi" publish = false repository.workspace = true +version = "0.1.0" [lib] crate-type = ["cdylib"] [dependencies] -libuv-sys-lite = "=1.48.2" -napi-sys = { version = "=2.2.2", default-features = false, features = ["napi7"] } +libuv-sys-lite = "=1.48.3" +napi-sys = { version = "=2.2.2", default-features = false, features = [ + "napi7", +] } [dev-dependencies] test_util.workspace = true From 9d3044e529e94113f50ad874e36052ccb7216709 Mon Sep 17 00:00:00 2001 From: stefnotch Date: Sat, 18 Jan 2025 16:24:54 +0100 Subject: [PATCH 05/10] Enable dyn-symbols for libuv --- Cargo.lock | 1 + ext/napi/Cargo.toml | 2 +- tests/napi/Cargo.toml | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c62533e1acaabd..3a57769082c45f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4809,6 +4809,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "10de9b449ef5865c757dc2a0c1b1dde1578adf466db4bb0074e5d83b530164f1" dependencies = [ "bindgen", + "libloading 0.8.5", ] [[package]] diff --git a/ext/napi/Cargo.toml b/ext/napi/Cargo.toml index a5fcc8a3bd2c62..7941a28a1d19ad 100644 --- a/ext/napi/Cargo.toml +++ b/ext/napi/Cargo.toml @@ -27,4 +27,4 @@ thiserror.workspace = true windows-sys.workspace = true [dev-dependencies] -libuv-sys-lite = "=1.48.3" +libuv-sys-lite = { version = "=1.48.3", features = ["dyn-symbols"] } diff --git a/tests/napi/Cargo.toml b/tests/napi/Cargo.toml index 5a52902cbe38b5..fc0d5a461f122b 100644 --- a/tests/napi/Cargo.toml +++ b/tests/napi/Cargo.toml @@ -13,7 +13,7 @@ version = "0.1.0" crate-type = ["cdylib"] [dependencies] -libuv-sys-lite = "=1.48.3" +libuv-sys-lite = { version = "=1.48.3", features = ["dyn-symbols"] } napi-sys = { version = "=2.2.2", default-features = false, features = [ "napi7", ] } From 17c9c18ac7626fa49385dcc90e0537033093d931 Mon Sep 17 00:00:00 2001 From: stefnotch Date: Sat, 18 Jan 2025 20:41:34 +0100 Subject: [PATCH 06/10] chore(lsp): factor out test event handler --- cli/lsp/testing/execution.rs | 248 ++++++++++++++++++++--------------- 1 file changed, 139 insertions(+), 109 deletions(-) diff --git a/cli/lsp/testing/execution.rs b/cli/lsp/testing/execution.rs index 09b64b7b2b1b8c..d457057ef451d6 100644 --- a/cli/lsp/testing/execution.rs +++ b/cli/lsp/testing/execution.rs @@ -305,9 +305,6 @@ impl TestRun { let mut queue = self.queue.iter().collect::>(); queue.sort(); - let tests: Arc>> = - Arc::new(RwLock::new(IndexMap::new())); - let mut test_steps = IndexMap::new(); let worker_factory = Arc::new(factory.create_cli_main_worker_factory().await?); @@ -364,119 +361,29 @@ impl TestRun { .buffer_unordered(concurrent_jobs) .collect::, tokio::task::JoinError>>>(); - let mut reporter = Box::new(LspTestReporter::new( - self, - client.clone(), - maybe_root_uri, - self.tests.clone(), - )); + let mut test_event_handler = LspTestEventHandler { + reporter: Box::new(LspTestReporter::new( + self, + client.clone(), + maybe_root_uri, + self.tests.clone(), + )), + tests: Default::default(), + test_steps: Default::default(), + summary: Default::default(), + tests_with_result: Default::default(), + used_only: Default::default(), + }; let handler = { spawn(async move { - let earlier = Instant::now(); - let mut summary = test::TestSummary::new(); - let mut tests_with_result = HashSet::new(); - let mut used_only = false; + let start_time = Instant::now(); while let Some((_, event)) = receiver.recv().await { - match event { - test::TestEvent::Register(description) => { - for (_, description) in description.into_iter() { - reporter.report_register(description).await; - // TODO(mmastrac): we shouldn't need to clone here - we can re-use the descriptions - tests.write().insert(description.id, description.clone()); - } - } - test::TestEvent::Plan(plan) => { - summary.total += plan.total; - summary.filtered_out += plan.filtered_out; - - if plan.used_only { - used_only = true; - } - - reporter.report_plan(&plan); - } - test::TestEvent::Wait(id) => { - reporter.report_wait(tests.read().get(&id).unwrap()); - } - test::TestEvent::Output(output) => { - reporter.report_output(&output); - } - test::TestEvent::Slow(id, elapsed) => { - reporter.report_slow(tests.read().get(&id).unwrap(), elapsed); - } - test::TestEvent::Result(id, result, elapsed) => { - if tests_with_result.insert(id) { - let description = tests.read().get(&id).unwrap().clone(); - match &result { - test::TestResult::Ok => summary.passed += 1, - test::TestResult::Ignored => summary.ignored += 1, - test::TestResult::Failed(error) => { - summary.failed += 1; - summary - .failures - .push(((&description).into(), error.clone())); - } - test::TestResult::Cancelled => { - summary.failed += 1; - } - } - reporter.report_result(&description, &result, elapsed); - } - } - test::TestEvent::UncaughtError(origin, error) => { - reporter.report_uncaught_error(&origin, &error); - summary.failed += 1; - summary.uncaught_errors.push((origin, error)); - } - test::TestEvent::StepRegister(description) => { - reporter.report_step_register(&description).await; - test_steps.insert(description.id, description); - } - test::TestEvent::StepWait(id) => { - reporter.report_step_wait(test_steps.get(&id).unwrap()); - } - test::TestEvent::StepResult(id, result, duration) => { - if tests_with_result.insert(id) { - match &result { - test::TestStepResult::Ok => { - summary.passed_steps += 1; - } - test::TestStepResult::Ignored => { - summary.ignored_steps += 1; - } - test::TestStepResult::Failed(_) => { - summary.failed_steps += 1; - } - } - reporter.report_step_result( - test_steps.get(&id).unwrap(), - &result, - duration, - ); - } - } - test::TestEvent::Completed => { - reporter.report_completed(); - } - test::TestEvent::ForceEndReport => {} - test::TestEvent::Sigint => {} - } + test_event_handler.handle_event(event).await; } - let elapsed = Instant::now().duration_since(earlier); - reporter.report_summary(&summary, &elapsed); - - if used_only { - return Err(anyhow!( - "Test failed because the \"only\" option was used" - )); - } - - if summary.failed > 0 { - return Err(anyhow!("Test failed")); - } + test_event_handler.finish(start_time.elapsed())?; Ok(()) }) @@ -598,6 +505,129 @@ impl LspTestDescription { } } +struct LspTestEventHandler { + reporter: Box, + tests: Arc>>, + test_steps: IndexMap, + summary: test::TestSummary, + tests_with_result: HashSet, + used_only: bool, +} + +impl LspTestEventHandler { + async fn handle_event(&mut self, event: test::TestEvent) { + match event { + test::TestEvent::Register(description) => { + for (_, description) in description.into_iter() { + self.reporter.report_register(description).await; + // TODO(mmastrac): we shouldn't need to clone here - we can re-use the descriptions + self + .tests + .write() + .insert(description.id, description.clone()); + } + } + test::TestEvent::Plan(plan) => { + self.summary.total += plan.total; + self.summary.filtered_out += plan.filtered_out; + + if plan.used_only { + self.used_only = true; + } + + self.reporter.report_plan(&plan); + } + test::TestEvent::Wait(id) => { + self + .reporter + .report_wait(self.tests.read().get(&id).unwrap()); + } + test::TestEvent::Output(output) => { + self.reporter.report_output(&output); + } + test::TestEvent::Slow(id, elapsed) => { + self + .reporter + .report_slow(self.tests.read().get(&id).unwrap(), elapsed); + } + test::TestEvent::Result(id, result, elapsed) => { + if self.tests_with_result.insert(id) { + let description = self.tests.read().get(&id).unwrap().clone(); + match &result { + test::TestResult::Ok => self.summary.passed += 1, + test::TestResult::Ignored => self.summary.ignored += 1, + test::TestResult::Failed(error) => { + self.summary.failed += 1; + self + .summary + .failures + .push(((&description).into(), error.clone())); + } + test::TestResult::Cancelled => { + self.summary.failed += 1; + } + } + self.reporter.report_result(&description, &result, elapsed); + } + } + test::TestEvent::UncaughtError(origin, error) => { + self.reporter.report_uncaught_error(&origin, &error); + self.summary.failed += 1; + self.summary.uncaught_errors.push((origin, error)); + } + test::TestEvent::StepRegister(description) => { + self.reporter.report_step_register(&description).await; + self.test_steps.insert(description.id, description); + } + test::TestEvent::StepWait(id) => { + self + .reporter + .report_step_wait(self.test_steps.get(&id).unwrap()); + } + test::TestEvent::StepResult(id, result, duration) => { + if self.tests_with_result.insert(id) { + match &result { + test::TestStepResult::Ok => { + self.summary.passed_steps += 1; + } + test::TestStepResult::Ignored => { + self.summary.ignored_steps += 1; + } + test::TestStepResult::Failed(_) => { + self.summary.failed_steps += 1; + } + } + self.reporter.report_step_result( + self.test_steps.get(&id).unwrap(), + &result, + duration, + ); + } + } + test::TestEvent::Completed => { + self.reporter.report_completed(); + } + test::TestEvent::ForceEndReport => {} + test::TestEvent::Sigint => {} + } + } + + fn finish( + mut self, + elapsed: Duration, + ) -> Result<(), deno_core::anyhow::Error> { + self.reporter.report_summary(&self.summary, &elapsed); + + if self.used_only { + return Err(anyhow!("Test failed because the \"only\" option was used")); + } + + if self.summary.failed > 0 { + return Err(anyhow!("Test failed")); + } + Ok(()) + } +} struct LspTestReporter { client: Client, id: u32, From b90785e06607b060182ee290b813b9a852d2fb35 Mon Sep 17 00:00:00 2001 From: stefnotch Date: Mon, 20 Jan 2025 13:34:50 +0100 Subject: [PATCH 07/10] Implement continuous run --- cli/lsp/testing/execution.rs | 300 ++++++++++++++++++++++++++++------- cli/lsp/testing/server.rs | 7 +- cli/tools/run/mod.rs | 4 +- cli/tools/serve.rs | 4 +- cli/util/file_watcher.rs | 24 ++- 5 files changed, 271 insertions(+), 68 deletions(-) diff --git a/cli/lsp/testing/execution.rs b/cli/lsp/testing/execution.rs index d457057ef451d6..286c150c651175 100644 --- a/cli/lsp/testing/execution.rs +++ b/cli/lsp/testing/execution.rs @@ -44,11 +44,13 @@ use crate::tools::test::create_test_event_channel; use crate::tools::test::FailFastTracker; use crate::tools::test::TestFailure; use crate::tools::test::TestFailureFormatOptions; +use crate::util::file_watcher; /// Logic to convert a test request into a set of test modules to be tested and /// any filters to be applied to those tests fn as_queue_and_filters( - params: &lsp_custom::TestRunRequestParams, + exclude_tests: &[lsp_custom::TestIdentifier], + include_tests: Option<&[lsp_custom::TestIdentifier]>, tests: &HashMap, ) -> ( HashSet, @@ -57,7 +59,7 @@ fn as_queue_and_filters( let mut queue: HashSet = HashSet::new(); let mut filters: HashMap = HashMap::new(); - if let Some(include) = ¶ms.include { + if let Some(include) = include_tests { for item in include { let url = uri_to_url(&item.text_document.uri); if let Some((test_definitions, _)) = tests.get(&url) { @@ -80,7 +82,7 @@ fn as_queue_and_filters( queue.extend(tests.keys().cloned()); } - for item in ¶ms.exclude { + for item in exclude_tests { let url = uri_to_url(&item.text_document.uri); if let Some((test_definitions, _)) = tests.get(&url) { if let Some(id) = &item.id { @@ -103,26 +105,6 @@ fn as_queue_and_filters( (queue, filters) } -fn as_test_messages>( - message: S, - is_markdown: bool, -) -> Vec { - let message = lsp::MarkupContent { - kind: if is_markdown { - lsp::MarkupKind::Markdown - } else { - lsp::MarkupKind::PlainText - }, - value: message.as_ref().to_string(), - }; - vec![lsp_custom::TestMessage { - message, - expected_output: None, - actual_output: None, - location: None, - }] -} - fn failure_to_test_message( test_uri: lsp::Uri, failure: &TestFailure, @@ -191,8 +173,8 @@ pub struct TestRun { id: u32, kind: lsp_custom::TestRunKind, is_continuous: bool, - filters: HashMap, - queue: HashSet, + exclude_tests: Vec, + include_tests: Option>, tests: TestServerTests, token: CancellationToken, workspace_settings: config::WorkspaceSettings, @@ -200,21 +182,16 @@ pub struct TestRun { impl TestRun { pub async fn init( - params: &lsp_custom::TestRunRequestParams, + params: lsp_custom::TestRunRequestParams, tests: TestServerTests, workspace_settings: config::WorkspaceSettings, ) -> Self { - let (queue, filters) = { - let tests = tests.lock().await; - as_queue_and_filters(params, &tests) - }; - Self { id: params.id, - kind: params.kind.clone(), + kind: params.kind, is_continuous: params.is_continuous, - filters, - queue, + exclude_tests: params.exclude, + include_tests: params.include, tests, token: CancellationToken::new(), workspace_settings, @@ -225,12 +202,17 @@ impl TestRun { /// to the client to indicate tests are enqueued for testing. pub async fn as_enqueued(&self) -> Vec { let tests = self.tests.lock().await; - self - .queue + let (queue, filters) = as_queue_and_filters( + &self.exclude_tests, + self.include_tests.as_deref(), + &tests, + ); + + queue .iter() .filter_map(|s| { let ids = if let Some((test_module, _)) = tests.get(s) { - if let Some(filter) = self.filters.get(s) { + if let Some(filter) = filters.get(s) { filter.as_ids(test_module) } else { LspTestFilter::default().as_ids(test_module) @@ -259,6 +241,26 @@ impl TestRun { client: &Client, maybe_root_uri: Option<&ModuleSpecifier>, ) -> Result<(), AnyError> { + if self.is_continuous { + self.run_tests_with_watch(client, maybe_root_uri).await + } else { + self.run_tests(client, maybe_root_uri).await + } + } + + async fn run_tests( + &self, + client: &Client, + maybe_root_uri: Option<&ModuleSpecifier>, + ) -> Result<(), AnyError> { + let (queue, filters) = { + let tests = self.tests.lock().await; + as_queue_and_filters( + &self.exclude_tests, + self.include_tests.as_deref(), + &tests, + ) + }; let args = self.get_args(); lsp_log!("Executing test run with arguments: {}", args.join(" ")); let flags = Arc::new(flags_from_vec( @@ -276,7 +278,7 @@ impl TestRun { )?; let main_graph_container = factory.main_module_graph_container().await?; main_graph_container - .check_specifiers(&self.queue.iter().cloned().collect::>(), None) + .check_specifiers(&queue.iter().cloned().collect::>(), None) .await?; let (concurrent_jobs, fail_fast) = @@ -300,9 +302,10 @@ impl TestRun { let concurrent_jobs = std::cmp::min(concurrent_jobs, 4); let (test_event_sender_factory, mut receiver) = create_test_event_channel(); + // test_event_sender_factory.weak_sender().send(test::TestEvent::Sigint); // TODO: Dis? let fail_fast_tracker = FailFastTracker::new(fail_fast); - let mut queue = self.queue.iter().collect::>(); + let mut queue = queue.iter().collect::>(); queue.sort(); let worker_factory = @@ -317,7 +320,7 @@ impl TestRun { ); let worker_sender = test_event_sender_factory.worker(); let fail_fast_tracker = fail_fast_tracker.clone(); - let lsp_filter = self.filters.get(&specifier); + let lsp_filter = filters.get(&specifier); let filter = test::TestFilter { substring: None, regex: None, @@ -370,24 +373,22 @@ impl TestRun { )), tests: Default::default(), test_steps: Default::default(), - summary: Default::default(), + summary: test::TestSummary::new(), tests_with_result: Default::default(), used_only: Default::default(), }; - let handler = { - spawn(async move { - let start_time = Instant::now(); + let handler = spawn(async move { + let start_time = Instant::now(); - while let Some((_, event)) = receiver.recv().await { - test_event_handler.handle_event(event).await; - } + while let Some((_, event)) = receiver.recv().await { + test_event_handler.handle_event(event).await; + } - test_event_handler.finish(start_time.elapsed())?; + test_event_handler.finish(start_time.elapsed())?; - Ok(()) - }) - }; + Result::<_, AnyError>::Ok(()) + }); let (join_results, result) = future::join(join_stream, handler).await; @@ -401,6 +402,182 @@ impl TestRun { Ok(()) } + async fn run_tests_with_watch( + &self, + client: &Client, + maybe_root_uri: Option<&ModuleSpecifier>, + ) -> Result<(), AnyError> { + let args = self.get_args(); + lsp_log!("Executing test run with arguments: {}", args.join(" ")); + let flags = Arc::new(flags_from_vec( + args.into_iter().map(|s| From::from(s.as_ref())).collect(), + )?); + + let (concurrent_jobs, fail_fast) = { + let factory = CliFactory::from_flags(flags.clone()); + let cli_options = factory.cli_options()?; + if let DenoSubcommand::Test(test_flags) = cli_options.sub_command() { + let concurrent_jobs = test_flags + .concurrent_jobs + .unwrap_or_else(|| NonZeroUsize::new(1).unwrap()) + .into(); + + // TODO(mmastrac): Temporarily limit concurrency in windows testing to avoid named pipe issue: + // *** Unexpected server pipe failure '"\\\\.\\pipe\\deno_pipe_e30f45c9df61b1e4.1198.222\\0"': 3 + // This is likely because we're hitting some sort of invisible resource limit + // This limit is both in cli/lsp/testing/execution.rs and cli/tools/test/mod.rs + #[cfg(windows)] + let concurrent_jobs = std::cmp::min(concurrent_jobs, 4); + (concurrent_jobs, test_flags.fail_fast) + } else { + unreachable!("Should always be Test subcommand."); + } + }; + let (test_event_sender_factory, mut receiver) = create_test_event_channel(); + let test_event_sender_factory = Arc::new(test_event_sender_factory); + + let join_handle = file_watcher::watch_recv( + flags.clone(), + file_watcher::PrintConfig::new("Test", false), + file_watcher::WatcherRestartMode::Automatic, + self.token.clone(), + move |flags, watcher_communicator, changed_paths| { + let test_event_sender_factory = test_event_sender_factory.clone(); + Ok(async move { + let (queue, filters) = { + let tests = self.tests.lock().await; + as_queue_and_filters( + &self.exclude_tests, + self.include_tests.as_deref(), + &tests, + ) + }; + let factory = CliFactory::from_flags(flags); + let cli_options = factory.cli_options()?; + // Various test files should not share the same permissions in terms of + // `PermissionsContainer` - otherwise granting/revoking permissions in one + // file would have impact on other files, which is undesirable. + let permission_desc_parser = + factory.permission_desc_parser()?.clone(); + let permissions = Permissions::from_options( + permission_desc_parser.as_ref(), + &cli_options.permissions_options(), + )?; + let main_graph_container = + factory.main_module_graph_container().await?; + main_graph_container + .check_specifiers(&queue.iter().cloned().collect::>(), None) + .await?; + + let fail_fast_tracker = FailFastTracker::new(fail_fast); + + let mut queue = queue.iter().collect::>(); + queue.sort(); + + let worker_factory = + Arc::new(factory.create_cli_main_worker_factory().await?); + + let token = self.token.clone(); + if token.is_cancelled() { + return Ok(()); + } + + let join_handles = queue.into_iter().map(move |specifier| { + let specifier = specifier.clone(); + let worker_factory = worker_factory.clone(); + let permissions_container = PermissionsContainer::new( + permission_desc_parser.clone(), + permissions.clone(), + ); + let worker_sender = test_event_sender_factory.worker(); + let fail_fast_tracker = fail_fast_tracker.clone(); + let lsp_filter = filters.get(&specifier); + let filter = test::TestFilter { + substring: None, + regex: None, + include: lsp_filter.and_then(|f| { + f.include + .as_ref() + .map(|i| i.values().map(|t| t.name.clone()).collect()) + }), + exclude: lsp_filter + .map(|f| f.exclude.values().map(|t| t.name.clone()).collect()) + .unwrap_or_default(), + }; + let token = self.token.clone(); + + spawn_blocking(move || { + if fail_fast_tracker.should_stop() { + return Ok(()); + } + if token.is_cancelled() { + Ok(()) + } else { + // All JsErrors are handled by test_specifier and piped into the test + // channel. + create_and_run_current_thread(test::test_specifier( + worker_factory, + permissions_container, + specifier, + worker_sender, + fail_fast_tracker, + test::TestSpecifierOptions { + filter, + shuffle: None, + trace_leaks: false, + }, + )) + } + }) + }); + + let join_stream = stream::iter(join_handles) + .buffer_unordered(concurrent_jobs) + .collect::, tokio::task::JoinError>>>() + .await; + for join_result in join_stream { + join_result??; + } + Ok(()) + }) + }, + ); + + let mut test_event_handler = LspTestEventHandler { + reporter: Box::new(LspTestReporter::new( + self, + client.clone(), + maybe_root_uri, + self.tests.clone(), + )), + tests: Default::default(), + test_steps: Default::default(), + summary: test::TestSummary::new(), + tests_with_result: Default::default(), + used_only: Default::default(), + }; + + let handler = spawn(async move { + let start_time = Instant::now(); + + while let Some((_, event)) = receiver.recv().await { + test_event_handler.handle_event(event).await; + } + + test_event_handler.finish(start_time.elapsed())?; + + Result::<_, AnyError>::Ok(()) + }); + + let (join_result, result) = future::join(join_handle, handler).await; + + // propagate any errors + join_result?; + result??; + + Ok(()) + } + fn get_args(&self) -> Vec> { let mut args = vec![Cow::Borrowed("deno"), Cow::Borrowed("test")]; args.extend( @@ -508,7 +685,7 @@ impl LspTestDescription { struct LspTestEventHandler { reporter: Box, tests: Arc>>, - test_steps: IndexMap, + test_steps: IndexMap, summary: test::TestSummary, tests_with_result: HashSet, used_only: bool, @@ -612,10 +789,7 @@ impl LspTestEventHandler { } } - fn finish( - mut self, - elapsed: Duration, - ) -> Result<(), deno_core::anyhow::Error> { + fn finish(mut self, elapsed: Duration) -> Result<(), AnyError> { self.reporter.report_summary(&self.summary, &elapsed); if self.used_only { @@ -767,7 +941,16 @@ impl LspTestReporter { origin, test::fmt::format_test_error(js_error, &TestFailureFormatOptions::default()) ); - let messages = as_test_messages(err_string, false); + let messages = vec![lsp_custom::TestMessage { + message: lsp::MarkupContent { + kind: lsp::MarkupKind::PlainText, + value: err_string, + }, + expected_output: None, + actual_output: None, + location: None, + }]; + for desc in self.tests.values().filter(|d| d.origin() == origin) { self.progress(lsp_custom::TestRunProgressMessage::Failed { test: desc.as_test_identifier(&self.tests), @@ -941,7 +1124,8 @@ mod tests { non_test_specifier.clone(), (TestModule::new(non_test_specifier), "1".to_string()), ); - let (queue, filters) = as_queue_and_filters(¶ms, &tests); + let (queue, filters) = + as_queue_and_filters(¶ms.exclude, params.include.as_deref(), &tests); assert_eq!(json!(queue), json!([specifier])); let mut exclude = HashMap::new(); exclude.insert( diff --git a/cli/lsp/testing/server.rs b/cli/lsp/testing/server.rs index e0570fcada11dc..d35d1cf301fda7 100644 --- a/cli/lsp/testing/server.rs +++ b/cli/lsp/testing/server.rs @@ -215,14 +215,15 @@ impl TestServer { params: lsp_custom::TestRunRequestParams, workspace_settings: config::WorkspaceSettings, ) -> LspResult> { + let id = params.id; let test_run = - { TestRun::init(¶ms, self.tests.clone(), workspace_settings).await }; + { TestRun::init(params, self.tests.clone(), workspace_settings).await }; let enqueued = test_run.as_enqueued().await; { let mut runs = self.runs.lock(); - runs.insert(params.id, test_run); + runs.insert(id, test_run); } - self.enqueue_run(params.id).map_err(|err| { + self.enqueue_run(id).map_err(|err| { log::error!("cannot enqueue run: {}", err); LspError::internal_error() })?; diff --git a/cli/tools/run/mod.rs b/cli/tools/run/mod.rs index ecf1bd52f3e247..1d41817731db23 100644 --- a/cli/tools/run/mod.rs +++ b/cli/tools/run/mod.rs @@ -14,7 +14,6 @@ use crate::args::WatchFlagsWithPaths; use crate::factory::CliFactory; use crate::npm::installer::PackageCaching; use crate::util; -use crate::util::file_watcher::WatcherRestartMode; pub mod hmr; @@ -117,14 +116,13 @@ async fn run_with_watch( flags: Arc, watch_flags: WatchFlagsWithPaths, ) -> Result { - util::file_watcher::watch_recv( + util::file_watcher::watch_func( flags, util::file_watcher::PrintConfig::new_with_banner( if watch_flags.hmr { "HMR" } else { "Watcher" }, "Process", !watch_flags.no_clear_screen, ), - WatcherRestartMode::Automatic, move |flags, watcher_communicator, changed_paths| { watcher_communicator.show_path_changed(changed_paths.clone()); Ok(async move { diff --git a/cli/tools/serve.rs b/cli/tools/serve.rs index 2143eb33bbfdab..b2a0ba891eefa4 100644 --- a/cli/tools/serve.rs +++ b/cli/tools/serve.rs @@ -12,7 +12,6 @@ use crate::args::Flags; use crate::args::ServeFlags; use crate::args::WatchFlagsWithPaths; use crate::factory::CliFactory; -use crate::util::file_watcher::WatcherRestartMode; use crate::worker::CliMainWorkerFactory; pub async fn serve( @@ -144,14 +143,13 @@ async fn serve_with_watch( worker_count: Option, ) -> Result { let hmr = watch_flags.hmr; - crate::util::file_watcher::watch_recv( + crate::util::file_watcher::watch_func( flags, crate::util::file_watcher::PrintConfig::new_with_banner( if watch_flags.hmr { "HMR" } else { "Watcher" }, "Process", !watch_flags.no_clear_screen, ), - WatcherRestartMode::Automatic, move |flags, watcher_communicator, changed_paths| { watcher_communicator.show_path_changed(changed_paths.clone()); Ok(async move { diff --git a/cli/util/file_watcher.rs b/cli/util/file_watcher.rs index d3ff1bae7747e3..b78b1c12ac5814 100644 --- a/cli/util/file_watcher.rs +++ b/cli/util/file_watcher.rs @@ -28,6 +28,7 @@ use tokio::sync::mpsc; use tokio::sync::mpsc::error::SendError; use tokio::sync::mpsc::UnboundedReceiver; use tokio::time::sleep; +use tokio_util::sync::CancellationToken; use crate::args::Flags; use crate::colors; @@ -66,7 +67,10 @@ impl DebouncedReceiver { loop { select! { items = self.receiver.recv() => { - self.received_items.extend(items?); + match items { + Some(items) => self.received_items.extend(items), + None => return Some(self.received_items.drain().collect()) + }; } _ = sleep(DEBOUNCE_INTERVAL) => { return Some(self.received_items.drain().collect()); @@ -262,6 +266,7 @@ where flags, print_config, WatcherRestartMode::Automatic, + CancellationToken::new(), operation, ) .boxed_local(); @@ -288,6 +293,7 @@ pub async fn watch_recv( mut flags: Arc, print_config: PrintConfig, restart_mode: WatcherRestartMode, + token: CancellationToken, mut operation: O, ) -> Result<(), AnyError> where @@ -331,12 +337,17 @@ where deno_core::unsync::spawn(async move { loop { let received_changed_paths = watcher_receiver.recv().await; + let should_stop = received_changed_paths.is_none(); changed_paths_ .borrow_mut() .clone_from(&received_changed_paths); // TODO(bartlomieju): should we fail on sending changed paths? let _ = watcher_.send(received_changed_paths); + + if should_stop { + break; + } } }); @@ -347,6 +358,9 @@ where for _ in 0..10 { tokio::task::yield_now().await; } + if token.is_cancelled() { + break; + } let mut watcher = new_watcher(watcher_sender.clone())?; consume_paths_to_watch(&mut watcher, &mut paths_to_watch_rx, &exclude_set); @@ -372,6 +386,9 @@ where } select! { + _ = token.cancelled() => { + break; + } _ = receiver_future => {}, _ = restart_rx.recv() => { print_after_restart(); @@ -403,6 +420,9 @@ where // and see if there are any new paths to watch received or any of the already // watched paths has changed. select! { + _ = token.cancelled() => { + break; + } _ = receiver_future => {}, _ = restart_rx.recv() => { print_after_restart(); @@ -410,6 +430,8 @@ where }, } } + + Ok(()) } fn new_watcher( From 5f45051edb0ab8484e2cab6738d018bf2102a6af Mon Sep 17 00:00:00 2001 From: stefnotch Date: Mon, 20 Jan 2025 13:35:01 +0100 Subject: [PATCH 08/10] Add debug logging --- cli/lsp/testing/execution.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/cli/lsp/testing/execution.rs b/cli/lsp/testing/execution.rs index 286c150c651175..8f983c4dfd59da 100644 --- a/cli/lsp/testing/execution.rs +++ b/cli/lsp/testing/execution.rs @@ -232,6 +232,8 @@ impl TestRun { /// If being executed, cancel the test. pub fn cancel(&self) { + lsp_log!("TOKEN CANCEL"); + self.token.cancel(); } @@ -436,12 +438,14 @@ impl TestRun { let (test_event_sender_factory, mut receiver) = create_test_event_channel(); let test_event_sender_factory = Arc::new(test_event_sender_factory); + lsp_log!("Starting the setup"); let join_handle = file_watcher::watch_recv( flags.clone(), file_watcher::PrintConfig::new("Test", false), file_watcher::WatcherRestartMode::Automatic, self.token.clone(), move |flags, watcher_communicator, changed_paths| { + lsp_log!("Running dem tests"); let test_event_sender_factory = test_event_sender_factory.clone(); Ok(async move { let (queue, filters) = { @@ -479,7 +483,8 @@ impl TestRun { let token = self.token.clone(); if token.is_cancelled() { - return Ok(()); + // TODO: Really? That's not really an error now is it. + return Err(anyhow!("Tests have been cancelled")); } let join_handles = queue.into_iter().map(move |specifier| { @@ -506,6 +511,8 @@ impl TestRun { }; let token = self.token.clone(); + lsp_log!("Running a test"); + spawn_blocking(move || { if fail_fast_tracker.should_stop() { return Ok(()); @@ -538,10 +545,13 @@ impl TestRun { for join_result in join_stream { join_result??; } + lsp_log!("Done running dem tests"); + Ok(()) }) }, ); + lsp_log!("After file watcher"); let mut test_event_handler = LspTestEventHandler { reporter: Box::new(LspTestReporter::new( @@ -559,12 +569,14 @@ impl TestRun { let handler = spawn(async move { let start_time = Instant::now(); + lsp_log!("Handler startup"); while let Some((_, event)) = receiver.recv().await { test_event_handler.handle_event(event).await; } test_event_handler.finish(start_time.elapsed())?; + lsp_log!("Handler shutdown"); Result::<_, AnyError>::Ok(()) }); @@ -575,6 +587,8 @@ impl TestRun { join_result?; result??; + lsp_log!("Done propagating the errors"); + Ok(()) } From 87e2e005a6f22847e1ea00ced3f2dfa78ebad68a Mon Sep 17 00:00:00 2001 From: stefnotch Date: Mon, 20 Jan 2025 13:48:27 +0100 Subject: [PATCH 09/10] Hook up file watching --- cli/lsp/testing/execution.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/cli/lsp/testing/execution.rs b/cli/lsp/testing/execution.rs index 8f983c4dfd59da..0541dc2929f540 100644 --- a/cli/lsp/testing/execution.rs +++ b/cli/lsp/testing/execution.rs @@ -467,6 +467,14 @@ impl TestRun { permission_desc_parser.as_ref(), &cli_options.permissions_options(), )?; + + let _ = watcher_communicator.watch_paths(cli_options.watch_paths()); + let test_files = queue + .iter() + .filter_map(|v| v.to_file_path().ok()) + .collect::>(); + let _ = watcher_communicator.watch_paths(test_files); + let main_graph_container = factory.main_module_graph_container().await?; main_graph_container From c373951410a484533c64dd7b1e2e6357abbfde04 Mon Sep 17 00:00:00 2001 From: stefnotch Date: Tue, 21 Jan 2025 21:56:13 +0100 Subject: [PATCH 10/10] Report restarts in continuous run --- cli/lsp/testing/execution.rs | 112 ++++++++++++++++++++++------------ cli/lsp/testing/lsp_custom.rs | 3 + 2 files changed, 76 insertions(+), 39 deletions(-) diff --git a/cli/lsp/testing/execution.rs b/cli/lsp/testing/execution.rs index 0541dc2929f540..586681d0e39aad 100644 --- a/cli/lsp/testing/execution.rs +++ b/cli/lsp/testing/execution.rs @@ -435,9 +435,10 @@ impl TestRun { unreachable!("Should always be Test subcommand."); } }; - let (test_event_sender_factory, mut receiver) = create_test_event_channel(); - let test_event_sender_factory = Arc::new(test_event_sender_factory); + let (test_run_event_sender, mut receiver) = + tokio::sync::mpsc::unbounded_channel::(); + let mut first_run_token = Some(()); lsp_log!("Starting the setup"); let join_handle = file_watcher::watch_recv( flags.clone(), @@ -446,8 +447,19 @@ impl TestRun { self.token.clone(), move |flags, watcher_communicator, changed_paths| { lsp_log!("Running dem tests"); - let test_event_sender_factory = test_event_sender_factory.clone(); + let test_run_event_sender = test_run_event_sender.clone(); + let is_first_run = first_run_token.take().is_some(); Ok(async move { + let (test_event_sender_factory, receiver) = + create_test_event_channel(); + if is_first_run { + let _ = test_run_event_sender.send(TestRunEvent::Started(receiver)); + } else { + let _ = test_run_event_sender.send(TestRunEvent::Restarted( + receiver, + self.as_enqueued().await, + )); + } let (queue, filters) = { let tests = self.tests.lock().await; as_queue_and_filters( @@ -579,8 +591,18 @@ impl TestRun { let start_time = Instant::now(); lsp_log!("Handler startup"); - while let Some((_, event)) = receiver.recv().await { - test_event_handler.handle_event(event).await; + while let Some(test_run_event) = receiver.recv().await { + let mut test_event_receiver = match test_run_event { + TestRunEvent::Started(v) => v, + TestRunEvent::Restarted(v, enqueued) => { + test_event_handler.restart(enqueued); + v + } + }; + + while let Some((_, event)) = test_event_receiver.recv().await { + test_event_handler.handle_event(event).await; + } } test_event_handler.finish(start_time.elapsed())?; @@ -644,6 +666,11 @@ impl TestRun { } } +enum TestRunEvent { + Started(test::TestEventReceiver), + Restarted(test::TestEventReceiver, Vec), +} + #[derive(Debug, PartialEq)] enum LspTestDescription { /// `(desc, static_id)` @@ -708,12 +735,16 @@ struct LspTestEventHandler { reporter: Box, tests: Arc>>, test_steps: IndexMap, - summary: test::TestSummary, - tests_with_result: HashSet, + summary: test::TestSummary, // TODO: This can accumulate some very questionable values. like 22 failed, 1 total + tests_with_result: HashSet, // TODO: This is useless now used_only: bool, } impl LspTestEventHandler { + fn restart(&mut self, enqueued: Vec) { + self.reporter.report_restart(enqueued); + } + async fn handle_event(&mut self, event: test::TestEvent) { match event { test::TestEvent::Register(description) => { @@ -750,24 +781,24 @@ impl LspTestEventHandler { .report_slow(self.tests.read().get(&id).unwrap(), elapsed); } test::TestEvent::Result(id, result, elapsed) => { - if self.tests_with_result.insert(id) { - let description = self.tests.read().get(&id).unwrap().clone(); - match &result { - test::TestResult::Ok => self.summary.passed += 1, - test::TestResult::Ignored => self.summary.ignored += 1, - test::TestResult::Failed(error) => { - self.summary.failed += 1; - self - .summary - .failures - .push(((&description).into(), error.clone())); - } - test::TestResult::Cancelled => { - self.summary.failed += 1; - } + self.tests_with_result.insert(id); + // TODO: Dis surpresses duplicate test results + let description = self.tests.read().get(&id).unwrap().clone(); + match &result { + test::TestResult::Ok => self.summary.passed += 1, + test::TestResult::Ignored => self.summary.ignored += 1, + test::TestResult::Failed(error) => { + self.summary.failed += 1; + self + .summary + .failures + .push(((&description).into(), error.clone())); + } + test::TestResult::Cancelled => { + self.summary.failed += 1; } - self.reporter.report_result(&description, &result, elapsed); } + self.reporter.report_result(&description, &result, elapsed); } test::TestEvent::UncaughtError(origin, error) => { self.reporter.report_uncaught_error(&origin, &error); @@ -784,24 +815,23 @@ impl LspTestEventHandler { .report_step_wait(self.test_steps.get(&id).unwrap()); } test::TestEvent::StepResult(id, result, duration) => { - if self.tests_with_result.insert(id) { - match &result { - test::TestStepResult::Ok => { - self.summary.passed_steps += 1; - } - test::TestStepResult::Ignored => { - self.summary.ignored_steps += 1; - } - test::TestStepResult::Failed(_) => { - self.summary.failed_steps += 1; - } + self.tests_with_result.insert(id); + match &result { + test::TestStepResult::Ok => { + self.summary.passed_steps += 1; + } + test::TestStepResult::Ignored => { + self.summary.ignored_steps += 1; + } + test::TestStepResult::Failed(_) => { + self.summary.failed_steps += 1; } - self.reporter.report_step_result( - self.test_steps.get(&id).unwrap(), - &result, - duration, - ); } + self.reporter.report_step_result( + self.test_steps.get(&id).unwrap(), + &result, + duration, + ); } test::TestEvent::Completed => { self.reporter.report_completed(); @@ -1056,6 +1086,10 @@ impl LspTestReporter { } } + fn report_restart(&mut self, enqueued: Vec) { + self.progress(lsp_custom::TestRunProgressMessage::Restart { enqueued }) + } + fn report_completed(&mut self) { // there is nothing to do on report_completed } diff --git a/cli/lsp/testing/lsp_custom.rs b/cli/lsp/testing/lsp_custom.rs index 10711a9118c5c3..a185db565c389c 100644 --- a/cli/lsp/testing/lsp_custom.rs +++ b/cli/lsp/testing/lsp_custom.rs @@ -166,6 +166,9 @@ pub enum TestRunProgressMessage { #[serde(skip_serializing_if = "Option::is_none")] location: Option, }, + Restart { + enqueued: Vec, + }, End, }