From a526cff0a9888a475d5b542efda443fe720a93d0 Mon Sep 17 00:00:00 2001 From: Valentin Anger Date: Sat, 26 Aug 2023 01:19:23 +0200 Subject: [PATCH] feat(cli/tools): add TAP test reporter (#14390) (#20073) This PR adds a test reporter for the [Test Anything Protocol](https://testanything.org). It makes the following implementation decisions: - No TODO pragma, as there is no such marker in `Deno.test` - SKIP pragma for `ignore`d tests - Test steps are treated as TAP14 subtests - Support for this in consumers seems spotty - Some consumers will incorrectly interpret these markers, resulting in unexpected output - Considering the lack of support, and to avoid implementation complexity, subtests are at most one level deep (all test steps are in the same subtest) - To accommodate consumers that use comments to indicate test-suites (unspecced) - The test module path is output as a comment - This is disabled for `--parallel` testing - Failure diagnostics are output as JSON, which is also valid YAML - The structure is not specified, so the format roughly follows the spec example: ``` --- message: "Failed with error 'hostname peebles.example.com not found'" severity: fail found: hostname: 'peebles.example.com' address: ~ wanted: hostname: 'peebles.example.com' address: '85.193.201.85' at: file: test/dns-resolve.c line: 142 ... ``` --- cli/args/flags.rs | 21 +- cli/tests/integration/test_tests.rs | 19 ++ .../testdata/test/steps/failing_steps.tap.out | 43 +++ .../testdata/test/steps/ignored_steps.tap.out | 8 + .../testdata/test/steps/passing_steps.tap.out | 42 +++ cli/tools/test/mod.rs | 4 + cli/tools/test/reporters/mod.rs | 2 + cli/tools/test/reporters/tap.rs | 247 ++++++++++++++++++ 8 files changed, 384 insertions(+), 2 deletions(-) create mode 100644 cli/tests/testdata/test/steps/failing_steps.tap.out create mode 100644 cli/tests/testdata/test/steps/ignored_steps.tap.out create mode 100644 cli/tests/testdata/test/steps/passing_steps.tap.out create mode 100644 cli/tools/test/reporters/tap.rs diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 5441238873..773c95fec1 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -225,6 +225,7 @@ pub enum TestReporterConfig { Pretty, Dot, Junit, + Tap, } #[derive(Clone, Debug, Default, Eq, PartialEq)] @@ -1980,7 +1981,7 @@ Directory arguments are expanded to all contained files matching the glob Arg::new("reporter") .long("reporter") .help("Select reporter to use. Default to 'pretty'.") - .value_parser(["pretty", "dot", "junit"]) + .value_parser(["pretty", "dot", "junit", "tap"]) ) ) } @@ -3369,13 +3370,14 @@ fn test_parse(flags: &mut Flags, matches: &mut ArgMatches) { "pretty" => TestReporterConfig::Pretty, "junit" => TestReporterConfig::Junit, "dot" => TestReporterConfig::Dot, + "tap" => TestReporterConfig::Tap, _ => unreachable!(), } } else { TestReporterConfig::Pretty }; - if matches!(reporter, TestReporterConfig::Dot) { + if matches!(reporter, TestReporterConfig::Dot | TestReporterConfig::Tap) { flags.log_level = Some(Level::Error); } @@ -6839,6 +6841,21 @@ mod tests { } ); + let r = flags_from_vec(svec!["deno", "test", "--reporter=tap"]); + assert_eq!( + r.unwrap(), + Flags { + subcommand: DenoSubcommand::Test(TestFlags { + reporter: TestReporterConfig::Tap, + ..Default::default() + }), + no_prompt: true, + type_check_mode: TypeCheckMode::Local, + log_level: Some(Level::Error), + ..Flags::default() + } + ); + let r = flags_from_vec(svec![ "deno", "test", diff --git a/cli/tests/integration/test_tests.rs b/cli/tests/integration/test_tests.rs index 04465dd53e..030da05e1e 100644 --- a/cli/tests/integration/test_tests.rs +++ b/cli/tests/integration/test_tests.rs @@ -356,6 +356,25 @@ itest!(steps_dot_ignored_steps { output: "test/steps/ignored_steps.dot.out", }); +itest!(steps_tap_passing_steps { + args: "test --reporter=tap test/steps/passing_steps.ts", + exit_code: 0, + output: "test/steps/passing_steps.tap.out", +}); + +itest!(steps_tap_failing_steps { + args: "test --reporter=tap test/steps/failing_steps.ts", + exit_code: 1, + envs: vec![("NO_COLOR".to_owned(), "1".to_owned())], + output: "test/steps/failing_steps.tap.out", +}); + +itest!(steps_tap_ignored_steps { + args: "test --reporter=tap test/steps/ignored_steps.ts", + exit_code: 0, + output: "test/steps/ignored_steps.tap.out", +}); + itest!(steps_invalid_usage { args: "test test/steps/invalid_usage.ts", exit_code: 1, diff --git a/cli/tests/testdata/test/steps/failing_steps.tap.out b/cli/tests/testdata/test/steps/failing_steps.tap.out new file mode 100644 index 0000000000..11b289f08c --- /dev/null +++ b/cli/tests/testdata/test/steps/failing_steps.tap.out @@ -0,0 +1,43 @@ +TAP version 14 +# ./test/steps/failing_steps.ts +# Subtest: nested failure + not ok 1 - inner 1 + --- + {"message":"Error: Failed.\n throw new Error(\"Failed.\");\n ^\n at [WILDCARD]/failing_steps.ts:[WILDCARD]\n[WILDCARD]","severity":"fail","at":{"file":"./test/steps/failing_steps.ts","line":[WILDCARD]}} + ... + ok 2 - inner 2 + not ok 3 - step 1 + --- + {"message":"1 test step failed.","severity":"fail","at":{"file":"./test/steps/failing_steps.ts","line":[WILDCARD]}} + ... + 1..3 +not ok 1 - nested failure + --- + {"message":"1 test step failed.","severity":"fail","at":{"file":"./test/steps/failing_steps.ts","line":[WILDCARD]}} + ... +# Subtest: multiple test step failures + not ok 1 - step 1 + --- + {"message":"Error: Fail.\n throw new Error(\"Fail.\");\n ^\n at [WILDCARD]/failing_steps.ts:[WILDCARD]\n[WILDCARD]","severity":"fail","at":{"file":"./test/steps/failing_steps.ts","line":[WILDCARD]}} + ... + not ok 2 - step 2 + --- + {"message":"Error: Fail.\n await t.step(\"step 2\", () => Promise.reject(new Error(\"Fail.\")));\n ^\n at [WILDCARD]/failing_steps.ts:[WILDCARD]\n[WILDCARD]","severity":"fail","at":{"file":"./test/steps/failing_steps.ts","line":[WILDCARD]}} + ... + 1..2 +not ok 2 - multiple test step failures + --- + {"message":"2 test steps failed.","severity":"fail","at":{"file":"./test/steps/failing_steps.ts","line":[WILDCARD]}} + ... +# Subtest: failing step in failing test + not ok 1 - step 1 + --- + {"message":"Error: Fail.\n throw new Error(\"Fail.\");\n ^\n at [WILDCARD]/failing_steps.ts:[WILDCARD]\n[WILDCARD]","severity":"fail","at":{"file":"./test/steps/failing_steps.ts","line":[WILDCARD]}} + ... + 1..1 +not ok 3 - failing step in failing test + --- + {"message":"Error: Fail test.\n throw new Error(\"Fail test.\");\n ^\n at [WILDCARD]/failing_steps.ts:[WILDCARD]","severity":"fail","at":{"file":"./test/steps/failing_steps.ts","line":[WILDCARD]}} + ... +1..3 +error: Test failed diff --git a/cli/tests/testdata/test/steps/ignored_steps.tap.out b/cli/tests/testdata/test/steps/ignored_steps.tap.out new file mode 100644 index 0000000000..b2b2f50703 --- /dev/null +++ b/cli/tests/testdata/test/steps/ignored_steps.tap.out @@ -0,0 +1,8 @@ +TAP version 14 +# ./test/steps/ignored_steps.ts +# Subtest: ignored step + ok 1 - step 1 # SKIP + ok 2 - step 2 + 1..2 +ok 1 - ignored step +1..1 diff --git a/cli/tests/testdata/test/steps/passing_steps.tap.out b/cli/tests/testdata/test/steps/passing_steps.tap.out new file mode 100644 index 0000000000..20a9fa312f --- /dev/null +++ b/cli/tests/testdata/test/steps/passing_steps.tap.out @@ -0,0 +1,42 @@ +TAP version 14 +# ./test/steps/passing_steps.ts +# Subtest: description + ok 1 - inner 1 + ok 2 - inner 2 + ok 3 - step 1 + 1..3 +ok 1 - description +# Subtest: description function as first arg + ok 1 - inner1 + ok 2 - inner1 + ok 3 - step1 + 1..3 +ok 2 - description function as first arg +# Subtest: parallel steps without sanitizers + ok 1 - step 1 + ok 2 - step 2 + 1..2 +ok 3 - parallel steps without sanitizers +# Subtest: parallel steps without sanitizers due to parent + ok 1 - step 1 + ok 2 - step 2 + 1..2 +ok 4 - parallel steps without sanitizers due to parent +# Subtest: steps with disabled sanitizers, then enabled, then parallel disabled + ok 1 - step 2 + ok 2 - step 1 + ok 3 - step 1 + ok 4 - step 1 + ok 5 - step 1 + ok 6 - step 1 + 1..6 +ok 5 - steps with disabled sanitizers, then enabled, then parallel disabled +# Subtest: steps buffered then streaming reporting + ok 1 - step 1 - 2 - 1 + ok 2 - step 1 - 2 + ok 3 - step 1 - 1 + ok 4 - step 1 + ok 5 - step 2 + 1..5 +ok 6 - steps buffered then streaming reporting +1..6 diff --git a/cli/tools/test/mod.rs b/cli/tools/test/mod.rs index 365b832686..9ad205e2e7 100644 --- a/cli/tools/test/mod.rs +++ b/cli/tools/test/mod.rs @@ -89,6 +89,7 @@ use reporters::CompoundTestReporter; use reporters::DotTestReporter; use reporters::JunitTestReporter; use reporters::PrettyTestReporter; +use reporters::TapTestReporter; use reporters::TestReporter; /// The test mode is used to determine how a specifier is to be tested. @@ -404,6 +405,9 @@ fn get_test_reporter(options: &TestSpecifiersOptions) -> Box { TestReporterConfig::Junit => { Box::new(JunitTestReporter::new("-".to_string())) } + TestReporterConfig::Tap => Box::new(TapTestReporter::new( + options.concurrent_jobs > NonZeroUsize::new(1).unwrap(), + )), }; if let Some(junit_path) = &options.junit_path { diff --git a/cli/tools/test/reporters/mod.rs b/cli/tools/test/reporters/mod.rs index a95c729dd6..35d2776e4f 100644 --- a/cli/tools/test/reporters/mod.rs +++ b/cli/tools/test/reporters/mod.rs @@ -7,11 +7,13 @@ mod compound; mod dot; mod junit; mod pretty; +mod tap; pub use compound::CompoundTestReporter; pub use dot::DotTestReporter; pub use junit::JunitTestReporter; pub use pretty::PrettyTestReporter; +pub use tap::TapTestReporter; pub trait TestReporter { fn report_register(&mut self, description: &TestDescription); diff --git a/cli/tools/test/reporters/tap.rs b/cli/tools/test/reporters/tap.rs new file mode 100644 index 0000000000..c40c1eed70 --- /dev/null +++ b/cli/tools/test/reporters/tap.rs @@ -0,0 +1,247 @@ +// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. + +use deno_core::serde_json::json; +use deno_core::serde_json::{self}; +use serde::Serialize; + +use super::common; +use super::fmt::to_relative_path_or_remote_url; +use super::*; + +const VERSION_HEADER: &str = "TAP version 14"; + +/// A test reporter for the Test Anything Protocol as defined at +/// https://testanything.org/tap-version-14-specification.html +pub struct TapTestReporter { + cwd: Url, + is_concurrent: bool, + header: bool, + planned: usize, + n: usize, + step_n: usize, + step_results: HashMap>, +} + +impl TapTestReporter { + pub fn new(is_concurrent: bool) -> TapTestReporter { + TapTestReporter { + cwd: Url::from_directory_path(std::env::current_dir().unwrap()).unwrap(), + is_concurrent, + header: false, + planned: 0, + n: 0, + step_n: 0, + step_results: HashMap::new(), + } + } + + fn escape_description(description: &str) -> String { + description + .replace('\\', "\\\\") + .replace('\n', "\\n") + .replace('\r', "\\r") + .replace('#', "\\#") + } + + fn print_diagnostic( + indent: usize, + failure: &TestFailure, + location: DiagnosticLocation, + ) { + // Unspecified behaviour: + // The diagnostic schema is not specified by the TAP spec, + // but there is an example, so we use it. + + // YAML is a superset of JSON, so we can avoid a YAML dependency here. + // This makes the output less readable though. + let diagnostic = serde_json::to_string(&json!({ + "message": failure.to_string(), + "severity": "fail".to_string(), + "at": location, + })) + .expect("failed to serialize TAP diagnostic"); + println!("{:indent$} ---", "", indent = indent); + println!("{:indent$} {}", "", diagnostic, indent = indent); + println!("{:indent$} ...", "", indent = indent); + } + + fn print_line( + indent: usize, + status: &str, + step: usize, + description: &str, + directive: &str, + ) { + println!( + "{:indent$}{} {} - {}{}", + "", + status, + step, + Self::escape_description(description), + directive, + indent = indent + ); + } + + fn print_step_result( + &mut self, + desc: &TestStepDescription, + result: &TestStepResult, + ) { + if self.step_n == 0 { + println!("# Subtest: {}", desc.root_name) + } + + let (status, directive) = match result { + TestStepResult::Ok => ("ok", ""), + TestStepResult::Ignored => ("ok", " # SKIP"), + TestStepResult::Failed(_failure) => ("not ok", ""), + }; + self.step_n += 1; + Self::print_line(4, status, self.step_n, &desc.name, directive); + + if let TestStepResult::Failed(failure) = result { + Self::print_diagnostic( + 4, + failure, + DiagnosticLocation { + file: to_relative_path_or_remote_url(&self.cwd, &desc.origin), + line: desc.location.line_number, + }, + ); + } + } +} + +impl TestReporter for TapTestReporter { + fn report_register(&mut self, _description: &TestDescription) {} + + fn report_plan(&mut self, plan: &TestPlan) { + if !self.header { + println!("{}", VERSION_HEADER); + self.header = true; + } + self.planned += plan.total; + + if !self.is_concurrent { + // Unspecified behavior: Consumers tend to interpret a comment as a test suite name. + // During concurrent execution these would not correspond to the actual test file, so skip them. + println!( + "# {}", + to_relative_path_or_remote_url(&self.cwd, &plan.origin) + ) + } + } + + fn report_wait(&mut self, _description: &TestDescription) { + // flush for faster feedback when line buffered + std::io::stdout().flush().unwrap(); + } + + fn report_output(&mut self, _output: &[u8]) {} + + fn report_result( + &mut self, + description: &TestDescription, + result: &TestResult, + _elapsed: u64, + ) { + if self.is_concurrent { + let results = self.step_results.remove(&description.id); + for (desc, result) in results.iter().flat_map(|v| v.iter()) { + self.print_step_result(desc, result); + } + } + + if self.step_n != 0 { + println!(" 1..{}", self.step_n); + self.step_n = 0; + } + + let (status, directive) = match result { + TestResult::Ok => ("ok", ""), + TestResult::Ignored => ("ok", " # SKIP"), + TestResult::Failed(_failure) => ("not ok", ""), + TestResult::Cancelled => ("not ok", ""), + }; + self.n += 1; + Self::print_line(0, status, self.n, &description.name, directive); + + if let TestResult::Failed(failure) = result { + Self::print_diagnostic( + 0, + failure, + DiagnosticLocation { + file: to_relative_path_or_remote_url(&self.cwd, &description.origin), + line: description.location.line_number, + }, + ); + } + } + + fn report_uncaught_error(&mut self, _origin: &str, _errorr: Box) {} + + fn report_step_register(&mut self, _description: &TestStepDescription) {} + + fn report_step_wait(&mut self, _description: &TestStepDescription) { + // flush for faster feedback when line buffered + std::io::stdout().flush().unwrap(); + } + + fn report_step_result( + &mut self, + desc: &TestStepDescription, + result: &TestStepResult, + _elapsed: u64, + _tests: &IndexMap, + _test_steps: &IndexMap, + ) { + if self.is_concurrent { + // All subtests must be reported immediately before the parent test. + // So during concurrent execution we need to defer printing the results. + // TODO(SyrupThinker) This only outputs one level of subtests, it could support multiple. + self + .step_results + .entry(desc.root_id) + .or_default() + .push((desc.clone(), result.clone())); + return; + } + + self.print_step_result(desc, result); + } + + fn report_summary( + &mut self, + _elapsed: &Duration, + _tests: &IndexMap, + _test_steps: &IndexMap, + ) { + println!("1..{}", self.planned); + } + + fn report_sigint( + &mut self, + tests_pending: &HashSet, + tests: &IndexMap, + test_steps: &IndexMap, + ) { + println!("Bail out! SIGINT received."); + common::report_sigint(&self.cwd, tests_pending, tests, test_steps); + } + + fn flush_report( + &mut self, + _elapsed: &Duration, + _tests: &IndexMap, + _test_steps: &IndexMap, + ) -> anyhow::Result<()> { + Ok(()) + } +} + +#[derive(Serialize)] +struct DiagnosticLocation { + file: String, + line: u32, +}