From 69ca44d8e229255760740432ba5d2f95860a66bb Mon Sep 17 00:00:00 2001 From: Casper Beyer Date: Thu, 15 Jul 2021 03:05:16 +0800 Subject: [PATCH] refactor(cli/tools/test_runner): split reporter into distinct stages (#11395) This splits up the reporter into smaller functions, one for each distinct event that happens during the testing process. --- cli/ops/testing.rs | 41 +++----- cli/tools/test_runner.rs | 220 ++++++++++++++++++--------------------- runtime/js/40_testing.js | 53 +++++----- 3 files changed, 146 insertions(+), 168 deletions(-) diff --git a/cli/ops/testing.rs b/cli/ops/testing.rs index 8f6a4e2b82..cab498ab18 100644 --- a/cli/ops/testing.rs +++ b/cli/ops/testing.rs @@ -1,5 +1,4 @@ use crate::tools::test_runner::TestEvent; -use crate::tools::test_runner::TestMessage; use deno_core::error::generic_error; use deno_core::error::AnyError; use deno_core::JsRuntime; @@ -8,7 +7,6 @@ use deno_core::OpState; use deno_runtime::ops::worker_host::create_worker_permissions; use deno_runtime::ops::worker_host::PermissionsArg; use deno_runtime::permissions::Permissions; -use serde::Deserialize; use std::sync::mpsc::Sender; use uuid::Uuid; @@ -19,7 +17,8 @@ pub fn init(rt: &mut JsRuntime) { "op_restore_test_permissions", op_restore_test_permissions, ); - super::reg_sync(rt, "op_post_test_message", op_post_test_message); + super::reg_sync(rt, "op_get_test_origin", op_get_test_origin); + super::reg_sync(rt, "op_dispatch_test_event", op_dispatch_test_event); } #[derive(Clone)] @@ -65,27 +64,21 @@ pub fn op_restore_test_permissions( } } -#[derive(Debug, Deserialize)] -#[serde(rename_all = "camelCase")] -struct PostTestMessageArgs { - message: TestMessage, -} - -fn op_post_test_message( +fn op_get_test_origin( state: &mut OpState, - args: PostTestMessageArgs, _: (), -) -> Result { - let origin = state.borrow::().to_string(); - let message = args.message; - - let event = TestEvent { origin, message }; - - let sender = state.borrow::>().clone(); - - if sender.send(event).is_err() { - Ok(false) - } else { - Ok(true) - } + _: (), +) -> Result { + Ok(state.borrow::().to_string()) +} + +fn op_dispatch_test_event( + state: &mut OpState, + event: TestEvent, + _: (), +) -> Result<(), AnyError> { + let sender = state.borrow::>().clone(); + sender.send(event).ok(); + + Ok(()) } diff --git a/cli/tools/test_runner.rs b/cli/tools/test_runner.rs index 56102c1bf9..39d6bd62bd 100644 --- a/cli/tools/test_runner.rs +++ b/cli/tools/test_runner.rs @@ -35,6 +35,13 @@ use std::time::Duration; use std::time::Instant; use swc_common::comments::CommentKind; +#[derive(Debug, Clone, PartialEq, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct TestDescription { + pub origin: String, + pub name: String, +} + #[derive(Debug, Clone, PartialEq, Deserialize)] #[serde(rename_all = "camelCase")] pub enum TestResult { @@ -43,28 +50,21 @@ pub enum TestResult { Failed(String), } -#[derive(Debug, Clone, Deserialize)] -#[serde(tag = "kind", content = "data", rename_all = "camelCase")] -pub enum TestMessage { - Plan { - pending: usize, - filtered: usize, - only: bool, - }, - Wait { - name: String, - }, - Result { - name: String, - duration: usize, - result: TestResult, - }, +#[derive(Debug, Clone, PartialEq, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct TestPlan { + pub origin: String, + pub total: usize, + pub filtered_out: usize, + pub used_only: bool, } #[derive(Debug, Clone, Deserialize)] -pub struct TestEvent { - pub origin: String, - pub message: TestMessage, +#[serde(rename_all = "camelCase")] +pub enum TestEvent { + Plan(TestPlan), + Wait(TestDescription), + Result(TestDescription, TestResult, u64), } #[derive(Debug, Clone, Deserialize)] @@ -75,7 +75,7 @@ pub struct TestSummary { pub ignored: usize, pub filtered_out: usize, pub measured: usize, - pub failures: Vec<(String, String)>, + pub failures: Vec<(TestDescription, String)>, } impl TestSummary { @@ -101,8 +101,15 @@ impl TestSummary { } trait TestReporter { - fn visit_event(&mut self, event: TestEvent); - fn done(&mut self, summary: &TestSummary, elapsed: &Duration); + fn report_plan(&mut self, plan: &TestPlan); + fn report_wait(&mut self, description: &TestDescription); + fn report_result( + &mut self, + description: &TestDescription, + result: &TestResult, + elapsed: u64, + ); + fn report_summary(&mut self, summary: &TestSummary, elapsed: &Duration); } struct PrettyTestReporter { @@ -116,76 +123,52 @@ impl PrettyTestReporter { } impl TestReporter for PrettyTestReporter { - fn visit_event(&mut self, event: TestEvent) { - match &event.message { - TestMessage::Plan { - pending, - filtered: _, - only: _, - } => { - if *pending == 1 { - println!("running {} test from {}", pending, event.origin); - } else { - println!("running {} tests from {}", pending, event.origin); - } - } + fn report_plan(&mut self, plan: &TestPlan) { + let inflection = if plan.total == 1 { "test" } else { "tests" }; + println!("running {} {} from {}", plan.total, inflection, plan.origin); + } - TestMessage::Wait { name } => { - if !self.concurrent { - print!("test {} ...", name); - } - } - - TestMessage::Result { - name, - duration, - result, - } => { - if self.concurrent { - print!("test {} ...", name); - } - - match result { - TestResult::Ok => { - println!( - " {} {}", - colors::green("ok"), - colors::gray(format!("({}ms)", duration)) - ); - } - - TestResult::Ignored => { - println!( - " {} {}", - colors::yellow("ignored"), - colors::gray(format!("({}ms)", duration)) - ); - } - - TestResult::Failed(_) => { - println!( - " {} {}", - colors::red("FAILED"), - colors::gray(format!("({}ms)", duration)) - ); - } - } - } + fn report_wait(&mut self, description: &TestDescription) { + if !self.concurrent { + print!("test {} ...", description.name); } } - fn done(&mut self, summary: &TestSummary, elapsed: &Duration) { + fn report_result( + &mut self, + description: &TestDescription, + result: &TestResult, + elapsed: u64, + ) { + if self.concurrent { + print!("test {} ...", description.name); + } + + let status = match result { + TestResult::Ok => colors::green("ok").to_string(), + TestResult::Ignored => colors::yellow("ignored").to_string(), + TestResult::Failed(_) => colors::red("FAILED").to_string(), + }; + + println!( + " {} {}", + status, + colors::gray(format!("({}ms)", elapsed)).to_string() + ); + } + + fn report_summary(&mut self, summary: &TestSummary, elapsed: &Duration) { if !summary.failures.is_empty() { println!("\nfailures:\n"); - for (name, error) in &summary.failures { - println!("{}", name); + for (description, error) in &summary.failures { + println!("{}", description.name); println!("{}", error); println!(); } println!("failures:\n"); - for (name, _) in &summary.failures { - println!("\t{}", name); + for (description, _) in &summary.failures { + println!("\t{}", description.name); } } @@ -196,15 +179,15 @@ impl TestReporter for PrettyTestReporter { }; println!( - "\ntest result: {}. {} passed; {} failed; {} ignored; {} measured; {} filtered out {}\n", - status, - summary.passed, - summary.failed, - summary.ignored, - summary.measured, - summary.filtered_out, - colors::gray(format!("({}ms)", elapsed.as_millis())), - ); + "\ntest result: {}. {} passed; {} failed; {} ignored; {} measured; {} filtered out {}\n", + status, + summary.passed, + summary.failed, + summary.ignored, + summary.measured, + summary.filtered_out, + colors::gray(format!("({}ms)", elapsed.as_millis())), + ); } } @@ -522,43 +505,42 @@ pub async fn run_tests( let mut used_only = false; for event in receiver.iter() { - match event.message.clone() { - TestMessage::Plan { - pending, - filtered, - only, - } => { - summary.total += pending; - summary.filtered_out += filtered; + match event { + TestEvent::Plan(plan) => { + summary.total += plan.total; + summary.filtered_out += plan.filtered_out; - if only { + if plan.used_only { used_only = true; } + + reporter.report_plan(&plan); } - TestMessage::Result { - name, - duration: _, - result, - } => match result { - TestResult::Ok => { - summary.passed += 1; + TestEvent::Wait(description) => { + reporter.report_wait(&description); + } + + TestEvent::Result(description, result, elapsed) => { + match &result { + TestResult::Ok => { + summary.passed += 1; + } + + TestResult::Ignored => { + summary.ignored += 1; + } + + TestResult::Failed(error) => { + summary.failed += 1; + summary.failures.push((description.clone(), error.clone())); + } } - TestResult::Ignored => { - summary.ignored += 1; - } - - TestResult::Failed(error) => { - summary.failed += 1; - summary.failures.push((name.clone(), error.clone())); - } - }, - _ => {} + reporter.report_result(&description, &result, elapsed); + } } - reporter.visit_event(event); - if let Some(x) = fail_fast { if summary.failed >= x { break; @@ -567,7 +549,7 @@ pub async fn run_tests( } let elapsed = Instant::now().duration_since(earlier); - reporter.done(&summary, &elapsed); + reporter.report_summary(&summary, &elapsed); if used_only { println!( diff --git a/runtime/js/40_testing.js b/runtime/js/40_testing.js index 0d4c231209..617df22d49 100644 --- a/runtime/js/40_testing.js +++ b/runtime/js/40_testing.js @@ -186,10 +186,6 @@ finishing test case.`; ArrayPrototypePush(tests, testDef); } - function postTestMessage(kind, data) { - return core.opSync("op_post_test_message", { message: { kind, data } }); - } - function createTestFilter(filter) { return (def) => { if (filter) { @@ -223,25 +219,38 @@ finishing test case.`; } } + function getTestOrigin() { + return core.opSync("op_get_test_origin"); + } + + function dispatchTestEvent(event) { + return core.opSync("op_dispatch_test_event", event); + } + async function runTests({ disableLog = false, filter = null, shuffle = null, } = {}) { + const origin = getTestOrigin(); const originalConsole = globalThis.console; if (disableLog) { globalThis.console = new Console(() => {}); } const only = ArrayPrototypeFilter(tests, (test) => test.only); - const pending = ArrayPrototypeFilter( + const filtered = ArrayPrototypeFilter( (only.length > 0 ? only : tests), createTestFilter(filter), ); - postTestMessage("plan", { - filtered: tests.length - pending.length, - pending: pending.length, - only: only.length > 0, + + dispatchTestEvent({ + plan: { + origin, + total: filtered.length, + filteredOut: tests.length - filtered.length, + usedOnly: only.length > 0, + }, }); if (shuffle !== null) { @@ -256,31 +265,25 @@ finishing test case.`; }; }(shuffle)); - for (let i = pending.length - 1; i > 0; i--) { + for (let i = filtered.length - 1; i > 0; i--) { const j = nextInt(i); - [pending[i], pending[j]] = [pending[j], pending[i]]; + [filtered[i], filtered[j]] = [filtered[j], filtered[i]]; } } - for (const test of pending) { - const { - name, - } = test; - + for (const test of filtered) { + const description = { + origin, + name: test.name, + }; const earlier = DateNow(); - postTestMessage("wait", { - name, - }); + dispatchTestEvent({ wait: description }); const result = await runTest(test); - const duration = DateNow() - earlier; + const elapsed = DateNow() - earlier; - postTestMessage("result", { - name, - result, - duration, - }); + dispatchTestEvent({ result: [description, result, elapsed] }); } if (disableLog) {