chore(task): various small refactorings (#23793)

Cargo.lock

@@ -2617,9 +2617,9 @@ checksum = "c007b1ae3abe1cb6f85a16305acd418b7ca6343b953633fee2b76d8f108b830f"
 
 [[package]]
 name = "file_test_runner"
-version = "0.6.0"
+version = "0.7.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "1eab6c1529960afefbd3c4e3021f23ba8030360aa6465e21cacb8e5c634f4854"
+checksum = "b8797fcdc5c6b8c06839900c30f5c59b3541ef2bec218579470ce7b1afc17ee9"
 dependencies = [
  "anyhow",
  "crossbeam-channel",

cli/tools/task.rs

@@ -1,6 +1,5 @@
 // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
 
-use crate::args::CliOptions;
 use crate::args::Flags;
 use crate::args::TaskFlags;
 use crate::colors;
@@ -55,6 +54,7 @@ pub async fn execute_script(
   };
   let npm_resolver = factory.npm_resolver().await?;
   let node_resolver = factory.node_resolver().await?;
+  let env_vars = real_env_vars();
 
   if let Some(
     deno_config::Task::Definition(script)
@@ -74,15 +74,17 @@ pub async fn execute_script(
       None => config_file_path.parent().unwrap().to_owned(),
     };
 
-    let npm_commands =
+    let custom_commands =
-      resolve_npm_commands(npm_resolver.as_ref(), node_resolver)?;
+      resolve_custom_commands(npm_resolver.as_ref(), node_resolver)?;
     run_task(
       task_name,
       script,
       &cwd,
-      cli_options,
+      cli_options.initial_cwd(),
-      npm_commands,
+      env_vars,
-      npm_resolver.as_ref(),
+      cli_options.argv(),
+      custom_commands,
+      npm_resolver.root_node_modules_path().map(|p| p.as_path()),
     )
     .await
   } else if package_json_scripts.contains_key(task_name) {
@@ -129,17 +131,19 @@ pub async fn execute_script(
       task_name.clone(),
       format!("post{}", task_name),
     ];
-    let npm_commands =
+    let custom_commands =
-      resolve_npm_commands(npm_resolver.as_ref(), node_resolver)?;
+      resolve_custom_commands(npm_resolver.as_ref(), node_resolver)?;
     for task_name in task_names {
       if let Some(script) = package_json_scripts.get(&task_name) {
         let exit_code = run_task(
           &task_name,
           script,
           &cwd,
-          cli_options,
+          cli_options.initial_cwd(),
-          npm_commands.clone(),
+          env_vars.clone(),
-          npm_resolver.as_ref(),
+          cli_options.argv(),
+          custom_commands.clone(),
+          npm_resolver.root_node_modules_path().map(|p| p.as_path()),
         )
         .await?;
         if exit_code > 0 {
@@ -162,30 +166,30 @@ pub async fn execute_script(
   }
 }
 
+#[allow(clippy::too_many_arguments)]
 async fn run_task(
   task_name: &str,
   script: &str,
   cwd: &Path,
-  cli_options: &CliOptions,
+  init_cwd: &Path,
-  npm_commands: HashMap<String, Rc<dyn ShellCommand>>,
+  env_vars: HashMap<String, String>,
-  npm_resolver: &dyn CliNpmResolver,
+  argv: &[String],
+  custom_commands: HashMap<String, Rc<dyn ShellCommand>>,
+  root_node_modules_dir: Option<&Path>,
 ) -> Result<i32, AnyError> {
-  let script = get_script_with_args(script, cli_options);
+  let script = get_script_with_args(script, argv);
   output_task(task_name, &script);
   let seq_list = deno_task_shell::parser::parse(&script)
     .with_context(|| format!("Error parsing script '{}'.", task_name))?;
-  let env_vars = match npm_resolver.root_node_modules_path() {
+  let env_vars = prepare_env_vars(env_vars, init_cwd, root_node_modules_dir);
-    Some(dir_path) => collect_env_vars_with_node_modules_dir(dir_path),
-    None => collect_env_vars(),
-  };
   let local = LocalSet::new();
-  let future = deno_task_shell::execute(seq_list, env_vars, cwd, npm_commands);
+  let future =
+    deno_task_shell::execute(seq_list, env_vars, cwd, custom_commands);
   Ok(local.run_until(future).await)
 }
 
-fn get_script_with_args(script: &str, options: &CliOptions) -> String {
+fn get_script_with_args(script: &str, argv: &[String]) -> String {
-  let additional_args = options
+  let additional_args = argv
-    .argv()
     .iter()
     // surround all the additional arguments in double quotes
     // and sanitize any command substitution
@@ -200,22 +204,30 @@ fn output_task(task_name: &str, script: &str) {
   log::info!(
     "{} {} {}",
     colors::green("Task"),
-    colors::cyan(&task_name),
+    colors::cyan(task_name),
     script,
   );
 }
 
-fn collect_env_vars_with_node_modules_dir(
+fn prepare_env_vars(
-  node_modules_dir_path: &Path,
+  mut env_vars: HashMap<String, String>,
+  initial_cwd: &Path,
+  node_modules_dir: Option<&Path>,
 ) -> HashMap<String, String> {
-  let mut env_vars = collect_env_vars();
+  const INIT_CWD_NAME: &str = "INIT_CWD";
+  if !env_vars.contains_key(INIT_CWD_NAME) {
+    // if not set, set an INIT_CWD env var that has the cwd
+    env_vars.insert(
+      INIT_CWD_NAME.to_string(),
+      initial_cwd.to_string_lossy().to_string(),
+    );
+  }
+  if let Some(node_modules_dir) = node_modules_dir {
     prepend_to_path(
       &mut env_vars,
-    node_modules_dir_path
+      node_modules_dir.join(".bin").to_string_lossy().to_string(),
-      .join(".bin")
-      .to_string_lossy()
-      .to_string(),
     );
+  }
   env_vars
 }
 
@@ -235,9 +247,8 @@ fn prepend_to_path(env_vars: &mut HashMap<String, String>, value: String) {
   }
 }
 
-fn collect_env_vars() -> HashMap<String, String> {
+fn real_env_vars() -> HashMap<String, String> {
-  // get the starting env vars (the PWD env var will be set by deno_task_shell)
+  std::env::vars()
-  let mut env_vars = std::env::vars()
     .map(|(k, v)| {
       if cfg!(windows) {
         (k.to_uppercase(), v)
@@ -245,16 +256,7 @@ fn collect_env_vars() -> HashMap<String, String> {
         (k, v)
       }
     })
-    .collect::<HashMap<String, String>>();
+    .collect::<HashMap<String, String>>()
-  const INIT_CWD_NAME: &str = "INIT_CWD";
-  if !env_vars.contains_key(INIT_CWD_NAME) {
-    if let Ok(cwd) = std::env::current_dir() {
-      // if not set, set an INIT_CWD env var that has the cwd
-      env_vars
-        .insert(INIT_CWD_NAME.to_string(), cwd.to_string_lossy().to_string());
-    }
-  }
-  env_vars
 }
 
 fn print_available_tasks(
@@ -406,7 +408,7 @@ impl ShellCommand for NodeModulesFileRunCommand {
   }
 }
 
-fn resolve_npm_commands(
+fn resolve_custom_commands(
   npm_resolver: &dyn CliNpmResolver,
   node_resolver: &NodeResolver,
 ) -> Result<HashMap<String, Rc<dyn ShellCommand>>, AnyError> {

tests/Cargo.toml

@@ -43,7 +43,7 @@ deno_lockfile.workspace = true
 deno_terminal.workspace = true
 deno_tls.workspace = true
 fastwebsockets = { workspace = true, features = ["upgrade", "unstable-split"] }
-file_test_runner = "0.6.0"
+file_test_runner = "0.7.0"
 flaky_test = "=0.1.0"
 http.workspace = true
 http-body-util.workspace = true

tests/specs/mod.rs

@@ -6,7 +6,6 @@ use std::collections::HashMap;
 use std::collections::HashSet;
 use std::panic::AssertUnwindSafe;
 use std::rc::Rc;
-use std::sync::Arc;
 
 use deno_core::anyhow::Context;
 use deno_core::serde_json;
@@ -174,7 +173,7 @@ pub fn main() {
   file_test_runner::run_tests(
     &root_category,
     file_test_runner::RunOptions { parallel: true },
-    Arc::new(run_test),
+    run_test,
   );
 }