From db287e216dd752bfcb3484cbfd93225e8463c363 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Thu, 3 Aug 2023 04:05:34 +0200 Subject: [PATCH] refactor: use '--reporter' and '--junit-path' flags for 'deno test' (#20031) This commit adds "--reporter" and "--junit-path" flags to "deno test" subcommand instead of using "--dot" and "--junit" flags. --- cli/args/flags.rs | 185 +++++++++++++--------------- cli/args/mod.rs | 2 + cli/tests/integration/test_tests.rs | 6 +- cli/tools/test/mod.rs | 28 ++--- 4 files changed, 103 insertions(+), 118 deletions(-) diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 6070ac831f..9674c68a61 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -223,8 +223,7 @@ pub enum TestReporterConfig { #[default] Pretty, Dot, - // Contains path to write to or "-" to print to stdout. - Junit(String), + Junit, } #[derive(Clone, Debug, Default, Eq, PartialEq)] @@ -241,6 +240,7 @@ pub struct TestFlags { pub trace_ops: bool, pub watch: Option, pub reporter: TestReporterConfig, + pub junit_path: Option, } #[derive(Clone, Debug, Eq, PartialEq)] @@ -1877,21 +1877,17 @@ Directory arguments are expanded to all contained files matching the glob .arg(no_clear_screen_arg()) .arg(script_arg().last(true)) .arg( - Arg::new("junit") - .long("junit") + Arg::new("junit-path") + .long("junit-path") .value_name("PATH") .value_hint(ValueHint::FilePath) .help("Write a JUnit XML test report to PATH. Use '-' to write to stdout which is the default when PATH is not provided.") - .num_args(0..=1) - .require_equals(true) - .default_missing_value("-") ) .arg( - Arg::new("dot-reporter") - .long("dot") - .conflicts_with("junit") - .help("Use 'dot' test reporter with a concise output format") - .action(ArgAction::SetTrue), + Arg::new("reporter") + .long("reporter") + .help("Select reporter to use. Default to 'pretty'.") + .value_parser(["pretty", "dot", "junit"]) ) ) } @@ -3093,20 +3089,24 @@ fn test_parse(flags: &mut Flags, matches: &mut ArgMatches) { Vec::new() }; - let junit_path = matches.remove_one::("junit"); - let dot_reporter = matches.get_flag("dot-reporter"); - if dot_reporter { + let junit_path = matches.remove_one::("junit-path"); + + let reporter = + if let Some(reporter) = matches.remove_one::("reporter") { + match reporter.as_str() { + "pretty" => TestReporterConfig::Pretty, + "junit" => TestReporterConfig::Junit, + "dot" => TestReporterConfig::Dot, + _ => unreachable!(), + } + } else { + TestReporterConfig::Pretty + }; + + if matches!(reporter, TestReporterConfig::Dot) { flags.log_level = Some(Level::Error); } - let reporter = if dot_reporter { - TestReporterConfig::Dot - } else if let Some(path) = junit_path { - TestReporterConfig::Junit(path) - } else { - TestReporterConfig::Pretty - }; - flags.subcommand = DenoSubcommand::Test(TestFlags { no_run, doc, @@ -3120,6 +3120,7 @@ fn test_parse(flags: &mut Flags, matches: &mut ArgMatches) { trace_ops, watch: watch_arg_parse(matches), reporter, + junit_path, }); } @@ -6024,6 +6025,7 @@ mod tests { coverage_dir: Some("cov".to_string()), watch: Default::default(), reporter: Default::default(), + junit_path: None, }), unstable: true, no_prompt: true, @@ -6103,6 +6105,7 @@ mod tests { trace_ops: false, coverage_dir: None, watch: Default::default(), + junit_path: None, }), type_check_mode: TypeCheckMode::Local, no_prompt: true, @@ -6136,6 +6139,7 @@ mod tests { coverage_dir: None, watch: Default::default(), reporter: Default::default(), + junit_path: None, }), type_check_mode: TypeCheckMode::Local, no_prompt: true, @@ -6173,6 +6177,7 @@ mod tests { coverage_dir: None, watch: Default::default(), reporter: Default::default(), + junit_path: None, }), no_prompt: true, type_check_mode: TypeCheckMode::Local, @@ -6183,27 +6188,28 @@ mod tests { } #[test] - fn test_dot() { - let r = flags_from_vec(svec!["deno", "test", "--dot"]); + fn test_reporter() { + let r = flags_from_vec(svec!["deno", "test", "--reporter=pretty"]); + assert_eq!( + r.unwrap(), + Flags { + subcommand: DenoSubcommand::Test(TestFlags { + reporter: TestReporterConfig::Pretty, + ..Default::default() + }), + no_prompt: true, + type_check_mode: TypeCheckMode::Local, + ..Flags::default() + } + ); + + let r = flags_from_vec(svec!["deno", "test", "--reporter=dot"]); assert_eq!( r.unwrap(), Flags { subcommand: DenoSubcommand::Test(TestFlags { - no_run: false, - doc: false, - fail_fast: None, - filter: None, reporter: TestReporterConfig::Dot, - allow_none: false, - shuffle: None, - files: FileFlags { - include: vec![], - ignore: vec![], - }, - concurrent_jobs: None, - trace_ops: false, - coverage_dir: None, - watch: Default::default(), + ..Default::default() }), no_prompt: true, type_check_mode: TypeCheckMode::Local, @@ -6212,7 +6218,42 @@ mod tests { } ); - let r = flags_from_vec(svec!["deno", "test", "--dot", "--junit"]); + let r = flags_from_vec(svec!["deno", "test", "--reporter=junit"]); + assert_eq!( + r.unwrap(), + Flags { + subcommand: DenoSubcommand::Test(TestFlags { + reporter: TestReporterConfig::Junit, + ..Default::default() + }), + no_prompt: true, + type_check_mode: TypeCheckMode::Local, + ..Flags::default() + } + ); + + let r = flags_from_vec(svec![ + "deno", + "test", + "--reporter=dot", + "--junit-path=report.xml" + ]); + assert_eq!( + r.unwrap(), + Flags { + subcommand: DenoSubcommand::Test(TestFlags { + reporter: TestReporterConfig::Dot, + junit_path: Some("report.xml".to_string()), + ..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", "--junit-path"]); assert!(r.is_err()); } @@ -6238,6 +6279,7 @@ mod tests { coverage_dir: None, watch: Default::default(), reporter: Default::default(), + junit_path: None, }), no_prompt: true, type_check_mode: TypeCheckMode::Local, @@ -6270,6 +6312,7 @@ mod tests { no_clear_screen: false, }), reporter: Default::default(), + junit_path: None, }), no_prompt: true, type_check_mode: TypeCheckMode::Local, @@ -6301,6 +6344,7 @@ mod tests { no_clear_screen: false, }), reporter: Default::default(), + junit_path: None, }), no_prompt: true, type_check_mode: TypeCheckMode::Local, @@ -6334,66 +6378,7 @@ mod tests { no_clear_screen: true, }), reporter: Default::default(), - }), - type_check_mode: TypeCheckMode::Local, - no_prompt: true, - ..Flags::default() - } - ); - } - - #[test] - fn test_junit_default() { - let r = flags_from_vec(svec!["deno", "test", "--junit"]); - assert_eq!( - r.unwrap(), - Flags { - subcommand: DenoSubcommand::Test(TestFlags { - no_run: false, - doc: false, - fail_fast: None, - filter: None, - allow_none: false, - shuffle: None, - files: FileFlags { - include: vec![], - ignore: vec![], - }, - concurrent_jobs: None, - trace_ops: false, - coverage_dir: None, - watch: Default::default(), - reporter: TestReporterConfig::Junit("-".to_string()), - }), - type_check_mode: TypeCheckMode::Local, - no_prompt: true, - ..Flags::default() - } - ); - } - - #[test] - fn test_junit_with_path() { - let r = flags_from_vec(svec!["deno", "test", "--junit=junit.xml"]); - assert_eq!( - r.unwrap(), - Flags { - subcommand: DenoSubcommand::Test(TestFlags { - no_run: false, - doc: false, - fail_fast: None, - filter: None, - allow_none: false, - shuffle: None, - files: FileFlags { - include: vec![], - ignore: vec![], - }, - concurrent_jobs: None, - trace_ops: false, - coverage_dir: None, - watch: Default::default(), - reporter: TestReporterConfig::Junit("junit.xml".to_string()), + junit_path: None, }), type_check_mode: TypeCheckMode::Local, no_prompt: true, diff --git a/cli/args/mod.rs b/cli/args/mod.rs index eb7eea44f6..cea0c0ca14 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -228,6 +228,7 @@ pub struct TestOptions { pub concurrent_jobs: NonZeroUsize, pub trace_ops: bool, pub reporter: TestReporterConfig, + pub junit_path: Option, } impl TestOptions { @@ -253,6 +254,7 @@ impl TestOptions { shuffle: test_flags.shuffle, trace_ops: test_flags.trace_ops, reporter: test_flags.reporter, + junit_path: test_flags.junit_path, }) } } diff --git a/cli/tests/integration/test_tests.rs b/cli/tests/integration/test_tests.rs index ec334aa6ad..57acb723ec 100644 --- a/cli/tests/integration/test_tests.rs +++ b/cli/tests/integration/test_tests.rs @@ -334,19 +334,19 @@ itest!(steps_ignored_steps { }); itest!(steps_dot_passing_steps { - args: "test --dot test/steps/passing_steps.ts", + args: "test --reporter=dot test/steps/passing_steps.ts", exit_code: 0, output: "test/steps/passing_steps.dot.out", }); itest!(steps_dot_failing_steps { - args: "test --dot test/steps/failing_steps.ts", + args: "test --reporter=dot test/steps/failing_steps.ts", exit_code: 1, output: "test/steps/failing_steps.dot.out", }); itest!(steps_dot_ignored_steps { - args: "test --dot test/steps/ignored_steps.ts", + args: "test --reporter=dot test/steps/ignored_steps.ts", exit_code: 0, output: "test/steps/ignored_steps.dot.out", }); diff --git a/cli/tools/test/mod.rs b/cli/tools/test/mod.rs index c3fb6f772f..101817ac9d 100644 --- a/cli/tools/test/mod.rs +++ b/cli/tools/test/mod.rs @@ -361,6 +361,7 @@ struct TestSpecifiersOptions { log_level: Option, specifier: TestSpecifierOptions, reporter: TestReporterConfig, + junit_path: Option, } #[derive(Debug, Clone)] @@ -394,28 +395,23 @@ impl TestSummary { fn get_test_reporter(options: &TestSpecifiersOptions) -> Box { let parallel = options.concurrent_jobs.get() > 1; - match &options.reporter { + let reporter: Box = match &options.reporter { TestReporterConfig::Dot => Box::new(DotTestReporter::new()), TestReporterConfig::Pretty => Box::new(PrettyTestReporter::new( parallel, options.log_level != Some(Level::Error), )), - TestReporterConfig::Junit(path) => { - let junit = Box::new(JunitTestReporter::new(path.clone())); - // If junit is writing to stdout, only enable the junit reporter - if path == "-" { - junit - } else { - Box::new(CompoundTestReporter::new(vec![ - Box::new(PrettyTestReporter::new( - parallel, - options.log_level != Some(Level::Error), - )), - junit, - ])) - } + TestReporterConfig::Junit => { + Box::new(JunitTestReporter::new("-".to_string())) } + }; + + if let Some(junit_path) = &options.junit_path { + let junit = Box::new(JunitTestReporter::new(junit_path.to_string())); + return Box::new(CompoundTestReporter::new(vec![reporter, junit])); } + + reporter } /// Test a single specifier as documentation containing test programs, an executable test module or @@ -1162,6 +1158,7 @@ pub async fn run_tests( fail_fast: test_options.fail_fast, log_level, reporter: test_options.reporter, + junit_path: test_options.junit_path, specifier: TestSpecifierOptions { filter: TestFilter::from_flag(&test_options.filter), shuffle: test_options.shuffle, @@ -1293,6 +1290,7 @@ pub async fn run_tests_with_watch( fail_fast: test_options.fail_fast, log_level, reporter: test_options.reporter, + junit_path: test_options.junit_path, specifier: TestSpecifierOptions { filter: TestFilter::from_flag(&test_options.filter), shuffle: test_options.shuffle,