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

@@ -223,8 +223,7 @@ pub enum TestReporterConfig {
   #[default]
   Pretty,
   Dot,
-  // Contains path to write to or "-" to print to stdout.
+  Junit,
-  Junit(String),
 }
 
 #[derive(Clone, Debug, Default, Eq, PartialEq)]
@@ -241,6 +240,7 @@ pub struct TestFlags {
   pub trace_ops: bool,
   pub watch: Option<WatchFlags>,
   pub reporter: TestReporterConfig,
+  pub junit_path: Option<String>,
 }
 
 #[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")
+      Arg::new("junit-path")
-        .long("junit")
+        .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")
+      Arg::new("reporter")
-        .long("dot")
+        .long("reporter")
-        .conflicts_with("junit")
+        .help("Select reporter to use. Default to 'pretty'.")
-        .help("Use 'dot' test reporter with a concise output format")
+        .value_parser(["pretty", "dot", "junit"])
-        .action(ArgAction::SetTrue),
     )
   )
 }
@@ -3093,20 +3089,24 @@ fn test_parse(flags: &mut Flags, matches: &mut ArgMatches) {
     Vec::new()
   };
 
-  let junit_path = matches.remove_one::<String>("junit");
+  let junit_path = matches.remove_one::<String>("junit-path");
-  let dot_reporter = matches.get_flag("dot-reporter");
+
-  if dot_reporter {
+  let reporter =
+    if let Some(reporter) = matches.remove_one::<String>("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() {
+  fn test_reporter() {
-    let r = flags_from_vec(svec!["deno", "test", "--dot"]);
+    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,
+          ..Default::default()
-          shuffle: None,
-          files: FileFlags {
-            include: vec![],
-            ignore: vec![],
-          },
-          concurrent_jobs: None,
-          trace_ops: false,
-          coverage_dir: None,
-          watch: 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(),
-        }),
+          junit_path: None,
-        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()),
         }),
         type_check_mode: TypeCheckMode::Local,
         no_prompt: true,

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<String>,
 }
 
 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,
     })
   }
 }

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",
 });

cli/tools/test/mod.rs

@@ -361,6 +361,7 @@ struct TestSpecifiersOptions {
   log_level: Option<log::Level>,
   specifier: TestSpecifierOptions,
   reporter: TestReporterConfig,
+  junit_path: Option<String>,
 }
 
 #[derive(Debug, Clone)]
@@ -394,28 +395,23 @@ impl TestSummary {
 
 fn get_test_reporter(options: &TestSpecifiersOptions) -> Box<dyn TestReporter> {
   let parallel = options.concurrent_jobs.get() > 1;
-  match &options.reporter {
+  let reporter: Box<dyn TestReporter> = 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) => {
+    TestReporterConfig::Junit => {
-      let junit = Box::new(JunitTestReporter::new(path.clone()));
+      Box::new(JunitTestReporter::new("-".to_string()))
-      // 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,
-        ]))
-      }
     }
+  };
+
+  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,