Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BREAKING: remove --jobs flag #25336

Merged
merged 1 commit into from
Sep 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 8 additions & 86 deletions cli/args/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2741,7 +2741,7 @@ fn serve_subcommand() -> Command {
.value_parser(serve_host_validator),
)
.arg(
parallel_arg("multiple server workers", false)
parallel_arg("multiple server workers")
)
.arg(check_arg(false))
.arg(watch_arg(true))
Expand Down Expand Up @@ -2915,17 +2915,7 @@ Directory arguments are expanded to all contained files matching the glob
.help_heading(TEST_HEADING),
)
.arg(
parallel_arg("test modules", true)
)
.arg(
Arg::new("jobs")
.short('j')
.long("jobs")
.help("deprecated: The `--jobs` flag is deprecated and will be removed in Deno 2.0. Use the `--parallel` flag with possibly the `DENO_JOBS` environment variable instead.")
.hide(true)
.num_args(0..=1)
.value_parser(value_parser!(NonZeroUsize))
.help_heading(TEST_HEADING),
parallel_arg("test modules")
)
.arg(
Arg::new("files")
Expand Down Expand Up @@ -2967,16 +2957,11 @@ Directory arguments are expanded to all contained files matching the glob
)
}

fn parallel_arg(descr: &str, jobs_fallback: bool) -> Arg {
let arg = Arg::new("parallel")
fn parallel_arg(descr: &str) -> Arg {
Arg::new("parallel")
.long("parallel")
.help(format!("Run {descr} in parallel. Parallelism defaults to the number of available CPUs or the value of the DENO_JOBS environment variable"))
.action(ArgAction::SetTrue);
if jobs_fallback {
arg.conflicts_with("jobs")
} else {
arg
}
.action(ArgAction::SetTrue)
}

fn types_subcommand() -> Command {
Expand Down Expand Up @@ -4704,7 +4689,7 @@ fn serve_parse(
.remove_one::<String>("host")
.unwrap_or_else(|| "0.0.0.0".to_owned());

let worker_count = parallel_arg_parse(matches, false).map(|v| v.get());
let worker_count = parallel_arg_parse(matches).map(|v| v.get());

runtime_args_parse(flags, matches, true, true);
// If the user didn't pass --allow-net, add this port to the network
Expand Down Expand Up @@ -4780,37 +4765,13 @@ fn task_parse(flags: &mut Flags, matches: &mut ArgMatches) {
flags.subcommand = DenoSubcommand::Task(task_flags);
}

fn parallel_arg_parse(
matches: &mut ArgMatches,
fallback_to_jobs: bool,
) -> Option<NonZeroUsize> {
fn parallel_arg_parse(matches: &mut ArgMatches) -> Option<NonZeroUsize> {
if matches.get_flag("parallel") {
if let Ok(value) = env::var("DENO_JOBS") {
value.parse::<NonZeroUsize>().ok()
} else {
std::thread::available_parallelism().ok()
}
} else if fallback_to_jobs && matches.contains_id("jobs") {
// We can't change this to use the log crate because its not configured
// yet at this point since the flags haven't been parsed. This flag is
// deprecated though so it's not worth changing the code to use the log
// crate here and this is only done for testing anyway.
#[allow(clippy::print_stderr)]
{
eprintln!(
"⚠️ {}",
crate::colors::yellow(concat!(
"The `--jobs` flag is deprecated and will be removed in Deno 2.0.\n",
"Use the `--parallel` flag with possibly the `DENO_JOBS` environment variable instead.\n",
"Learn more at: https://docs.deno.com/runtime/manual/basics/env_variables"
)),
);
}
if let Some(value) = matches.remove_one::<NonZeroUsize>("jobs") {
Some(value)
} else {
std::thread::available_parallelism().ok()
}
} else {
None
}
Expand Down Expand Up @@ -4882,7 +4843,7 @@ fn test_parse(flags: &mut Flags, matches: &mut ArgMatches) {
flags.argv.extend(script_arg);
}

let concurrent_jobs = parallel_arg_parse(matches, true);
let concurrent_jobs = parallel_arg_parse(matches);

let include = if let Some(files) = matches.remove_many::<String>("files") {
files.collect()
Expand Down Expand Up @@ -8913,45 +8874,6 @@ 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(TestFlags {
no_run: false,
reporter: Default::default(),
doc: false,
fail_fast: None,
filter: None,
allow_none: false,
shuffle: None,
files: FileFlags {
include: vec![],
ignore: vec![],
},
concurrent_jobs: Some(NonZeroUsize::new(4).unwrap()),
trace_leaks: false,
coverage_dir: None,
clean: false,
watch: Default::default(),
junit_path: None,
hide_stacktraces: false,
}),
type_check_mode: TypeCheckMode::Local,
permissions: PermissionFlags {
no_prompt: true,
..Default::default()
},
..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"]);
Expand Down
12 changes: 0 additions & 12 deletions tests/integration/test_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,18 +154,6 @@ itest!(parallel_flag_with_env_variable {
output: "test/short-pass.out",
});

itest!(jobs_flag {
args: "test test/short-pass.ts --jobs",
exit_code: 0,
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-warning.out",
});

itest!(load_unload {
args: "test test/load_unload.ts",
exit_code: 0,
Expand Down
8 changes: 0 additions & 8 deletions tests/testdata/test/short-pass-jobs-flag-warning.out

This file was deleted.

2 changes: 1 addition & 1 deletion tools/lint.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ async function ensureNoNewITests() {
"run_tests.rs": 352,
"shared_library_tests.rs": 0,
"task_tests.rs": 30,
"test_tests.rs": 77,
"test_tests.rs": 75,
"upgrade_tests.rs": 0,
"vendor_tests.rs": 1,
"watcher_tests.rs": 0,
Expand Down