From c3f4a31af96da04d3867ae49456bb56ce92733d4 Mon Sep 17 00:00:00 2001 From: mrkldshv Date: Sun, 17 Jul 2022 18:37:29 +0000 Subject: [PATCH 1/6] fix: add deprecation notice and require equals for `jobs` flag --- cli/args/flags.rs | 5 +++++ .../test/short-pass-jobs-flag-with-numeric-value.out | 1 + 2 files changed, 6 insertions(+) diff --git a/cli/args/flags.rs b/cli/args/flags.rs index c6fbb320b25870..531fa75718ab62 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -1556,6 +1556,7 @@ fn test_subcommand<'a>() -> Command<'a> { .min_values(0) .max_values(1) .takes_value(true) + .require_equals(true) .validator(|val: &str| match val.parse::() { Ok(_) => Ok(()), Err(_) => Err("jobs should be a non zero unsigned integer".to_string()), @@ -2669,6 +2670,10 @@ fn test_parse(flags: &mut Flags, matches: &clap::ArgMatches) { let concurrent_jobs = if matches.is_present("jobs") { if let Some(value) = matches.value_of("jobs") { + println!( + "{}", + crate::colors::yellow("Warning: --jobs flag with numeric value is deprecated. Use 'DENO_JOBS' environment variable with --jobs flag instead."), + ); value.parse().unwrap() } else if let Ok(value) = env::var("DENO_JOBS") { value diff --git a/cli/tests/testdata/test/short-pass-jobs-flag-with-numeric-value.out b/cli/tests/testdata/test/short-pass-jobs-flag-with-numeric-value.out index 09b72d5fd90170..4ee49166ad583c 100644 --- a/cli/tests/testdata/test/short-pass-jobs-flag-with-numeric-value.out +++ b/cli/tests/testdata/test/short-pass-jobs-flag-with-numeric-value.out @@ -1,3 +1,4 @@ +Warning: --jobs flag with numeric value is deprecated. Use 'DENO_JOBS' environment variable with --jobs flag instead. Check [WILDCARD]/test/short-pass.ts running 1 test from ./test/short-pass.ts test ... ok ([WILDCARD]) From 39f82fd7ce65fb06b04bebd0d6277e947c407da3 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 20 Jul 2022 11:41:04 -0400 Subject: [PATCH 2/6] Add `--parallel` flag. --- cli/args/flags.rs | 36 ++++++++++++------- cli/tests/integration/test_tests.rs | 21 ++++++----- ...hort-pass-jobs-flag-with-jobs-warning.out} | 2 +- 3 files changed, 34 insertions(+), 25 deletions(-) rename cli/tests/testdata/test/{short-pass-jobs-flag-with-numeric-value.out => short-pass-jobs-flag-with-jobs-warning.out} (54%) diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 14ba1900e24fa4..0e9d4491efb75d 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -480,10 +480,9 @@ static ENV_VARIABLES_HELP: &str = r#"ENVIRONMENT VARIABLES: DENO_NO_PROMPT Set to disable permission prompts on access (alternative to passing --no-prompt on invocation) DENO_WEBGPU_TRACE Directory to use for wgpu traces - DENO_JOBS Number of parallel workers used for test subcommand. - Defaults to number of available CPUs when used with - --jobs flag and no value is provided. - Defaults to 1 when --jobs flag is not used. + DENO_JOBS Number of parallel workers used for the --parallel + flag with the test subcommand. Defaults to number + of available CPUs. HTTP_PROXY Proxy address for HTTP requests (module downloads, fetch) HTTPS_PROXY Proxy address for HTTPS requests @@ -1548,6 +1547,13 @@ fn test_subcommand<'a>() -> Command<'a> { .conflicts_with("inspect-brk") .help("UNSTABLE: Collect coverage profile data into DIR"), ) + .arg( + Arg::new("parallel") + .long("parallel") + .help("Runs the tests in parallel. Parallelism defaults to the number of available CPUs or the value in the DENO_JOBS environment variable.") + .conflicts_with("jobs") + .takes_value(false) + ) .arg( Arg::new("jobs") .short('j') @@ -1556,7 +1562,6 @@ fn test_subcommand<'a>() -> Command<'a> { .min_values(0) .max_values(1) .takes_value(true) - .require_equals(true) .validator(|val: &str| match val.parse::() { Ok(_) => Ok(()), Err(_) => Err("jobs should be a non zero unsigned integer".to_string()), @@ -2668,14 +2673,8 @@ fn test_parse(flags: &mut Flags, matches: &clap::ArgMatches) { } } - let concurrent_jobs = if matches.is_present("jobs") { - if let Some(value) = matches.value_of("jobs") { - println!( - "{}", - crate::colors::yellow("Warning: --jobs flag with numeric value is deprecated. Use 'DENO_JOBS' environment variable with --jobs flag instead."), - ); - value.parse().unwrap() - } else if let Ok(value) = env::var("DENO_JOBS") { + let concurrent_jobs = if matches.is_present("parallel") { + if let Ok(value) = env::var("DENO_JOBS") { value .parse::() .unwrap_or(NonZeroUsize::new(1).unwrap()) @@ -2683,6 +2682,17 @@ fn test_parse(flags: &mut Flags, matches: &clap::ArgMatches) { std::thread::available_parallelism() .unwrap_or(NonZeroUsize::new(1).unwrap()) } + } else if matches.is_present("jobs") { + println!( + "{}", + crate::colors::yellow("Warning: --jobs flag is deprecated. Use the --parallel flag with possibly the 'DENO_JOBS' environment variable."), + ); + if let Some(value) = matches.value_of("jobs") { + value.parse().unwrap() + } else { + std::thread::available_parallelism() + .unwrap_or(NonZeroUsize::new(1).unwrap()) + } } else { NonZeroUsize::new(1).unwrap() }; diff --git a/cli/tests/integration/test_tests.rs b/cli/tests/integration/test_tests.rs index 32be3c127fecce..d2efdd023c1a06 100644 --- a/cli/tests/integration/test_tests.rs +++ b/cli/tests/integration/test_tests.rs @@ -80,30 +80,29 @@ itest!(test_with_malformed_config { output: "test/collect_with_malformed_config.out", }); -itest!(jobs_flag { - args: "test test/short-pass.ts --jobs", +itest!(parallel_flag { + args: "test test/short-pass.ts --parallel", exit_code: 0, output: "test/short-pass.out", }); -itest!(jobs_flag_with_numeric_value { - args: "test test/short-pass.ts --jobs=2", +itest!(parallel_flag_with_env_variable { + args: "test test/short-pass.ts --parallel", + envs: vec![("DENO_JOBS".to_owned(), "2".to_owned())], exit_code: 0, - output: "test/short-pass-jobs-flag-with-numeric-value.out", + output: "test/short-pass.out", }); -itest!(jobs_flag_with_env_variable { +itest!(jobs_flag { args: "test test/short-pass.ts --jobs", - envs: vec![("DENO_JOBS".to_owned(), "2".to_owned())], exit_code: 0, - output: "test/short-pass.out", + output: "test/short-pass-jobs-flag-with-jobs-warning.out", }); -itest!(jobs_flag_with_numeric_value_and_env_var { +itest!(jobs_flag_with_numeric_value { args: "test test/short-pass.ts --jobs=2", - envs: vec![("DENO_JOBS".to_owned(), "3".to_owned())], exit_code: 0, - output: "test/short-pass-jobs-flag-with-numeric-value.out", + output: "test/short-pass-jobs-flag-with-jobs-warning.out", }); itest!(load_unload { diff --git a/cli/tests/testdata/test/short-pass-jobs-flag-with-numeric-value.out b/cli/tests/testdata/test/short-pass-jobs-flag-with-jobs-warning.out similarity index 54% rename from cli/tests/testdata/test/short-pass-jobs-flag-with-numeric-value.out rename to cli/tests/testdata/test/short-pass-jobs-flag-with-jobs-warning.out index 4ee49166ad583c..b70ec5ee1b366e 100644 --- a/cli/tests/testdata/test/short-pass-jobs-flag-with-numeric-value.out +++ b/cli/tests/testdata/test/short-pass-jobs-flag-with-jobs-warning.out @@ -1,4 +1,4 @@ -Warning: --jobs flag with numeric value is deprecated. Use 'DENO_JOBS' environment variable with --jobs flag instead. +Warning: --jobs flag is deprecated. Use the --parallel flag with possibly the 'DENO_JOBS' environment variable. Check [WILDCARD]/test/short-pass.ts running 1 test from ./test/short-pass.ts test ... ok ([WILDCARD]) From f0aa6faf1681ec0312f7652a9d6698b7cab4b28b Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 20 Jul 2022 11:47:18 -0400 Subject: [PATCH 3/6] Update file name. --- cli/tests/integration/test_tests.rs | 4 ++-- ...with-jobs-warning.out => short-pass-jobs-flag-warning.out} | 0 2 files changed, 2 insertions(+), 2 deletions(-) rename cli/tests/testdata/test/{short-pass-jobs-flag-with-jobs-warning.out => short-pass-jobs-flag-warning.out} (100%) diff --git a/cli/tests/integration/test_tests.rs b/cli/tests/integration/test_tests.rs index d2efdd023c1a06..3dd62690987ca8 100644 --- a/cli/tests/integration/test_tests.rs +++ b/cli/tests/integration/test_tests.rs @@ -96,13 +96,13 @@ itest!(parallel_flag_with_env_variable { itest!(jobs_flag { args: "test test/short-pass.ts --jobs", exit_code: 0, - output: "test/short-pass-jobs-flag-with-jobs-warning.out", + output: "test/short-pass-jobs-flag-warning.out", }); itest!(jobs_flag_with_numeric_value { args: "test test/short-pass.ts --jobs=2", exit_code: 0, - output: "test/short-pass-jobs-flag-with-jobs-warning.out", + output: "test/short-pass-jobs-flag-warning.out", }); itest!(load_unload { diff --git a/cli/tests/testdata/test/short-pass-jobs-flag-with-jobs-warning.out b/cli/tests/testdata/test/short-pass-jobs-flag-warning.out similarity index 100% rename from cli/tests/testdata/test/short-pass-jobs-flag-with-jobs-warning.out rename to cli/tests/testdata/test/short-pass-jobs-flag-warning.out From a23cfa5ab4afca35ffe55e7be24066c805e27f1e Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 20 Jul 2022 16:56:16 -0400 Subject: [PATCH 4/6] Fix description. --- cli/args/flags.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 0e9d4491efb75d..410d64a63f497f 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -1550,7 +1550,7 @@ fn test_subcommand<'a>() -> Command<'a> { .arg( Arg::new("parallel") .long("parallel") - .help("Runs the tests in parallel. Parallelism defaults to the number of available CPUs or the value in the DENO_JOBS environment variable.") + .help("Runs test modules in parallel. Parallelism defaults to the number of available CPUs or the value in the DENO_JOBS environment variable.") .conflicts_with("jobs") .takes_value(false) ) From ab71f744459bc4c0d23a995240568b2625d2759a Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 20 Jul 2022 16:57:51 -0400 Subject: [PATCH 5/6] Update description. --- cli/args/flags.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 410d64a63f497f..8af2c502afbd80 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -1558,7 +1558,8 @@ fn test_subcommand<'a>() -> Command<'a> { Arg::new("jobs") .short('j') .long("jobs") - .help("Number of parallel workers, defaults to number of available CPUs when no value is provided. Defaults to 1 when the option is not present.") + .help("deprecated: Number of parallel workers, defaults to number of available CPUs when no value is provided. Defaults to 1 when the option is not present.") + .hide(true) .min_values(0) .max_values(1) .takes_value(true) From 6990900349740534d12fe0ee12626f5c019c68b4 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 20 Jul 2022 17:13:11 -0400 Subject: [PATCH 6/6] Whoops. Runs -> run. --- cli/args/flags.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 8af2c502afbd80..f4a379cf72727c 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -1550,7 +1550,7 @@ fn test_subcommand<'a>() -> Command<'a> { .arg( Arg::new("parallel") .long("parallel") - .help("Runs test modules in parallel. Parallelism defaults to the number of available CPUs or the value in the DENO_JOBS environment variable.") + .help("Run test modules in parallel. Parallelism defaults to the number of available CPUs or the value in the DENO_JOBS environment variable.") .conflicts_with("jobs") .takes_value(false) )