diff --git a/Cargo.lock b/Cargo.lock index 52b8d64b..ead66613 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -501,6 +501,15 @@ dependencies = [ "itertools 0.10.5", ] +[[package]] +name = "crossbeam-channel" +version = "0.5.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "82b8f8f868b36967f9606790d1903570de9ceaf870a7bf9fbbd3016d636a2cb2" +dependencies = [ + "crossbeam-utils", +] + [[package]] name = "crossbeam-deque" version = "0.8.6" @@ -1152,9 +1161,9 @@ version = "0.0.0" dependencies = [ "camino", "colored 3.0.0", + "crossbeam-channel", "ctor", "ignore", - "indexmap", "insta", "karva_project", "karva_test", diff --git a/Cargo.toml b/Cargo.toml index b68cdfd8..4a9b3a00 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,7 +24,6 @@ ctrlc = { version = "3.5.1" } divan = { package = "codspeed-divan-compat", version = "4.1.0" } dunce = { version = "1.0.5" } ignore = { version = "0.4.25" } -indexmap = { version = "2.12.1" } insta = { version = "1.44.3" } insta-cmd = { version = "0.6.0" } itertools = { version = "0.14.0" } diff --git a/crates/karva_benchmark/benches/karva.rs b/crates/karva_benchmark/benches/karva.rs index 8eab5b29..ad808e8e 100644 --- a/crates/karva_benchmark/benches/karva.rs +++ b/crates/karva_benchmark/benches/karva.rs @@ -8,7 +8,7 @@ use karva_benchmark::{ criterion::{BenchmarkId, Criterion, Throughput, criterion_group, criterion_main}, }; use karva_core::{DummyReporter, TestRunner, testing::setup_module}; -use karva_project::{path::absolute, project::Project}; +use karva_project::{Project, absolute}; fn create_test_cases() -> Vec { vec![ diff --git a/crates/karva_benchmark/src/walltime.rs b/crates/karva_benchmark/src/walltime.rs index 4c38d9d2..03cbba09 100644 --- a/crates/karva_benchmark/src/walltime.rs +++ b/crates/karva_benchmark/src/walltime.rs @@ -2,10 +2,7 @@ use std::sync::Once; use divan::Bencher; use karva_core::{TestRunner, testing::setup_module}; -use karva_project::{ - path::absolute, - project::{Project, ProjectOptions}, -}; +use karva_project::{Project, ProjectOptions, absolute}; use karva_test::{InstalledProject, RealWorldProject}; pub struct ProjectBenchmark<'a> { diff --git a/crates/karva_cli/src/args.rs b/crates/karva_cli/src/args.rs index 8db14e81..3f80130c 100644 --- a/crates/karva_cli/src/args.rs +++ b/crates/karva_cli/src/args.rs @@ -6,7 +6,7 @@ use clap::{ styling::{AnsiColor, Effects}, }, }; -use karva_project::project::{ProjectOptions, TestPrefix}; +use karva_project::{ProjectOptions, TestPrefix}; use ruff_db::diagnostic::DiagnosticFormat; use crate::logging::Verbosity; diff --git a/crates/karva_cli/src/lib.rs b/crates/karva_cli/src/lib.rs index 40cd0a95..550ca0bd 100644 --- a/crates/karva_cli/src/lib.rs +++ b/crates/karva_cli/src/lib.rs @@ -11,10 +11,7 @@ use colored::Colorize; use karva_core::{ DummyReporter, Reporter, TestCaseReporter, TestRunner, utils::current_python_version, }; -use karva_project::{ - path::absolute, - project::{Project, ProjectMetadata}, -}; +use karva_project::{Project, ProjectMetadata, absolute}; use crate::{ args::{Command, TerminalColor, TestCommand}, diff --git a/crates/karva_cli/src/logging.rs b/crates/karva_cli/src/logging.rs index 881ddecc..3c628a64 100644 --- a/crates/karva_cli/src/logging.rs +++ b/crates/karva_cli/src/logging.rs @@ -1,7 +1,7 @@ use std::{fmt, fs::File, io::BufWriter}; use colored::Colorize; -use karva_project::verbosity::VerbosityLevel; +use karva_project::VerbosityLevel; use tracing::{Event, Subscriber}; use tracing_subscriber::{ EnvFilter, diff --git a/crates/karva_core/Cargo.toml b/crates/karva_core/Cargo.toml index 1aa8feef..e3875651 100644 --- a/crates/karva_core/Cargo.toml +++ b/crates/karva_core/Cargo.toml @@ -26,7 +26,7 @@ tempfile = { workspace = true } tracing = { workspace = true } ignore = { workspace = true } regex = { workspace = true } -indexmap = { workspace = true } +crossbeam-channel = "0.5.15" [dev-dependencies] regex = { workspace = true } diff --git a/crates/karva_core/src/collection/collector.rs b/crates/karva_core/src/collection/collector.rs new file mode 100644 index 00000000..722b7878 --- /dev/null +++ b/crates/karva_core/src/collection/collector.rs @@ -0,0 +1,224 @@ +use std::{sync::Arc, thread}; + +use camino::{Utf8Path, Utf8PathBuf}; +use crossbeam_channel::unbounded; +use ignore::{WalkBuilder, WalkState}; +use karva_project::TestPath; +use ruff_python_ast::Stmt; +use ruff_python_parser::{Mode, ParseOptions, parse_unchecked}; + +use super::models::{CollectedModule, CollectedPackage}; +use crate::{ + Context, collection::ModuleType, diagnostic::report_invalid_path, + extensions::fixtures::is_fixture_function, name::ModulePath, +}; + +pub struct ParallelCollector<'ctx, 'proj, 'rep> { + context: &'ctx Context<'proj, 'rep>, +} + +impl<'ctx, 'proj, 'rep> ParallelCollector<'ctx, 'proj, 'rep> { + pub const fn new(context: &'ctx Context<'proj, 'rep>) -> Self { + Self { context } + } + + /// Collect from a directory in parallel using `WalkParallel`. + pub(crate) fn collect_directory(&self, path: &Utf8PathBuf) -> CollectedPackage { + // Create channels for communication + let (tx, rx) = unbounded::(); + + let cloned_path = path.clone(); + + // Spawn receiver thread to collect results + let receiver_handle = thread::spawn(move || { + let mut package = CollectedPackage::new(cloned_path); + + for collected_module in rx { + match collected_module.module_type() { + ModuleType::Test => { + package.add_module(collected_module); + } + ModuleType::Configuration => { + package.add_configuration_module(collected_module); + } + } + } + + package + }); + + let walker = self.create_parallel_walker(&path.clone()); + + walker.run(|| { + let tx = tx.clone(); + + Box::new(move |entry| { + let Ok(entry) = entry else { + return WalkState::Continue; + }; + + if !entry.file_type().is_some_and(|ft| ft.is_file()) { + return WalkState::Continue; + } + + let Ok(file_path) = Utf8PathBuf::from_path_buf(entry.path().to_path_buf()) else { + return WalkState::Continue; + }; + + if let Some(module) = collect_file(&file_path, self.context, None) { + let _ = tx.send(module); + } + + WalkState::Continue + }) + }); + + // Drop the original sender to close the channel + drop(tx); + + receiver_handle.join().unwrap() + } + + /// Collect from all paths and build a complete package structure. + pub(crate) fn collect_all(&self) -> CollectedPackage { + let mut session_package = CollectedPackage::new(self.context.project().cwd().clone()); + + for path_result in self.context.project().test_paths() { + let path = match path_result { + Ok(path) => path, + Err(error) => { + report_invalid_path(self.context, &error); + continue; + } + }; + + let path_for_config = path.path().to_owned(); + + match path { + TestPath::Directory(dir_path) => { + let package = self.collect_directory(&dir_path); + session_package.add_package(package); + } + TestPath::Function { + path: file_path, + function_name, + } => { + if let Some(module) = + collect_file(&file_path, self.context, Some(&function_name)) + { + session_package.add_module(module); + } + } + TestPath::File(file_path) => { + if let Some(module) = collect_file(&file_path, self.context, None) { + session_package.add_module(module); + } + } + } + + self.collect_parent_configuration(&path_for_config, &mut session_package); + } + + session_package.shrink(); + session_package + } + + /// Collect parent configuration files (conftest.py). + fn collect_parent_configuration( + &self, + path: &Utf8Path, + session_package: &mut CollectedPackage, + ) { + let mut current_path = if path.is_dir() { + path + } else { + match path.parent() { + Some(parent) => parent, + None => return, + } + }; + + loop { + let conftest_path = current_path.join("conftest.py"); + if conftest_path.exists() { + let mut package = CollectedPackage::new(current_path.to_path_buf()); + + if let Some(module) = collect_file(&conftest_path, self.context, None) { + package.add_configuration_module(module); + session_package.add_package(package); + } + } + + if current_path == self.context.project().cwd() { + break; + } + + current_path = match current_path.parent() { + Some(parent) => parent, + None => break, + }; + } + } + + /// Creates a configured parallel directory walker for Python file discovery. + fn create_parallel_walker(&self, path: &Utf8PathBuf) -> ignore::WalkParallel { + let num_threads = karva_project::max_parallelism().get(); + + WalkBuilder::new(path) + .threads(num_threads) + .standard_filters(true) + .require_git(false) + .git_global(false) + .parents(true) + .git_ignore(!self.context.project().options().no_ignore()) + .types({ + let mut types = ignore::types::TypesBuilder::new(); + types.add("python", "*.py").unwrap(); + types.select("python"); + types.build().unwrap() + }) + .build_parallel() + } +} + +fn collect_file( + path: &Utf8PathBuf, + context: &Context, + only_function_name: Option<&str>, +) -> Option { + let module_path = ModulePath::new(path, context.project().cwd())?; + + let source_text = std::fs::read_to_string(path).ok()?; + + let module_type: ModuleType = path.into(); + + let parsed = parse_unchecked( + &source_text, + ParseOptions::from(Mode::Module) + .with_target_version(context.project().metadata().python_version()), + ) + .try_into_module()?; + + let mut collected_module = CollectedModule::new(module_path, module_type, source_text); + + for stmt in parsed.into_syntax().body { + if let Stmt::FunctionDef(function_def) = stmt { + if let Some(only_function_name) = only_function_name { + if function_def.name.as_str() != only_function_name { + continue; + } + } + if is_fixture_function(&function_def) { + collected_module.add_fixture_function_def(Arc::new(function_def)); + } else if function_def + .name + .to_string() + .starts_with(context.project().options().test_prefix()) + { + collected_module.add_test_function_def(Arc::new(function_def)); + } + } + } + + Some(collected_module) +} diff --git a/crates/karva_core/src/collection/mod.rs b/crates/karva_core/src/collection/mod.rs new file mode 100644 index 00000000..7a814f49 --- /dev/null +++ b/crates/karva_core/src/collection/mod.rs @@ -0,0 +1,5 @@ +mod collector; +mod models; + +pub use collector::ParallelCollector; +pub use models::{CollectedModule, CollectedPackage, ModuleType}; diff --git a/crates/karva_core/src/collection/models.rs b/crates/karva_core/src/collection/models.rs new file mode 100644 index 00000000..5b8c8e8f --- /dev/null +++ b/crates/karva_core/src/collection/models.rs @@ -0,0 +1,264 @@ +use std::{collections::HashMap, sync::Arc}; + +use camino::Utf8PathBuf; +use ruff_python_ast::StmtFunctionDef; + +use crate::name::ModulePath; + +/// A collected module containing raw AST function definitions. +/// This is populated during the parallel collection phase. +#[derive(Debug, Clone)] +pub struct CollectedModule { + /// The path of the module. + pub(crate) path: ModulePath, + /// The type of module. + pub(crate) module_type: ModuleType, + /// The source text of the file (cached to avoid re-reading) + pub(crate) source_text: String, + /// Test function definitions (functions starting with test prefix) + pub(crate) test_function_defs: Vec>, + /// Fixture function definitions (functions with fixture decorators) + pub(crate) fixture_function_defs: Vec>, +} + +impl CollectedModule { + pub(crate) const fn new( + path: ModulePath, + + module_type: ModuleType, + source_text: String, + ) -> Self { + Self { + path, + + module_type, + source_text, + test_function_defs: Vec::new(), + fixture_function_defs: Vec::new(), + } + } + + pub(crate) fn add_test_function_def(&mut self, function_def: Arc) { + self.test_function_defs.push(function_def); + } + + pub(crate) fn add_fixture_function_def(&mut self, function_def: Arc) { + self.fixture_function_defs.push(function_def); + } + + pub(crate) const fn file_path(&self) -> &Utf8PathBuf { + self.path.path() + } + + pub(crate) const fn module_type(&self) -> ModuleType { + self.module_type + } + + pub(crate) fn is_empty(&self) -> bool { + self.test_function_defs.is_empty() && self.fixture_function_defs.is_empty() + } +} + +/// A collected package containing collected modules and subpackages. +/// This is populated during the parallel collection phase. +#[derive(Debug, Clone)] +pub struct CollectedPackage { + pub(crate) path: Utf8PathBuf, + pub(crate) modules: HashMap, + pub(crate) packages: HashMap, + pub(crate) configuration_module_path: Option, +} + +impl CollectedPackage { + pub(crate) fn new(path: Utf8PathBuf) -> Self { + Self { + path, + modules: HashMap::new(), + packages: HashMap::new(), + configuration_module_path: None, + } + } + + pub(crate) const fn path(&self) -> &Utf8PathBuf { + &self.path + } + + /// Add a module to this package. + /// + /// If the module path does not start with our path, do nothing. + /// + /// If the module path equals our path, use update method. + /// + /// Otherwise, strip the current path from the start and add the module to the appropriate sub-package. + pub(crate) fn add_module(&mut self, module: CollectedModule) { + if !module.file_path().starts_with(self.path()) { + return; + } + + if module.is_empty() { + return; + } + + let Some(parent_path) = module.file_path().parent() else { + return; + }; + + if parent_path == self.path() { + if let Some(existing_module) = self.modules.get_mut(module.file_path()) { + existing_module.update(module); + } else { + if module.module_type() == ModuleType::Configuration { + self.configuration_module_path = Some(module.file_path().clone()); + } + self.modules.insert(module.file_path().clone(), module); + } + return; + } + + let Ok(relative_path) = module.file_path().strip_prefix(self.path()) else { + return; + }; + + let components: Vec<_> = relative_path.components().collect(); + + if components.is_empty() { + return; + } + + let first_component = components[0]; + let intermediate_path = self.path().join(first_component); + + if let Some(existing_package) = self.packages.get_mut(&intermediate_path) { + existing_package.add_module(module); + } else { + let mut new_package = Self::new(intermediate_path); + new_package.add_module(module); + self.packages + .insert(new_package.path().clone(), new_package); + } + } + + pub(crate) fn add_configuration_module(&mut self, module: CollectedModule) { + self.configuration_module_path = Some(module.file_path().clone()); + self.add_module(module); + } + + /// Add a package to this package. + /// + /// If the package path equals our path, use update method. + /// + /// Otherwise, strip the current path from the start and add the package to the appropriate sub-package. + pub(crate) fn add_package(&mut self, package: Self) { + if !package.path().starts_with(self.path()) { + return; + } + + if package.path() == self.path() { + self.update(package); + return; + } + + let Ok(relative_path) = package.path().strip_prefix(self.path()) else { + return; + }; + + let components: Vec<_> = relative_path.components().collect(); + + if components.is_empty() { + return; + } + + let first_component = components[0]; + let intermediate_path = self.path().join(first_component); + + if let Some(existing_package) = self.packages.get_mut(&intermediate_path) { + existing_package.add_package(package); + } else { + let mut new_package = Self::new(intermediate_path); + new_package.add_package(package); + self.packages + .insert(new_package.path().clone(), new_package); + } + } + + pub(crate) fn update(&mut self, package: Self) { + for (_, module) in package.modules { + self.add_module(module); + } + for (_, package) in package.packages { + self.add_package(package); + } + + if let Some(module_path) = package.configuration_module_path { + self.configuration_module_path = Some(module_path); + } + } + + pub(crate) fn is_empty(&self) -> bool { + self.modules.is_empty() && self.packages.is_empty() + } + + pub(crate) fn shrink(&mut self) { + self.modules.retain(|_, module| !module.is_empty()); + + if let Some(configuration_module) = self.configuration_module_path.as_ref() { + if !self.modules.contains_key(configuration_module) { + self.configuration_module_path = None; + } + } + + self.packages.retain(|_, package| !package.is_empty()); + + for package in self.packages.values_mut() { + package.shrink(); + } + } +} + +impl CollectedModule { + /// Update this module with another module. + /// Merges function definitions from the other module into this one. + pub(crate) fn update(&mut self, module: Self) { + if self.path == module.path { + for function_def in module.test_function_defs { + if !self + .test_function_defs + .iter() + .any(|existing| existing.name == function_def.name) + { + self.test_function_defs.push(function_def); + } + } + for function_def in module.fixture_function_defs { + if !self + .fixture_function_defs + .iter() + .any(|existing| existing.name == function_def.name) + { + self.fixture_function_defs.push(function_def); + } + } + } + } +} + +/// The type of module. +/// This is used to differentiation between files that contain only test functions and files that contain only configuration functions. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ModuleType { + Test, + Configuration, +} + +impl From<&Utf8PathBuf> for ModuleType { + fn from(path: &Utf8PathBuf) -> Self { + if path + .file_name() + .is_some_and(|file_name| file_name == "conftest.py") + { + Self::Configuration + } else { + Self::Test + } + } +} diff --git a/crates/karva_core/src/diagnostic/mod.rs b/crates/karva_core/src/diagnostic/mod.rs index 787347e6..b43c3a86 100644 --- a/crates/karva_core/src/diagnostic/mod.rs +++ b/crates/karva_core/src/diagnostic/mod.rs @@ -6,7 +6,7 @@ use std::collections::HashMap; -use karva_project::path::TestPathError; +use karva_project::TestPathError; use pyo3::{Py, PyAny, PyErr, Python}; use ruff_db::diagnostic::{ Annotation, Diagnostic, Severity, Span, SubDiagnostic, SubDiagnosticSeverity, @@ -123,6 +123,8 @@ pub fn report_invalid_path(context: &Context, error: &TestPathError) { pub fn report_failed_to_import_module(context: &Context, module_name: &str, error: &str) { let builder = context.report_discovery_diagnostic(&FAILED_TO_IMPORT_MODULE); + tracing::debug!("Failed to import python module `{module_name}`: {error}"); + builder.into_diagnostic(format!( "Failed to import python module `{module_name}`: {error}" )); diff --git a/crates/karva_core/src/discovery/discoverer.rs b/crates/karva_core/src/discovery/discoverer.rs index 13ab3ed2..a73cd4a0 100644 --- a/crates/karva_core/src/discovery/discoverer.rs +++ b/crates/karva_core/src/discovery/discoverer.rs @@ -1,15 +1,11 @@ -use camino::{Utf8Path, Utf8PathBuf}; -use ignore::WalkBuilder; -use karva_project::path::TestPath; use pyo3::prelude::*; #[cfg(test)] use crate::utils::attach_with_project; use crate::{ Context, - diagnostic::report_invalid_path, - discovery::{DiscoveredModule, DiscoveredPackage, ModuleType, visitor::DiscoveredFunctions}, - name::ModulePath, + collection::{CollectedModule, CollectedPackage, ModuleType, ParallelCollector}, + discovery::{DiscoveredModule, DiscoveredPackage, visitor::discover}, utils::add_to_sys_path, }; @@ -28,243 +24,86 @@ impl<'ctx, 'proj, 'rep> StandardDiscoverer<'ctx, 'proj, 'rep> { } pub(crate) fn discover_with_py(self, py: Python<'_>) -> DiscoveredPackage { - let mut session_package = DiscoveredPackage::new(self.context.project().cwd().clone()); - let cwd = self.context.project().cwd(); if add_to_sys_path(py, cwd, 0).is_err() { - return session_package; - } - - tracing::info!("Discovering tests..."); - - for path in self.context.project().test_paths() { - match path { - Ok(path) => { - match &path { - TestPath::File(path) => { - let Some(module) = - self.discover_test_file(py, path, DiscoveryMode::All) - else { - continue; - }; - - session_package.add_module(module); - } - TestPath::Directory(path) => { - let package = self.discover_directory(py, path, DiscoveryMode::All); - - session_package.add_package(package); - } - TestPath::Function { - path, - function_name, - } => { - let Some(mut module) = - self.discover_test_file(py, path, DiscoveryMode::All) - else { - continue; - }; - - module.filter_test_functions(function_name); - - if !module.test_functions().is_empty() { - session_package.add_module(module); - } - } - } - - self.add_parent_configuration_packages(py, path.path(), &mut session_package); - } - Err(error) => { - report_invalid_path(self.context, &error); - } - } - } - - session_package.shrink(); - - session_package - } - - // Parse and run discovery on a single file - fn discover_test_file( - &self, - py: Python, - path: &Utf8PathBuf, - discovery_mode: DiscoveryMode, - ) -> Option { - let module_path = ModulePath::new(path, self.context.project().cwd())?; - - let mut module = DiscoveredModule::new(module_path, path.into()); - - let DiscoveredFunctions { - functions, - fixtures, - } = super::visitor::discover(self.context, py, &module); - - if !discovery_mode.is_configuration_only() { - module.extend_test_functions(functions); + return DiscoveredPackage::new(cwd.clone()); } - module.extend_fixtures(fixtures); - - Some(module) - } - - // This should look from the parent of path to the cwd for configuration files - fn add_parent_configuration_packages( - &self, - py: Python, - path: &Utf8Path, - session_package: &mut DiscoveredPackage, - ) { - let mut current_path = if path.is_dir() { - path - } else { - match path.parent() { - Some(parent) => parent, - None => return, - } - }; + tracing::info!("Collecting test files in parallel..."); - loop { - let conftest_path = current_path.join("conftest.py"); - if conftest_path.exists() { - let mut package = DiscoveredPackage::new(current_path.to_path_buf()); + let collector = ParallelCollector::new(self.context); + let collected_package = collector.collect_all(); - let Some(module) = - self.discover_test_file(py, &conftest_path, DiscoveryMode::ConfigurationOnly) - else { - break; - }; + tracing::info!("Discovering test functions and fixtures..."); - package.add_configuration_module(module); + let mut session_package = self.convert_collected_to_discovered(py, collected_package); - session_package.add_package(package); - } - - if current_path == *self.context.project().cwd() { - break; - } + session_package.shrink(); - current_path = match current_path.parent() { - Some(parent) => parent, - None => break, - }; - } + session_package } - /// Discovers test files and packages within a directory. - /// - /// This method recursively walks through a directory structure to find Python - /// test files and subdirectories. It respects .gitignore files and filters - /// out common non-test directories like __pycache__. - fn discover_directory( + /// Convert a collected package to a discovered package by importing Python modules + /// and resolving test functions and fixtures. + fn convert_collected_to_discovered( &self, - py: Python, - path: &Utf8PathBuf, - discovery_mode: DiscoveryMode, + py: Python<'_>, + collected_package: CollectedPackage, ) -> DiscoveredPackage { - let walker = self.create_directory_walker(path); - - let mut package = DiscoveredPackage::new(path.clone()); - - for entry in walker { - let Ok(entry) = entry else { - continue; - }; - let current_path = Utf8PathBuf::from_path_buf(entry.path().to_path_buf()) - .expect("Path is not valid UTF-8"); - - // Skip the package directory itself - if package.path() == ¤t_path { - continue; - } - - match entry.file_type() { - Some(file_type) if file_type.is_dir() => { - if discovery_mode.is_configuration_only() { - continue; - } - - let subpackage = self.discover_directory(py, ¤t_path, discovery_mode); - - package.add_package(subpackage); - } - Some(file_type) if file_type.is_file() => match (¤t_path).into() { - ModuleType::Test => { - if discovery_mode.is_configuration_only() { - continue; - } - - let Some(module) = - self.discover_test_file(py, ¤t_path, DiscoveryMode::All) - else { - continue; - }; - - package.add_module(module); - } - ModuleType::Configuration => { - let Some(module) = self.discover_test_file( - py, - ¤t_path, - DiscoveryMode::ConfigurationOnly, - ) else { - continue; - }; - - package.add_configuration_module(module); - } - }, - _ => {} + let CollectedPackage { + path, + modules, + packages, + configuration_module_path: _, + } = collected_package; + + let mut discovered_package = DiscoveredPackage::new(path); + + // Convert all modules + for collected_module in modules.into_values() { + let CollectedModule { + path, + module_type, + source_text, + test_function_defs, + fixture_function_defs, + } = collected_module; + + let mut module = DiscoveredModule::new_with_source(path.clone(), source_text); + + discover( + self.context, + py, + &mut module, + test_function_defs, + fixture_function_defs, + ); + + if module_type == ModuleType::Configuration { + discovered_package.add_configuration_module(module); + } else { + discovered_package.add_direct_module(module); } } - package - } - - /// Creates a configured directory walker for Python file discovery. - fn create_directory_walker(&self, path: &Utf8PathBuf) -> ignore::Walk { - WalkBuilder::new(path) - .max_depth(Some(1)) - .standard_filters(true) - .require_git(false) - .git_global(false) - .parents(true) - .git_ignore(!self.context.project().options().no_ignore()) - .types({ - let mut types = ignore::types::TypesBuilder::new(); - types.add("python", "*.py").unwrap(); - types.select("python"); - types.build().unwrap() - }) - .filter_entry(|entry| { - let file_name = entry.file_name(); - file_name != "__pycache__" - }) - .build() - } -} - -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub(super) enum DiscoveryMode { - All, - ConfigurationOnly, -} + // Convert all subpackages recursively + for collected_subpackage in packages.into_values() { + let discovered_subpackage = + self.convert_collected_to_discovered(py, collected_subpackage); + discovered_package.add_direct_subpackage(discovered_subpackage); + } -impl DiscoveryMode { - pub const fn is_configuration_only(self) -> bool { - matches!(self, Self::ConfigurationOnly) + discovered_package } } #[cfg(test)] mod tests { + use camino::Utf8PathBuf; use insta::{allow_duplicates, assert_snapshot}; - use karva_project::{Project, project::ProjectOptions}; + use karva_project::{Project, ProjectOptions}; use karva_test::TestContext; use super::*; @@ -566,16 +405,19 @@ def test_function(): pass fn test_discover_function_inside_function() { let env = TestContext::with_files([( "/test_file.py", - "def test_function(): def test_function2(): pass", + " +def test_function(): + def test_function2(): pass", )]); - let mapped_dir = env.mapped_path("").unwrap(); - let path = mapped_dir.join("test_file.py"); - - let project = Project::new(env.cwd(), vec![path]); + let project = Project::new(env.cwd(), vec![env.cwd()]); let session = session(&project); - assert_snapshot!(session.display(), @""); + assert_snapshot!(session.display(), @r" + └── // + └── .test_file + └── test_cases [test_function] + "); } #[test] diff --git a/crates/karva_core/src/discovery/mod.rs b/crates/karva_core/src/discovery/mod.rs index 70823e18..81e46fe2 100644 --- a/crates/karva_core/src/discovery/mod.rs +++ b/crates/karva_core/src/discovery/mod.rs @@ -3,8 +3,4 @@ pub mod models; pub mod visitor; pub use discoverer::StandardDiscoverer; -pub use models::{ - function::TestFunction, - module::{DiscoveredModule, ModuleType}, - package::DiscoveredPackage, -}; +pub use models::{function::TestFunction, module::DiscoveredModule, package::DiscoveredPackage}; diff --git a/crates/karva_core/src/discovery/models/function.rs b/crates/karva_core/src/discovery/models/function.rs index b24c5655..a69cd59b 100644 --- a/crates/karva_core/src/discovery/models/function.rs +++ b/crates/karva_core/src/discovery/models/function.rs @@ -29,7 +29,7 @@ impl TestFunction { pub(crate) fn new( py: Python<'_>, module: &DiscoveredModule, - stmt_function_def: StmtFunctionDef, + stmt_function_def: Arc, py_function: Py, ) -> Self { let name = QualifiedFunctionName::new( @@ -41,15 +41,11 @@ impl TestFunction { Self { name, - stmt_function_def: Arc::new(stmt_function_def), + stmt_function_def, py_function, tags, } } - - pub(crate) fn function_name(&self) -> &str { - &self.stmt_function_def.name - } } impl RequiresFixtures for TestFunction { @@ -64,7 +60,7 @@ impl RequiresFixtures for TestFunction { #[cfg(test)] mod tests { - use karva_project::project::Project; + use karva_project::Project; use karva_test::TestContext; use crate::{ diff --git a/crates/karva_core/src/discovery/models/module.rs b/crates/karva_core/src/discovery/models/module.rs index a9b12602..894cc499 100644 --- a/crates/karva_core/src/discovery/models/module.rs +++ b/crates/karva_core/src/discovery/models/module.rs @@ -9,19 +9,15 @@ pub struct DiscoveredModule { path: ModulePath, test_functions: Vec, fixtures: Vec, - type_: ModuleType, source_text: String, } impl DiscoveredModule { - pub(crate) fn new(path: ModulePath, module_type: ModuleType) -> Self { - let source_text = std::fs::read_to_string(path.path()).expect("Failed to read source file"); - + pub(crate) const fn new_with_source(path: ModulePath, source_text: String) -> Self { Self { path, test_functions: Vec::new(), fixtures: Vec::new(), - type_: module_type, source_text, } } @@ -38,28 +34,20 @@ impl DiscoveredModule { self.path.module_name() } - pub(crate) const fn module_type(&self) -> ModuleType { - self.type_ - } - pub(crate) fn test_functions(&self) -> Vec<&TestFunction> { self.test_functions.iter().collect() } - pub(crate) fn extend_test_functions(&mut self, test_functions: Vec) { - self.test_functions.extend(test_functions); - } - - pub(crate) fn filter_test_functions(&mut self, name: &str) { - self.test_functions.retain(|tc| tc.function_name() == name); + pub(crate) fn add_test_function(&mut self, test_function: TestFunction) { + self.test_functions.push(test_function); } pub(crate) const fn fixtures(&self) -> &Vec { &self.fixtures } - pub(crate) fn extend_fixtures(&mut self, fixtures: Vec) { - self.fixtures.extend(fixtures); + pub(crate) fn add_fixture(&mut self, fixture: Fixture) { + self.fixtures.push(fixture); } #[cfg(test)] @@ -75,30 +63,6 @@ impl DiscoveredModule { SourceFileBuilder::new(self.path().as_str(), self.source_text()).finish() } - pub(crate) fn update(&mut self, module: Self) { - if self.path == module.path { - for test_case in module.test_functions { - if !self - .test_functions - .iter() - .any(|existing| existing.name == test_case.name) - { - self.test_functions.push(test_case); - } - } - - for fixture in module.fixtures { - if !self - .fixtures - .iter() - .any(|existing| existing.name() == fixture.name()) - { - self.fixtures.push(fixture); - } - } - } - } - pub(crate) fn is_empty(&self) -> bool { self.test_functions.is_empty() && self.fixtures.is_empty() } @@ -109,27 +73,6 @@ impl DiscoveredModule { } } -/// The type of module. -/// This is used to differentiation between files that contain only test functions and files that contain only configuration functions. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum ModuleType { - Test, - Configuration, -} - -impl From<&Utf8PathBuf> for ModuleType { - fn from(path: &Utf8PathBuf) -> Self { - if path - .file_name() - .is_some_and(|file_name| file_name == "conftest.py") - { - Self::Configuration - } else { - Self::Test - } - } -} - #[cfg(test)] pub struct DisplayDiscoveredModule<'proj> { module: &'proj DiscoveredModule, diff --git a/crates/karva_core/src/discovery/models/package.rs b/crates/karva_core/src/discovery/models/package.rs index bad9a76a..44cb66a0 100644 --- a/crates/karva_core/src/discovery/models/package.rs +++ b/crates/karva_core/src/discovery/models/package.rs @@ -4,10 +4,7 @@ use camino::Utf8PathBuf; #[cfg(test)] use crate::discovery::TestFunction; -use crate::{ - discovery::{DiscoveredModule, ModuleType}, - name::ModulePath, -}; +use crate::{discovery::DiscoveredModule, name::ModulePath}; /// A package represents a single python directory. #[derive(Debug)] @@ -68,98 +65,19 @@ impl DiscoveredPackage { } } - /// Add a module to this package. - /// - /// If the module path does not start with our path, do nothing. - /// - /// If the module path equals our path, use update method. - /// - /// Otherwise, strip the current path from the start and add the module to the appropriate sub-package. - pub(crate) fn add_module(&mut self, module: DiscoveredModule) { - if !module.path().starts_with(self.path()) { - return; - } - - if module.is_empty() { - return; - } - - if module.path().parent().is_some_and(|p| p == self.path()) { - if let Some(existing_module) = self.modules.get_mut(module.path()) { - existing_module.update(module); - } else { - if module.module_type() == ModuleType::Configuration { - self.configuration_module_path = Some(module.module_path().clone()); - } - self.modules.insert(module.path().clone(), module); - } - return; - } - - let Ok(relative_path) = module.path().strip_prefix(self.path()) else { - return; - }; - - let components: Vec<_> = relative_path.components().collect(); - - if components.is_empty() { - return; - } - - let first_component = components[0]; - let intermediate_path = self.path().join(first_component); - - if let Some(existing_package) = self.packages.get_mut(&intermediate_path) { - existing_package.add_module(module); - } else { - let mut new_package = Self::new(intermediate_path); - new_package.add_module(module); - self.packages - .insert(new_package.path().clone(), new_package); - } + /// Add a module directly to this package. + pub(crate) fn add_direct_module(&mut self, module: DiscoveredModule) { + self.modules.insert(module.path().clone(), module); } pub(crate) fn add_configuration_module(&mut self, module: DiscoveredModule) { self.configuration_module_path = Some(module.module_path().clone()); - self.add_module(module); + self.add_direct_module(module); } - /// Add a package to this package. - /// - /// If the package path equals our path, use update method. - /// - /// Otherwise, strip the current path from the start and add the package to the appropriate sub-package. - pub(crate) fn add_package(&mut self, package: Self) { - if !package.path().starts_with(self.path()) { - return; - } - - if package.path() == self.path() { - self.update(package); - return; - } - - let Ok(relative_path) = package.path().strip_prefix(self.path()) else { - return; - }; - - let components: Vec<_> = relative_path.components().collect(); - - if components.is_empty() { - return; - } - - let first_component = components[0]; - let intermediate_path = self.path().join(first_component); - - if let Some(existing_package) = self.packages.get_mut(&intermediate_path) { - existing_package.add_package(package); - } else { - let mut new_package = Self::new(intermediate_path); - new_package.add_package(package); - self.packages - .insert(new_package.path().clone(), new_package); - } + /// Adds a package directly as a subpackage. + pub(crate) fn add_direct_subpackage(&mut self, other: Self) { + self.packages.insert(other.path().clone(), other); } #[cfg(test)] @@ -174,19 +92,6 @@ impl DiscoveredPackage { total } - pub(crate) fn update(&mut self, package: Self) { - for (_, module) in package.modules { - self.add_module(module); - } - for (_, package) in package.packages { - self.add_package(package); - } - - if let Some(module) = package.configuration_module_path { - self.configuration_module_path = Some(module); - } - } - #[cfg(test)] pub(crate) fn test_functions(&self) -> Vec<&TestFunction> { let mut functions = Vec::new(); diff --git a/crates/karva_core/src/discovery/visitor.rs b/crates/karva_core/src/discovery/visitor.rs index 382aa2cc..c936ade0 100644 --- a/crates/karva_core/src/discovery/visitor.rs +++ b/crates/karva_core/src/discovery/visitor.rs @@ -12,7 +12,7 @@ use crate::{ Context, diagnostic::{report_failed_to_import_module, report_invalid_fixture}, discovery::{DiscoveredModule, TestFunction}, - extensions::fixtures::{Fixture, is_fixture_function}, + extensions::fixtures::Fixture, }; /// Visitor for discovering test functions and fixture definitions in a given module. @@ -21,13 +21,7 @@ struct FunctionDefinitionVisitor<'ctx, 'proj, 'rep, 'py, 'a> { context: &'ctx Context<'proj, 'rep>, /// The current module. - module: &'a DiscoveredModule, - - /// `TestFunction` objects that have been discovered. - test_functions: Vec, - - /// `Fixture` objects that have been discovered. - fixtures: Vec, + module: &'a mut DiscoveredModule, /// We only import the module once we actually need it, this ensures we don't import random files. /// Which has a side effect of running them. @@ -43,13 +37,11 @@ impl<'ctx, 'proj, 'rep, 'py, 'a> FunctionDefinitionVisitor<'ctx, 'proj, 'rep, 'p const fn new( py: Python<'py>, context: &'ctx Context<'proj, 'rep>, - module: &'a DiscoveredModule, + module: &'a mut DiscoveredModule, ) -> Self { Self { context, module, - test_functions: Vec::new(), - fixtures: Vec::new(), py_module: None, py, tried_to_import_module: false, @@ -98,13 +90,13 @@ impl<'ctx, 'proj, 'rep, 'py, 'a> FunctionDefinitionVisitor<'ctx, 'proj, 'rep, 'p if value.is_callable() { let name_str = name.extract::().unwrap_or_default(); - for fixture in &self.fixtures { + for fixture in self.module.fixtures() { if fixture.original_function_name() == name_str { continue 'outer; } } - for function in &self.test_functions { + for function in self.module.test_functions() { if function.name.function_name() == name_str { continue 'outer; } @@ -184,108 +176,94 @@ impl<'ctx, 'proj, 'rep, 'py, 'a> FunctionDefinitionVisitor<'ctx, 'proj, 'rep, 'p if let Ok(fixture_def) = Fixture::try_from_function( self.py, - &Arc::new(stmt_function_def.clone()), + Arc::new(stmt_function_def.clone()), &py_module, self.module.module_path(), is_generator_function, ) { - self.fixtures.push(fixture_def); + self.module.add_fixture(fixture_def); } } } } } -impl SourceOrderVisitor<'_> for FunctionDefinitionVisitor<'_, '_, '_, '_, '_> { - fn visit_stmt(&mut self, stmt: &'_ Stmt) { - if let Stmt::FunctionDef(stmt_function_def) = stmt { - if is_fixture_function(stmt_function_def) { - self.try_import_module(); +impl FunctionDefinitionVisitor<'_, '_, '_, '_, '_> { + fn process_fixture_function(&mut self, stmt_function_def: &Arc) { + self.try_import_module(); - let Some(py_module) = self.py_module.as_ref() else { - return; - }; + let Some(py_module) = self.py_module.as_ref() else { + return; + }; - let mut generator_function_visitor = GeneratorFunctionVisitor::default(); + let mut generator_function_visitor = GeneratorFunctionVisitor::default(); - source_order::walk_body(&mut generator_function_visitor, &stmt_function_def.body); + source_order::walk_body(&mut generator_function_visitor, &stmt_function_def.body); - let is_generator_function = generator_function_visitor.is_generator; + let is_generator_function = generator_function_visitor.is_generator; - match Fixture::try_from_function( + match Fixture::try_from_function( + self.py, + stmt_function_def.clone(), + py_module, + self.module.module_path(), + is_generator_function, + ) { + Ok(fixture_def) => self.module.add_fixture(fixture_def), + Err(e) => { + report_invalid_fixture( + self.context, self.py, - &Arc::new(stmt_function_def.clone()), - py_module, - self.module.module_path(), - is_generator_function, - ) { - Ok(fixture_def) => self.fixtures.push(fixture_def), - Err(e) => { - report_invalid_fixture( - self.context, - self.py, - self.module.source_file(), - stmt_function_def, - &e, - ); - } - } - } else if stmt_function_def - .name - .to_string() - .starts_with(self.context.project().options().test_prefix()) - { - self.try_import_module(); - - let Some(py_module) = self.py_module.as_ref() else { - return; - }; - - if let Ok(py_function) = py_module.getattr(stmt_function_def.name.to_string()) { - self.test_functions.push(TestFunction::new( - self.py, - self.module, - stmt_function_def.clone(), - py_function.unbind(), - )); - } + self.module.source_file(), + stmt_function_def, + &e, + ); } - } else { - // Only walk statement if its not a function statement - // since we don't want to find nested functions - source_order::walk_stmt(self, stmt); } } -} -#[derive(Debug, Default)] -pub struct DiscoveredFunctions { - pub(crate) functions: Vec, - pub(crate) fixtures: Vec, + fn process_test_function(&mut self, stmt_function_def: Arc) { + self.try_import_module(); + + let Some(py_module) = self.py_module.as_ref() else { + return; + }; + + if let Ok(py_function) = py_module.getattr(stmt_function_def.name.to_string()) { + self.module.add_test_function(TestFunction::new( + self.py, + self.module, + stmt_function_def, + py_function.unbind(), + )); + } + } } -pub fn discover(context: &Context, py: Python, module: &DiscoveredModule) -> DiscoveredFunctions { +pub fn discover( + context: &Context, + py: Python, + module: &mut DiscoveredModule, + test_function_defs: Vec>, + fixture_function_defs: Vec>, +) { tracing::info!( "Discovering test functions and fixtures in module {}", module.name() ); - let parsed = parsed_module( - module.source_text(), - context.project().metadata().python_version(), - ); - let mut visitor = FunctionDefinitionVisitor::new(py, context, module); - visitor.visit_body(&parsed.syntax().body); + for test_function_def in test_function_defs { + visitor.process_test_function(test_function_def); + } - if context.project().options().try_import_fixtures() { - visitor.find_extra_fixtures(); + for fixture_function_def in fixture_function_defs { + visitor.process_fixture_function(&fixture_function_def); } - DiscoveredFunctions { - functions: visitor.test_functions, - fixtures: visitor.fixtures, + if context.project().options().try_import_fixtures() { + visitor.find_extra_fixtures(); } } diff --git a/crates/karva_core/src/extensions/fixtures/mod.rs b/crates/karva_core/src/extensions/fixtures/mod.rs index ac0e7b0b..0bde2d21 100644 --- a/crates/karva_core/src/extensions/fixtures/mod.rs +++ b/crates/karva_core/src/extensions/fixtures/mod.rs @@ -98,7 +98,7 @@ impl Fixture { pub(crate) fn try_from_function( py: Python<'_>, - stmt_function_def: &Arc, + stmt_function_def: Arc, py_module: &Bound<'_, PyModule>, module_path: &ModulePath, is_generator_function: bool, @@ -109,7 +109,7 @@ impl Fixture { let try_karva = Self::try_from_karva_function( py, - stmt_function_def, + stmt_function_def.clone(), &function, module_path.clone(), is_generator_function, @@ -142,7 +142,7 @@ impl Fixture { pub(crate) fn try_from_pytest_function( py: Python<'_>, - stmt_function_def: &Arc, + stmt_function_def: Arc, function: &Bound<'_, PyAny>, module_name: ModulePath, is_generator_function: bool, @@ -180,7 +180,7 @@ impl Fixture { Ok(Self::new( py, QualifiedFunctionName::new(name, module_name), - stmt_function_def.clone(), + stmt_function_def, fixture_scope, auto_use.extract::().unwrap_or(false), fixture_function.into(), @@ -191,7 +191,7 @@ impl Fixture { pub(crate) fn try_from_karva_function( py: Python<'_>, - stmt_function_def: &Arc, + stmt_function_def: Arc, function: &Bound<'_, PyAny>, module_path: ModulePath, is_generator_function: bool, @@ -213,7 +213,7 @@ impl Fixture { Ok(Self::new( py, QualifiedFunctionName::new(name, module_path), - stmt_function_def.clone(), + stmt_function_def, fixture_scope, auto_use, py_function.into(), diff --git a/crates/karva_core/src/lib.rs b/crates/karva_core/src/lib.rs index 96f9a372..aca74bc7 100644 --- a/crates/karva_core/src/lib.rs +++ b/crates/karva_core/src/lib.rs @@ -1,3 +1,4 @@ +mod collection; mod context; pub(crate) mod diagnostic; pub(crate) mod discovery; diff --git a/crates/karva_core/src/name.rs b/crates/karva_core/src/name.rs index 8b110299..8a094906 100644 --- a/crates/karva_core/src/name.rs +++ b/crates/karva_core/src/name.rs @@ -1,5 +1,5 @@ use camino::Utf8PathBuf; -use karva_project::utils::module_name; +use karva_project::module_name; /// Represents a fully qualified function name including its module path. #[derive(Clone, Debug, PartialEq, Eq, Hash)] diff --git a/crates/karva_core/src/utils.rs b/crates/karva_core/src/utils.rs index d163696c..6b7912e7 100644 --- a/crates/karva_core/src/utils.rs +++ b/crates/karva_core/src/utils.rs @@ -1,7 +1,7 @@ use std::{collections::HashMap, fmt::Write}; use camino::Utf8Path; -use karva_project::project::{Project, ProjectOptions}; +use karva_project::{Project, ProjectOptions}; use pyo3::{PyResult, Python, prelude::*, types::PyAnyMethods}; use ruff_python_ast::PythonVersion; use ruff_source_file::{SourceFile, SourceFileBuilder}; diff --git a/crates/karva_core/tests/integration/basic.rs b/crates/karva_core/tests/integration/basic.rs index 5b85dd84..dcb668d4 100644 --- a/crates/karva_core/tests/integration/basic.rs +++ b/crates/karva_core/tests/integration/basic.rs @@ -1,7 +1,7 @@ use camino::Utf8PathBuf; use insta::assert_snapshot; use karva_core::{StandardTestRunner, TestRunner, testing::setup_module}; -use karva_project::{Project, project::ProjectOptions}; +use karva_project::{Project, ProjectOptions}; use karva_test::TestContext; use crate::common::TestRunnerExt; diff --git a/crates/karva_core/tests/integration/extensions/fixtures/dynamic.rs b/crates/karva_core/tests/integration/extensions/fixtures/dynamic.rs index c18d3b2a..acd1f570 100644 --- a/crates/karva_core/tests/integration/extensions/fixtures/dynamic.rs +++ b/crates/karva_core/tests/integration/extensions/fixtures/dynamic.rs @@ -1,6 +1,6 @@ use insta::{allow_duplicates, assert_snapshot}; use karva_core::{StandardTestRunner, TestRunner}; -use karva_project::{Project, project::ProjectOptions}; +use karva_project::{Project, ProjectOptions}; use karva_test::TestContext; use rstest::rstest; diff --git a/crates/karva_diff/src/main.rs b/crates/karva_diff/src/main.rs index fcd5ef78..ef1b2ecd 100644 --- a/crates/karva_diff/src/main.rs +++ b/crates/karva_diff/src/main.rs @@ -8,7 +8,7 @@ use std::{ use anyhow::{Context, Result}; use camino::Utf8PathBuf; use clap::Parser; -use karva_project::path::absolute; +use karva_project::absolute; use karva_test::{RealWorldProject, all_projects}; use tempfile::NamedTempFile; @@ -177,5 +177,8 @@ fn run( fn extract_test_result(output: &[u8]) -> String { let output_str = String::from_utf8_lossy(output); - output_str.to_string() + // Strip `; finished in` and all text after it. + let strip_index = output_str.find("; finished in").unwrap_or(output_str.len()); + + output_str[..strip_index].to_string() } diff --git a/crates/karva_project/src/envs.rs b/crates/karva_project/src/envs.rs new file mode 100644 index 00000000..907c7e93 --- /dev/null +++ b/crates/karva_project/src/envs.rs @@ -0,0 +1,19 @@ +use std::num::NonZeroUsize; + +pub struct EnvVars; + +impl EnvVars { + // Externally defined environment variables + + /// This is a standard Rayon environment variable. + pub const RAYON_NUM_THREADS: &'static str = "RAYON_NUM_THREADS"; +} + +pub fn max_parallelism() -> NonZeroUsize { + std::env::var(EnvVars::RAYON_NUM_THREADS) + .ok() + .and_then(|s| s.parse().ok()) + .unwrap_or_else(|| { + std::thread::available_parallelism().unwrap_or_else(|_| NonZeroUsize::new(1).unwrap()) + }) +} diff --git a/crates/karva_project/src/lib.rs b/crates/karva_project/src/lib.rs index a7cfc5fa..5c0c08e5 100644 --- a/crates/karva_project/src/lib.rs +++ b/crates/karva_project/src/lib.rs @@ -1,6 +1,11 @@ -pub mod path; -pub mod project; -pub mod utils; -pub mod verbosity; +mod envs; +mod path; +mod project; +mod utils; +mod verbosity; -pub use project::Project; +pub use envs::{EnvVars, max_parallelism}; +pub use path::{TestPath, TestPathError, absolute}; +pub use project::{Project, ProjectMetadata, ProjectOptions, TestPrefix}; +pub use utils::module_name; +pub use verbosity::VerbosityLevel; diff --git a/crates/karva_project/src/path/mod.rs b/crates/karva_project/src/path/mod.rs index 042ffec0..489ec58f 100644 --- a/crates/karva_project/src/path/mod.rs +++ b/crates/karva_project/src/path/mod.rs @@ -1,4 +1,4 @@ -pub(crate) mod test_path; +pub mod test_path; mod utils; pub use test_path::{TestPath, TestPathError};