From 198699fabae37fb3f1edd6aa058ea050cc43bb19 Mon Sep 17 00:00:00 2001
From: Casper Beyer <caspervonb@pm.me>
Date: Mon, 23 Aug 2021 18:35:38 +0800
Subject: [PATCH] fix(cli/flags): require a non zero usize for concurrent jobs
 (#11802)

---
 cli/flags.rs             | 46 +++++++++++++++++++++++++++++++---------
 cli/main.rs              |  3 ++-
 cli/tools/test_runner.rs |  7 +++---
 3 files changed, 42 insertions(+), 14 deletions(-)

diff --git a/cli/flags.rs b/cli/flags.rs
index 15d0cb6a29..86a5405b2f 100644
--- a/cli/flags.rs
+++ b/cli/flags.rs
@@ -13,6 +13,7 @@ use deno_runtime::permissions::PermissionsOptions;
 use log::debug;
 use log::Level;
 use std::net::SocketAddr;
+use std::num::NonZeroUsize;
 use std::path::PathBuf;
 use std::str::FromStr;
 
@@ -106,7 +107,7 @@ pub enum DenoSubcommand {
     include: Option<Vec<String>>,
     filter: Option<String>,
     shuffle: Option<u64>,
-    concurrent_jobs: usize,
+    concurrent_jobs: NonZeroUsize,
   },
   Types,
   Upgrade {
@@ -1103,9 +1104,9 @@ fn test_subcommand<'a, 'b>() -> App<'a, 'b> {
         .min_values(0)
         .max_values(1)
         .takes_value(true)
-        .validator(|val: String| match val.parse::<usize>() {
+        .validator(|val: String| match val.parse::<NonZeroUsize>() {
           Ok(_) => Ok(()),
-          Err(_) => Err("jobs should be a number".to_string()),
+          Err(_) => Err("jobs should be a non zero unsigned integer".to_string()),
         }),
     )
     .arg(
@@ -1820,10 +1821,10 @@ fn test_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
       value.parse().unwrap()
     } else {
       // TODO(caspervonb) drop the dependency on num_cpus when https://doc.rust-lang.org/std/thread/fn.available_concurrency.html becomes stable.
-      num_cpus::get()
+      NonZeroUsize::new(num_cpus::get()).unwrap()
     }
   } else {
-    1
+    NonZeroUsize::new(1).unwrap()
   };
 
   let include = if matches.is_present("files") {
@@ -3575,7 +3576,7 @@ mod tests {
           quiet: false,
           include: Some(svec!["dir1/", "dir2/"]),
           shuffle: None,
-          concurrent_jobs: 1,
+          concurrent_jobs: NonZeroUsize::new(1).unwrap(),
         },
         unstable: true,
         coverage_dir: Some("cov".to_string()),
@@ -3628,6 +3629,31 @@ mod tests {
     );
   }
 
+  #[test]
+  fn test_with_concurrent_jobs() {
+    let r = flags_from_vec(svec!["deno", "test", "--jobs=4"]);
+    assert_eq!(
+      r.unwrap(),
+      Flags {
+        subcommand: DenoSubcommand::Test {
+          no_run: false,
+          doc: false,
+          fail_fast: None,
+          filter: None,
+          allow_none: false,
+          quiet: false,
+          shuffle: None,
+          include: None,
+          concurrent_jobs: NonZeroUsize::new(4).unwrap(),
+        },
+        ..Flags::default()
+      }
+    );
+
+    let r = flags_from_vec(svec!["deno", "test", "--jobs=0"]);
+    assert!(r.is_err());
+  }
+
   #[test]
   fn test_with_fail_fast() {
     let r = flags_from_vec(svec!["deno", "test", "--fail-fast=3"]);
@@ -3643,7 +3669,7 @@ mod tests {
           quiet: false,
           shuffle: None,
           include: None,
-          concurrent_jobs: 1,
+          concurrent_jobs: NonZeroUsize::new(1).unwrap(),
         },
         ..Flags::default()
       }
@@ -3669,7 +3695,7 @@ mod tests {
           quiet: false,
           shuffle: None,
           include: None,
-          concurrent_jobs: 1,
+          concurrent_jobs: NonZeroUsize::new(1).unwrap(),
         },
         enable_testing_features: true,
         ..Flags::default()
@@ -3692,7 +3718,7 @@ mod tests {
           quiet: false,
           shuffle: Some(1),
           include: None,
-          concurrent_jobs: 1,
+          concurrent_jobs: NonZeroUsize::new(1).unwrap(),
         },
         watch: false,
         ..Flags::default()
@@ -3715,7 +3741,7 @@ mod tests {
           quiet: false,
           shuffle: None,
           include: None,
-          concurrent_jobs: 1,
+          concurrent_jobs: NonZeroUsize::new(1).unwrap(),
         },
         watch: true,
         ..Flags::default()
diff --git a/cli/main.rs b/cli/main.rs
index 0e75257bdb..0c758e6316 100644
--- a/cli/main.rs
+++ b/cli/main.rs
@@ -1,6 +1,7 @@
 // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license.
 
 mod ast;
+use std::num::NonZeroUsize;
 mod auth_tokens;
 mod checksum;
 mod colors;
@@ -1008,7 +1009,7 @@ async fn test_command(
   allow_none: bool,
   filter: Option<String>,
   shuffle: Option<u64>,
-  concurrent_jobs: usize,
+  concurrent_jobs: NonZeroUsize,
 ) -> Result<(), AnyError> {
   if let Some(ref coverage_dir) = flags.coverage_dir {
     std::fs::create_dir_all(&coverage_dir)?;
diff --git a/cli/tools/test_runner.rs b/cli/tools/test_runner.rs
index 968378fdcd..d2612e59dc 100644
--- a/cli/tools/test_runner.rs
+++ b/cli/tools/test_runner.rs
@@ -26,6 +26,7 @@ use rand::seq::SliceRandom;
 use rand::SeedableRng;
 use regex::Regex;
 use serde::Deserialize;
+use std::num::NonZeroUsize;
 use std::path::PathBuf;
 use std::sync::mpsc::channel;
 use std::sync::mpsc::Sender;
@@ -473,7 +474,7 @@ pub async fn run_tests(
   allow_none: bool,
   filter: Option<String>,
   shuffle: Option<u64>,
-  concurrent_jobs: usize,
+  concurrent_jobs: NonZeroUsize,
 ) -> Result<(), AnyError> {
   if !allow_none && doc_modules.is_empty() && test_modules.is_empty() {
     return Err(generic_error("No test modules found"));
@@ -572,10 +573,10 @@ pub async fn run_tests(
   });
 
   let join_stream = stream::iter(join_handles)
-    .buffer_unordered(concurrent_jobs)
+    .buffer_unordered(concurrent_jobs.get())
     .collect::<Vec<Result<Result<(), AnyError>, tokio::task::JoinError>>>();
 
-  let mut reporter = create_reporter(concurrent_jobs > 1);
+  let mut reporter = create_reporter(concurrent_jobs.get() > 1);
   let handler = {
     tokio::task::spawn_blocking(move || {
       let earlier = Instant::now();