From dab83524b8b82ebfca09a85cc289715c1a33e7eb Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Mon, 9 Dec 2024 11:56:15 +0000 Subject: [PATCH] use WorkspaceFileContainer in deno test --- cli/factory.rs | 117 +++++++++++++++++++---------- cli/tools/check.rs | 22 ++++-- cli/tools/test/mod.rs | 171 ++++++++++++++++++++++++++++++++++-------- 3 files changed, 229 insertions(+), 81 deletions(-) diff --git a/cli/factory.rs b/cli/factory.rs index 7c72ae1ed5..0e4007ce24 100644 --- a/cli/factory.rs +++ b/cli/factory.rs @@ -25,6 +25,7 @@ use crate::cache::ModuleInfoCache; use crate::cache::NodeAnalysisCache; use crate::cache::ParsedSourceCache; use crate::emit::Emitter; +use crate::file_fetcher::File; use crate::file_fetcher::FileFetcher; use crate::graph_container::MainModuleGraphContainer; use crate::graph_util::FileWatcherReporter; @@ -61,7 +62,6 @@ use crate::tools::lint::LintRuleProvider; use crate::tools::run::hmr::HmrRunner; use crate::tsc::Diagnostics; use crate::tsc::TypeCheckingCjsTracker; -use crate::util::extract; use crate::util::file_watcher::WatcherCommunicator; use crate::util::fs::canonicalize_path_maybe_not_exists; use crate::util::progress_bar::ProgressBar; @@ -1064,31 +1064,55 @@ impl CliFactory { } } -pub struct WorkspaceFileContainerEntry { +#[derive(Debug, Copy, Clone)] +pub struct SpecifierInfo { + /// Type check as an ES module. + pub check: bool, + /// Type check virtual modules from doc snippets. If this is set but `check` + /// is not, this may be a markdown file for example. + pub check_doc: bool, +} + +struct WorkspaceFileContainerEntry { + specifiers: Vec<(ModuleSpecifier, SpecifierInfo)>, + doc_snippet_specifiers: Vec, main_graph_container: Arc, - specifiers: Vec<(ModuleSpecifier, T)>, - doc_snippet_specifiers: Option>, - check_doc_only: bool, + worker_factory: Arc, } -pub struct WorkspaceFileContainer { - entries: Vec>, +impl WorkspaceFileContainerEntry { + fn checked_specifiers(&self) -> impl Iterator { + self + .specifiers + .iter() + .filter_map(|(s, i)| i.check.then_some(s)) + .chain(self.doc_snippet_specifiers.iter()) + } } -impl WorkspaceFileContainer { +pub struct WorkspaceFileContainer { + entries: Vec, +} + +impl WorkspaceFileContainer { #[allow(clippy::type_complexity)] - pub async fn from_workspace_dirs_with_files( + pub async fn from_workspace_dirs_with_files( mut workspace_dirs_with_files: Vec<(Arc, FilePatterns)>, + cli_options: &CliOptions, + extract_doc_files: fn(File) -> Result, AnyError>, collect_specifiers: fn( FilePatterns, Arc, Arc, + T, ) -> std::pin::Pin< - Box, AnyError>>>, + Box< + dyn Future< + Output = Result, AnyError>, + >, + >, >, - cli_options: &CliOptions, - check_doc: bool, - check_doc_only: bool, + args: T, ) -> Result { workspace_dirs_with_files.sort_by_cached_key(|(d, _)| d.dir_url().clone()); let all_scopes = Arc::new( @@ -1121,30 +1145,32 @@ impl WorkspaceFileContainer { )?); let factory = CliFactory::from_cli_options(cli_options.clone()); let file_fetcher = factory.file_fetcher()?; + let specifiers = collect_specifiers( + files, + cli_options, + file_fetcher.clone(), + args.clone(), + ) + .await?; + let mut doc_snippet_specifiers = vec![]; + let root_permissions = factory.root_permissions_container()?; + for (s, _) in specifiers.iter().filter(|(_, i)| i.check_doc) { + let file = file_fetcher.fetch(s, root_permissions).await?; + let snippet_files = extract_doc_files(file)?; + for snippet_file in snippet_files { + doc_snippet_specifiers.push(snippet_file.specifier.clone()); + file_fetcher.insert_memory_files(snippet_file); + } + } let main_graph_container = factory.main_module_graph_container().await?.clone(); - let specifiers = - collect_specifiers(files, cli_options, file_fetcher.clone()).await?; - let doc_snippet_specifiers = if check_doc || check_doc_only { - let root_permissions = factory.root_permissions_container()?; - let mut doc_snippet_specifiers = Vec::new(); - for (s, _) in specifiers.iter() { - let file = file_fetcher.fetch(s, root_permissions).await?; - let snippet_files = extract::extract_snippet_files(file)?; - for snippet_file in snippet_files { - doc_snippet_specifiers.push(snippet_file.specifier.clone()); - file_fetcher.insert_memory_files(snippet_file); - } - } - Some(doc_snippet_specifiers) - } else { - None - }; + let worker_factory = + Arc::new(factory.create_cli_main_worker_factory().await?); entries.push(WorkspaceFileContainerEntry { - main_graph_container, specifiers, doc_snippet_specifiers, - check_doc_only, + main_graph_container, + worker_factory, }); } Ok(Self { entries }) @@ -1154,14 +1180,8 @@ impl WorkspaceFileContainer { let mut diagnostics = vec![]; let mut all_errors = vec![]; for entry in &self.entries { - let mut specifiers_for_typecheck = vec![]; - if !entry.check_doc_only { - specifiers_for_typecheck - .extend(entry.specifiers.iter().map(|(s, _)| s.clone())); - } - if let Some(doc_snippet_specifiers) = &entry.doc_snippet_specifiers { - specifiers_for_typecheck.extend(doc_snippet_specifiers.clone()); - } + let specifiers_for_typecheck = + entry.checked_specifiers().cloned().collect::>(); if specifiers_for_typecheck.is_empty() { continue; } @@ -1194,7 +1214,22 @@ impl WorkspaceFileContainer { Ok(()) } - pub fn has_specifiers(&self) -> bool { + pub fn found_specifiers(&self) -> bool { self.entries.iter().any(|e| !e.specifiers.is_empty()) } + + pub fn checked_specifiers_with_worker_factories( + &self, + ) -> Vec<(Vec, Arc)> { + self + .entries + .iter() + .map(|e| { + ( + e.checked_specifiers().cloned().collect(), + e.worker_factory.clone(), + ) + }) + .collect() + } } diff --git a/cli/tools/check.rs b/cli/tools/check.rs index 56e031bc92..cbb7163497 100644 --- a/cli/tools/check.rs +++ b/cli/tools/check.rs @@ -32,6 +32,7 @@ use crate::cache::Caches; use crate::cache::FastInsecureHasher; use crate::cache::TypeCheckCache; use crate::factory::CliFactory; +use crate::factory::SpecifierInfo; use crate::factory::WorkspaceFileContainer; use crate::graph_util::BuildFastCheckGraphOptions; use crate::graph_util::ModuleGraphBuilder; @@ -39,6 +40,7 @@ use crate::npm::CliNpmResolver; use crate::tsc; use crate::tsc::Diagnostics; use crate::tsc::TypeCheckingCjsTracker; +use crate::util::extract::extract_snippet_files; use crate::util::fs::collect_specifiers; use crate::util::path::is_script_ext; use crate::util::path::to_percent_decoded_str; @@ -72,28 +74,32 @@ pub async fn check( let patterns = cli_options.start_dir.to_resolved_file_patterns(patterns)?; vec![(cli_options.start_dir.clone(), patterns)] }; - let container = WorkspaceFileContainer::from_workspace_dirs_with_files( + let file_container = WorkspaceFileContainer::from_workspace_dirs_with_files( workspace_dirs_with_files, - |patterns, cli_options, _| { + cli_options, + extract_snippet_files, + |patterns, cli_options, _, (doc, doc_only)| { async move { + let info = SpecifierInfo { + check: !doc_only, + check_doc: doc || doc_only, + }; collect_specifiers( patterns, cli_options.vendor_dir_path().map(ToOwned::to_owned), |e| is_script_ext(e.path), ) - .map(|s| s.into_iter().map(|s| (s, ())).collect()) + .map(|s| s.into_iter().map(|s| (s, info)).collect()) } .boxed_local() }, - cli_options, - check_flags.doc, - check_flags.doc_only, + (check_flags.doc, check_flags.doc_only), ) .await?; - if !container.has_specifiers() { + if !file_container.found_specifiers() { log::warn!("{} No matching files found.", colors::yellow("Warning")); } - container.check().await + file_container.check().await } /// Options for performing a check of a module graph. Note that the decision to diff --git a/cli/tools/test/mod.rs b/cli/tools/test/mod.rs index 6357ebcae2..2131e563aa 100644 --- a/cli/tools/test/mod.rs +++ b/cli/tools/test/mod.rs @@ -7,6 +7,8 @@ use crate::args::TestReporterConfig; use crate::colors; use crate::display; use crate::factory::CliFactory; +use crate::factory::SpecifierInfo; +use crate::factory::WorkspaceFileContainer; use crate::file_fetcher::File; use crate::file_fetcher::FileFetcher; use crate::graph_util::has_graph_root_local_dependent_changed; @@ -1255,6 +1257,77 @@ async fn test_specifiers( Ok(()) } +/// Test a collection of specifiers with test modes concurrently. +async fn test_specifiers2( + file_container: WorkspaceFileContainer, + permissions: &Permissions, + permission_desc_parser: &Arc, + options: TestSpecifiersOptions, +) -> Result<(), AnyError> { + let mut specifiers_with_factory = file_container + .checked_specifiers_with_worker_factories() + .into_iter() + .flat_map(|(s, f)| { + s.into_iter().map(|s| (s, f.clone())).collect::>() + }) + .collect::>(); + if let Some(seed) = options.specifier.shuffle { + let mut rng = SmallRng::seed_from_u64(seed); + specifiers_with_factory.sort_by_cached_key(|(s, _)| s.to_string()); + specifiers_with_factory.shuffle(&mut rng); + } + + let (test_event_sender_factory, receiver) = create_test_event_channel(); + let concurrent_jobs = options.concurrent_jobs; + + let mut cancel_sender = test_event_sender_factory.weak_sender(); + let sigint_handler_handle = spawn(async move { + signal::ctrl_c().await.unwrap(); + cancel_sender.send(TestEvent::Sigint).ok(); + }); + HAS_TEST_RUN_SIGINT_HANDLER.store(true, Ordering::Relaxed); + let reporter = get_test_reporter(&options); + let fail_fast_tracker = FailFastTracker::new(options.fail_fast); + + let join_handles = specifiers_with_factory.into_iter().map( + move |(specifier, worker_factory)| { + let permissions_container = PermissionsContainer::new( + permission_desc_parser.clone(), + permissions.clone(), + ); + let worker_sender = test_event_sender_factory.worker(); + let fail_fast_tracker = fail_fast_tracker.clone(); + let specifier_options = options.specifier.clone(); + spawn_blocking(move || { + create_and_run_current_thread(test_specifier( + worker_factory, + permissions_container, + specifier, + worker_sender, + fail_fast_tracker, + specifier_options, + )) + }) + }, + ); + + let join_stream = stream::iter(join_handles) + .buffer_unordered(concurrent_jobs.get()) + .collect::, tokio::task::JoinError>>>(); + + let handler = spawn(async move { report_tests(receiver, reporter).await.0 }); + + let (join_results, result) = future::join(join_stream, handler).await; + sigint_handler_handle.abort(); + HAS_TEST_RUN_SIGINT_HANDLER.store(false, Ordering::Relaxed); + for join_result in join_results { + join_result??; + } + result??; + + Ok(()) +} + /// Gives receiver back in case it was ended with `TestEvent::ForceEndReport`. pub async fn report_tests( mut receiver: TestEventReceiver, @@ -1539,6 +1612,54 @@ async fn fetch_specifiers_with_test_mode( Ok(specifiers_with_mode) } +pub async fn collect_specifiers_for_tests( + patterns: FilePatterns, + cli_options: Arc, + file_fetcher: Arc, + doc: bool, +) -> Result, AnyError> { + let vendor_folder = cli_options.vendor_dir_path(); + let module_specifiers = collect_specifiers( + patterns.clone(), + vendor_folder.map(ToOwned::to_owned), + is_supported_test_path_predicate, + )?; + let mut specifiers = if doc { + collect_specifiers(patterns, vendor_folder.map(ToOwned::to_owned), |e| { + is_supported_test_ext(e.path) + }) + .map(|specifiers| { + specifiers + .into_iter() + .map(|specifier| { + let info = SpecifierInfo { + check: module_specifiers.contains(&specifier), + check_doc: true, + }; + (specifier, info) + }) + .collect::>() + })? + } else { + let info = SpecifierInfo { + check: true, + check_doc: false, + }; + module_specifiers + .into_iter() + .map(|specifier| (specifier, info)) + .collect::>() + }; + for (specifier, info) in &mut specifiers { + let file = file_fetcher.fetch_bypass_permissions(specifier).await?; + let (media_type, _) = file.resolve_media_type_and_charset(); + if matches!(media_type, MediaType::Unknown | MediaType::Dts) { + info.check = false; + } + } + Ok(specifiers) +} + pub async fn run_tests( flags: Arc, test_flags: TestFlags, @@ -1547,7 +1668,6 @@ pub async fn run_tests( let cli_options = factory.cli_options()?; let workspace_test_options = cli_options.resolve_workspace_test_options(&test_flags); - let file_fetcher = factory.file_fetcher()?; // Various test files should not share the same permissions in terms of // `PermissionsContainer` - otherwise granting/revoking permissions in one // file would have impact on other files, which is undesirable. @@ -1558,51 +1678,38 @@ pub async fn run_tests( )?; let log_level = cli_options.log_level(); - let members_with_test_options = - cli_options.resolve_test_options_for_members(&test_flags)?; - let specifiers_with_mode = fetch_specifiers_with_test_mode( + let workspace_dirs_with_files = cli_options + .resolve_test_options_for_members(&test_flags)? + .into_iter() + .map(|(d, o)| (Arc::new(d), o.files)) + .collect(); + let file_container = WorkspaceFileContainer::from_workspace_dirs_with_files( + workspace_dirs_with_files, cli_options, - file_fetcher, - members_with_test_options.into_iter().map(|(_, v)| v.files), - &workspace_test_options.doc, + extract_doc_tests, + |patterns, cli_options, file_fetcher, doc| { + collect_specifiers_for_tests(patterns, cli_options, file_fetcher, doc) + .boxed_local() + }, + workspace_test_options.doc, ) .await?; - - if !workspace_test_options.permit_no_files && specifiers_with_mode.is_empty() + if !workspace_test_options.permit_no_files + && !file_container.found_specifiers() { return Err(generic_error("No test modules found")); } - let doc_tests = get_doc_tests(&specifiers_with_mode, file_fetcher).await?; - let specifiers_for_typecheck_and_test = - get_target_specifiers(specifiers_with_mode, &doc_tests); - for doc_test in doc_tests { - file_fetcher.insert_memory_files(doc_test); - } - - let main_graph_container = factory.main_module_graph_container().await?; - - // Typecheck - main_graph_container - .check_specifiers( - &specifiers_for_typecheck_and_test, - cli_options.ext_flag().as_ref(), - ) - .await?; + file_container.check().await?; if workspace_test_options.no_run { return Ok(()); } - let worker_factory = - Arc::new(factory.create_cli_main_worker_factory().await?); - - // Run tests - test_specifiers( - worker_factory, + test_specifiers2( + file_container, &permissions, permission_desc_parser, - specifiers_for_typecheck_and_test, TestSpecifiersOptions { cwd: Url::from_directory_path(cli_options.initial_cwd()).map_err( |_| {