From ebb79b28a5e1689a1d198082b35d37a4541d2a90 Mon Sep 17 00:00:00 2001 From: Casper Beyer Date: Fri, 13 Aug 2021 18:06:49 +0800 Subject: [PATCH] refactor(cli): generalize module specifier collection (#11679) --- cli/fs_util.rs | 122 +++++++++++++++++++++++++++++++++++++++ cli/main.rs | 20 ++----- cli/tools/test_runner.rs | 118 ------------------------------------- 3 files changed, 128 insertions(+), 132 deletions(-) diff --git a/cli/fs_util.rs b/cli/fs_util.rs index 6103a08334..6fa841376f 100644 --- a/cli/fs_util.rs +++ b/cli/fs_util.rs @@ -3,6 +3,7 @@ use deno_core::error::AnyError; use deno_core::error::Context; pub use deno_core::normalize_path; +use deno_core::ModuleSpecifier; use deno_runtime::deno_crypto::rand; use std::env::current_dir; use std::fs::OpenOptions; @@ -171,6 +172,49 @@ where Ok(target_files) } +/// Collects module specifiers that satisfy the given predicate as a file path, by recursively walking `include`. +/// Specifiers that start with http and https are left intact. +pub fn collect_specifiers

( + include: Vec, + predicate: P, +) -> Result, AnyError> +where + P: Fn(&Path) -> bool, +{ + let (include_urls, include_paths): (Vec, Vec) = + include.into_iter().partition(|url| { + let url = url.to_lowercase(); + url.starts_with("http://") || url.starts_with("https://") + }); + + let mut prepared = vec![]; + + let root_path = std::env::current_dir()?; + for path in include_paths { + let p = normalize_path(&root_path.join(path)); + if p.is_dir() { + let test_files = collect_files(&[p], &[], &predicate).unwrap(); + let mut test_files_as_urls = test_files + .iter() + .map(|f| ModuleSpecifier::from_file_path(f).unwrap()) + .collect::>(); + + test_files_as_urls.sort(); + prepared.extend(test_files_as_urls); + } else { + let url = ModuleSpecifier::from_file_path(p).unwrap(); + prepared.push(url); + } + } + + for remote_url in include_urls { + let url = ModuleSpecifier::parse(&remote_url)?; + prepared.push(url); + } + + Ok(prepared) +} + // Asynchronously removes a directory and all its descendants, but does not error // when the directory does not exist. pub async fn remove_dir_all_if_exists(path: &Path) -> std::io::Result<()> { @@ -328,4 +372,82 @@ mod tests { } assert_eq!(result.len(), expected.len()); } + + #[test] + fn test_collect_specifiers() { + fn create_files(dir_path: &Path, files: &[&str]) { + std::fs::create_dir(dir_path).expect("Failed to create directory"); + for f in files { + let path = dir_path.join(f); + std::fs::write(path, "").expect("Failed to create file"); + } + } + + // dir.ts + // ├── a.ts + // ├── b.js + // ├── child + // │ ├── e.mjs + // │ ├── f.mjsx + // │ ├── .foo.TS + // │ └── README.md + // ├── c.tsx + // ├── d.jsx + // └── ignore + // ├── g.d.ts + // └── .gitignore + + let t = TempDir::new().expect("tempdir fail"); + + let root_dir_path = t.path().join("dir.ts"); + let root_dir_files = ["a.ts", "b.js", "c.tsx", "d.jsx"]; + create_files(&root_dir_path, &root_dir_files); + + let child_dir_path = root_dir_path.join("child"); + let child_dir_files = ["e.mjs", "f.mjsx", ".foo.TS", "README.md"]; + create_files(&child_dir_path, &child_dir_files); + + let ignore_dir_path = root_dir_path.join("ignore"); + let ignore_dir_files = ["g.d.ts", ".gitignore"]; + create_files(&ignore_dir_path, &ignore_dir_files); + + let result = collect_specifiers( + vec![ + "http://localhost:8080".to_string(), + root_dir_path.to_str().unwrap().to_string(), + "https://localhost:8080".to_string(), + ], + |path| { + // exclude dotfiles + path + .file_name() + .and_then(|f| f.to_str()) + .map_or(false, |f| !f.starts_with('.')) + }, + ) + .unwrap(); + + let root_dir_url = ModuleSpecifier::from_file_path( + canonicalize_path(&root_dir_path).unwrap(), + ) + .unwrap() + .to_string(); + let expected: Vec = [ + &format!("{}/a.ts", root_dir_url), + &format!("{}/b.js", root_dir_url), + &format!("{}/c.tsx", root_dir_url), + &format!("{}/child/README.md", root_dir_url), + &format!("{}/child/e.mjs", root_dir_url), + &format!("{}/child/f.mjsx", root_dir_url), + &format!("{}/d.jsx", root_dir_url), + &format!("{}/ignore/g.d.ts", root_dir_url), + "http://localhost:8080", + "https://localhost:8080", + ] + .iter() + .map(|f| ModuleSpecifier::parse(f).unwrap()) + .collect::>(); + + assert_eq!(result, expected); + } } diff --git a/cli/main.rs b/cli/main.rs index da40e45280..9191ab86b5 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -1019,7 +1019,6 @@ async fn test_command( let program_state = ProgramState::build(flags.clone()).await?; let include = include.unwrap_or_else(|| vec![".".to_string()]); - let cwd = std::env::current_dir().expect("No current directory"); let permissions = Permissions::from_options(&flags.clone().into()); let lib = if flags.unstable { @@ -1040,15 +1039,13 @@ async fn test_command( // TODO(caspervonb) clean this up. let resolver = |changed: Option>| { let test_modules_result = if doc { - test_runner::collect_test_module_specifiers( + fs_util::collect_specifiers( include.clone(), - &cwd, fs_util::is_supported_ext_test, ) } else { - test_runner::collect_test_module_specifiers( + fs_util::collect_specifiers( include.clone(), - &cwd, tools::test_runner::is_supported, ) }; @@ -1173,7 +1170,6 @@ async fn test_command( }; let operation = |modules_to_reload: Vec| { - let cwd = cwd.clone(); let filter = filter.clone(); let include = include.clone(); let lib = lib.clone(); @@ -1182,9 +1178,8 @@ async fn test_command( async move { let doc_modules = if doc { - test_runner::collect_test_module_specifiers( + fs_util::collect_specifiers( include.clone(), - &cwd, fs_util::is_supported_ext_test, )? } else { @@ -1197,9 +1192,8 @@ async fn test_command( .cloned() .collect(); - let test_modules = test_runner::collect_test_module_specifiers( + let test_modules = fs_util::collect_specifiers( include.clone(), - &cwd, tools::test_runner::is_supported, )?; @@ -1232,18 +1226,16 @@ async fn test_command( file_watcher::watch_func(resolver, operation, "Test").await?; } else { let doc_modules = if doc { - test_runner::collect_test_module_specifiers( + fs_util::collect_specifiers( include.clone(), - &cwd, fs_util::is_supported_ext_test, )? } else { Vec::new() }; - let test_modules = test_runner::collect_test_module_specifiers( + let test_modules = fs_util::collect_specifiers( include.clone(), - &cwd, tools::test_runner::is_supported, )?; diff --git a/cli/tools/test_runner.rs b/cli/tools/test_runner.rs index ca8fa77924..da0d760b53 100644 --- a/cli/tools/test_runner.rs +++ b/cli/tools/test_runner.rs @@ -5,8 +5,6 @@ use crate::ast::Location; use crate::colors; use crate::create_main_worker; use crate::file_fetcher::File; -use crate::fs_util::collect_files; -use crate::fs_util::normalize_path; use crate::media_type::MediaType; use crate::module_graph; use crate::program_state::ProgramState; @@ -20,7 +18,6 @@ use deno_core::futures::FutureExt; use deno_core::futures::StreamExt; use deno_core::located_script_name; use deno_core::serde_json::json; -use deno_core::url::Url; use deno_core::ModuleSpecifier; use deno_runtime::permissions::Permissions; use rand::rngs::SmallRng; @@ -222,48 +219,6 @@ pub(crate) fn is_supported(p: &Path) -> bool { } } -pub fn is_remote_url(module_url: &str) -> bool { - let lower = module_url.to_lowercase(); - lower.starts_with("http://") || lower.starts_with("https://") -} - -pub fn collect_test_module_specifiers

( - include: Vec, - root_path: &Path, - predicate: P, -) -> Result, AnyError> -where - P: Fn(&Path) -> bool, -{ - let (include_paths, include_urls): (Vec, Vec) = - include.into_iter().partition(|n| !is_remote_url(n)); - let mut prepared = vec![]; - - for path in include_paths { - let p = normalize_path(&root_path.join(path)); - if p.is_dir() { - let test_files = collect_files(&[p], &[], &predicate).unwrap(); - let mut test_files_as_urls = test_files - .iter() - .map(|f| Url::from_file_path(f).unwrap()) - .collect::>(); - - test_files_as_urls.sort(); - prepared.extend(test_files_as_urls); - } else { - let url = Url::from_file_path(p).unwrap(); - prepared.push(url); - } - } - - for remote_url in include_urls { - let url = Url::parse(&remote_url)?; - prepared.push(url); - } - - Ok(prepared) -} - pub async fn test_specifier( program_state: Arc, main_module: ModuleSpecifier, @@ -740,37 +695,6 @@ pub async fn run_tests( mod tests { use super::*; - #[test] - fn test_collect_test_module_specifiers() { - let sub_dir_path = test_util::testdata_path().join("subdir"); - let mut matched_urls = collect_test_module_specifiers( - vec![ - "https://example.com/colors_test.ts".to_string(), - "./mod1.ts".to_string(), - "./mod3.js".to_string(), - "subdir2/mod2.ts".to_string(), - "http://example.com/printf_test.ts".to_string(), - ], - &sub_dir_path, - is_supported, - ) - .unwrap(); - let test_data_url = Url::from_file_path(sub_dir_path).unwrap().to_string(); - - let expected: Vec = vec![ - format!("{}/mod1.ts", test_data_url), - format!("{}/mod3.js", test_data_url), - format!("{}/subdir2/mod2.ts", test_data_url), - "http://example.com/printf_test.ts".to_string(), - "https://example.com/colors_test.ts".to_string(), - ] - .into_iter() - .map(|f| Url::parse(&f).unwrap()) - .collect(); - matched_urls.sort(); - assert_eq!(matched_urls, expected); - } - #[test] fn test_is_supported() { assert!(is_supported(Path::new("tests/subdir/foo_test.ts"))); @@ -790,46 +714,4 @@ mod tests { assert!(!is_supported(Path::new("notatest.js"))); assert!(!is_supported(Path::new("NotAtest.ts"))); } - - #[test] - fn supports_dirs() { - // TODO(caspervonb) generate some fixtures in a temporary directory instead, there's no need - // for this to rely on external fixtures. - let root = test_util::root_path() - .join("test_util") - .join("std") - .join("http"); - println!("root {:?}", root); - let matched_urls = collect_test_module_specifiers( - vec![".".to_string()], - &root, - is_supported, - ) - .unwrap(); - - let root_url = Url::from_file_path(root).unwrap().to_string(); - println!("root_url {}", root_url); - let expected: Vec = vec![ - format!("{}/_io_test.ts", root_url), - format!("{}/cookie_test.ts", root_url), - format!("{}/file_server_test.ts", root_url), - format!("{}/racing_server_test.ts", root_url), - format!("{}/server_test.ts", root_url), - format!("{}/test.ts", root_url), - ] - .into_iter() - .map(|f| Url::parse(&f).unwrap()) - .collect(); - assert_eq!(matched_urls, expected); - } - - #[test] - fn test_is_remote_url() { - assert!(is_remote_url("https://deno.land/std/http/file_server.ts")); - assert!(is_remote_url("http://deno.land/std/http/file_server.ts")); - assert!(is_remote_url("HTTP://deno.land/std/http/file_server.ts")); - assert!(is_remote_url("HTTp://deno.land/std/http/file_server.ts")); - assert!(!is_remote_url("file:///dev/deno_std/http/file_server.ts")); - assert!(!is_remote_url("./dev/deno_std/http/file_server.ts")); - } }