From 538a2bd157fa2ddf955f5c463bcc85bb7ccb37ae Mon Sep 17 00:00:00 2001 From: MatthewMckee4 Date: Wed, 15 Apr 2026 14:54:41 +0100 Subject: [PATCH 1/3] Refactor karva_test_semantic for readability and discoverability Consolidate the PyO3 surface, simplify the test-variant classification logic, and extract the Python interpreter attachment helpers out of `utils.rs` into their own module. --- crates/karva_test_semantic/src/diagnostic.rs | 11 +- .../src/extensions/fixtures/mod.rs | 79 +++----- .../src/extensions/fixtures/python.rs | 4 + .../src/extensions/functions/mod.rs | 40 +--- .../src/extensions/functions/python.rs | 45 +++++ .../src/extensions/tags/python.rs | 6 + crates/karva_test_semantic/src/lib.rs | 5 +- crates/karva_test_semantic/src/py_attach.rs | 71 ++++++++ crates/karva_test_semantic/src/python.rs | 14 ++ .../src/runner/fixture_resolver.rs | 3 +- .../src/runner/package_runner.rs | 172 ++++++++---------- crates/karva_test_semantic/src/utils.rs | 111 ----------- 12 files changed, 257 insertions(+), 304 deletions(-) create mode 100644 crates/karva_test_semantic/src/py_attach.rs diff --git a/crates/karva_test_semantic/src/diagnostic.rs b/crates/karva_test_semantic/src/diagnostic.rs index bef1689b..03a51cfc 100644 --- a/crates/karva_test_semantic/src/diagnostic.rs +++ b/crates/karva_test_semantic/src/diagnostic.rs @@ -115,10 +115,13 @@ fn report_dependency_chain( dependency_chain: &[FixtureChainEntry], fixture_name: &str, ) { - let reversed: Vec<_> = dependency_chain.iter().rev().collect(); - - for (i, entry) in reversed.iter().enumerate() { - let next_name = reversed.get(i + 1).map_or(fixture_name, |next| &next.name); + // Walk the chain top-down, pairing each entry with the fixture it depends on. + // The final entry depends on `fixture_name` (the one that actually failed). + let mut entries = dependency_chain.iter().rev().peekable(); + while let Some(entry) = entries.next() { + let next_name = entries + .peek() + .map_or(fixture_name, |next| next.name.as_str()); let mut sub = SubDiagnostic::new( SubDiagnosticSeverity::Info, diff --git a/crates/karva_test_semantic/src/extensions/fixtures/mod.rs b/crates/karva_test_semantic/src/extensions/fixtures/mod.rs index 7c21fe10..444eb78b 100644 --- a/crates/karva_test_semantic/src/extensions/fixtures/mod.rs +++ b/crates/karva_test_semantic/src/extensions/fixtures/mod.rs @@ -204,38 +204,33 @@ impl DiscoveredFixture { } } +const MISSING_FIXTURE_INFO: &str = "Could not find fixture information"; + /// Get the fixture function marker from a function. +/// +/// The second name is for older versions of pytest. fn get_fixture_function_marker<'py>(function: &Bound<'py, PyAny>) -> PyResult> { - let attribute_names = ["_fixture_function_marker", "_pytestfixturefunction"]; - - // Older versions of pytest - for name in attribute_names { - if let Ok(attr) = function.getattr(name) { - return Ok(attr); - } - } - - Err(PyAttributeError::new_err( - "Could not find fixture information", - )) + ["_fixture_function_marker", "_pytestfixturefunction"] + .iter() + .find_map(|name| function.getattr(*name).ok()) + .ok_or_else(|| PyAttributeError::new_err(MISSING_FIXTURE_INFO)) } /// Get the fixture function from a function. +/// +/// Falls back to the pre-8.0 pytest `__pytest_wrapped__.obj` path. fn get_fixture_function<'py>(function: &Bound<'py, PyAny>) -> PyResult> { if let Ok(attr) = function.getattr("_fixture_function") { return Ok(attr); } - // Older versions of pytest - if let Ok(attr) = function.getattr("__pytest_wrapped__") { - if let Ok(attr) = attr.getattr("obj") { - return Ok(attr); - } + if let Ok(wrapped) = function.getattr("__pytest_wrapped__") + && let Ok(obj) = wrapped.getattr("obj") + { + return Ok(obj); } - Err(PyAttributeError::new_err( - "Could not find fixture information", - )) + Err(PyAttributeError::new_err(MISSING_FIXTURE_INFO)) } pub fn get_auto_use_fixtures<'a>( @@ -243,39 +238,17 @@ pub fn get_auto_use_fixtures<'a>( current: &'a dyn HasFixtures<'a>, scope: FixtureScope, ) -> Vec<&'a DiscoveredFixture> { - let mut auto_use_fixtures_called = Vec::new(); - let auto_use_fixtures = current.auto_use_fixtures(&scope.scopes_above()); - - for fixture in auto_use_fixtures { - let fixture_name = fixture.name().function_name().to_string(); - - if auto_use_fixtures_called - .iter() - .any(|fixture: &&DiscoveredFixture| fixture.name().function_name() == fixture_name) - { - continue; - } - - auto_use_fixtures_called.push(fixture); - } - - for parent in parents { - let parent_fixtures = parent.auto_use_fixtures(&[scope]); - for fixture in parent_fixtures { - let fixture_name = fixture.name().function_name().to_string(); - - if auto_use_fixtures_called - .iter() - .any(|fixture: &&DiscoveredFixture| fixture.name().function_name() == fixture_name) - { - continue; - } - - auto_use_fixtures_called.push(fixture); - } - } - - auto_use_fixtures_called + let current_fixtures = current.auto_use_fixtures(&scope.scopes_above()); + let parent_fixtures = parents + .iter() + .flat_map(|parent| parent.auto_use_fixtures(&[scope])); + + let mut seen: std::collections::HashSet<&str> = std::collections::HashSet::new(); + current_fixtures + .into_iter() + .chain(parent_fixtures) + .filter(|fixture| seen.insert(fixture.name().function_name())) + .collect() } #[cfg(test)] diff --git a/crates/karva_test_semantic/src/extensions/fixtures/python.rs b/crates/karva_test_semantic/src/extensions/fixtures/python.rs index 201d132d..71803d0d 100644 --- a/crates/karva_test_semantic/src/extensions/fixtures/python.rs +++ b/crates/karva_test_semantic/src/extensions/fixtures/python.rs @@ -1,3 +1,7 @@ +//! `PyO3` bindings for the `@karva.fixture` decorator and its companion types +//! (`FixtureFunctionMarker`, `FixtureFunctionDefinition`), plus the +//! `InvalidFixtureError` exception surfaced to Python. + use pyo3::prelude::*; use pyo3::types::{PyDict, PyTuple}; diff --git a/crates/karva_test_semantic/src/extensions/functions/mod.rs b/crates/karva_test_semantic/src/extensions/functions/mod.rs index 638fd2c2..d6d41b19 100644 --- a/crates/karva_test_semantic/src/extensions/functions/mod.rs +++ b/crates/karva_test_semantic/src/extensions/functions/mod.rs @@ -1,45 +1,7 @@ +pub use self::python::{FailError, Param, SkipError, fail, param, skip}; pub use self::raises::{ExceptionInfo, RaisesContext}; pub use self::snapshot::{Command, SnapshotMismatchError, SnapshotSettings}; -use pyo3::prelude::*; -pub use python::Param; pub mod python; pub mod raises; pub mod snapshot; - -// SkipError exception that can be raised to skip tests at runtime with an optional reason -pyo3::create_exception!(karva, SkipError, pyo3::exceptions::PyException); - -// FailError exception that can be raised to fail tests at runtime with an optional reason -pyo3::create_exception!(karva, FailError, pyo3::exceptions::PyException); - -/// Skip the current test at runtime with an optional reason. -/// -/// This function raises a `SkipError` exception which will be caught by the test runner -/// and mark the test as skipped. -#[pyfunction] -#[pyo3(signature = (reason = None))] -pub fn skip(_py: Python<'_>, reason: Option) -> PyResult<()> { - let message = reason.unwrap_or_default(); - Err(SkipError::new_err(message)) -} - -/// Fail the current test at runtime with an optional reason. -/// -/// This function raises a `FailError` exception which will be caught by the test runner -/// and mark the test as failed with the given reason. -#[pyfunction] -#[pyo3(signature = (reason = None))] -pub fn fail(_py: Python<'_>, reason: Option) -> PyResult<()> { - Err(FailError::new_err(reason)) -} - -#[pyfunction] -#[pyo3(signature = (*values, tags = None))] -pub fn param( - py: Python<'_>, - values: Vec>, - tags: Option>>, -) -> PyResult { - Param::new(py, values, tags.unwrap_or_default()) -} diff --git a/crates/karva_test_semantic/src/extensions/functions/python.rs b/crates/karva_test_semantic/src/extensions/functions/python.rs index 6cbde6bd..60b16ed1 100644 --- a/crates/karva_test_semantic/src/extensions/functions/python.rs +++ b/crates/karva_test_semantic/src/extensions/functions/python.rs @@ -1,3 +1,11 @@ +//! `PyO3` bindings for the top-level `karva.*` test API — runtime test-control +//! functions (`skip`, `fail`, `param`), the value types they produce, and the +//! exception classes the runner understands. +//! +//! The more involved `PyO3` types for this module live next to their Rust +//! helpers in [`super::raises`] and [`super::snapshot`], where splitting them +//! would only add `pub(super)` noise. + use std::sync::Arc; use pyo3::exceptions::PyTypeError; @@ -6,6 +14,12 @@ use pyo3::prelude::*; use crate::extensions::tags::parametrize::Parametrization; use crate::extensions::tags::{Tag, Tags}; +// SkipError exception that can be raised to skip tests at runtime with an optional reason +pyo3::create_exception!(karva, SkipError, pyo3::exceptions::PyException); + +// FailError exception that can be raised to fail tests at runtime with an optional reason +pyo3::create_exception!(karva, FailError, pyo3::exceptions::PyException); + #[derive(Debug, Clone)] #[pyclass(from_py_object)] pub struct Param { @@ -37,3 +51,34 @@ impl Param { Self { values, tags } } } + +/// Skip the current test at runtime with an optional reason. +/// +/// This function raises a `SkipError` exception which will be caught by the test runner +/// and mark the test as skipped. +#[pyfunction] +#[pyo3(signature = (reason = None))] +pub fn skip(_py: Python<'_>, reason: Option) -> PyResult<()> { + let message = reason.unwrap_or_default(); + Err(SkipError::new_err(message)) +} + +/// Fail the current test at runtime with an optional reason. +/// +/// This function raises a `FailError` exception which will be caught by the test runner +/// and mark the test as failed with the given reason. +#[pyfunction] +#[pyo3(signature = (reason = None))] +pub fn fail(_py: Python<'_>, reason: Option) -> PyResult<()> { + Err(FailError::new_err(reason)) +} + +#[pyfunction] +#[pyo3(signature = (*values, tags = None))] +pub fn param( + py: Python<'_>, + values: Vec>, + tags: Option>>, +) -> PyResult { + Param::new(py, values, tags.unwrap_or_default()) +} diff --git a/crates/karva_test_semantic/src/extensions/tags/python.rs b/crates/karva_test_semantic/src/extensions/tags/python.rs index 1818c70c..346c48b1 100644 --- a/crates/karva_test_semantic/src/extensions/tags/python.rs +++ b/crates/karva_test_semantic/src/extensions/tags/python.rs @@ -1,3 +1,9 @@ +//! `PyO3` bindings for the `@karva.tags.*` decorator API. +//! +//! Exposes the `tag` enum, the `Tags` builder, the nested `tags` submodule +//! with built-in tags (`parametrize`, `use_fixtures`, `skip`, `expect_fail`), +//! and the dynamic `CustomTagBuilder` / `TestFunction` classes. + use pyo3::IntoPyObjectExt; use pyo3::exceptions::PyTypeError; use pyo3::prelude::*; diff --git a/crates/karva_test_semantic/src/lib.rs b/crates/karva_test_semantic/src/lib.rs index 2316a1f5..296ce888 100644 --- a/crates/karva_test_semantic/src/lib.rs +++ b/crates/karva_test_semantic/src/lib.rs @@ -3,6 +3,7 @@ mod context; pub(crate) mod diagnostic; pub(crate) mod discovery; pub(crate) mod extensions; +mod py_attach; mod python; mod runner; pub mod utils; @@ -17,8 +18,8 @@ use karva_project::path::{TestPath, TestPathError}; use ruff_python_ast::PythonVersion; use crate::discovery::StandardDiscoverer; +use crate::py_attach::attach_with_output; use crate::runner::PackageRunner; -use crate::utils::attach_with_project; /// Run tests given the system, settings, Python version, reporter, and test paths. /// @@ -33,7 +34,7 @@ pub fn run_tests( ) -> TestRunResult { let context = Context::new(cwd, settings, python_version, reporter); - attach_with_project(settings.terminal().show_python_output, |py| { + attach_with_output(settings.terminal().show_python_output, |py| { let session = StandardDiscoverer::new(&context).discover_with_py(py, test_paths); PackageRunner::new(&context).execute(py, &session); diff --git a/crates/karva_test_semantic/src/py_attach.rs b/crates/karva_test_semantic/src/py_attach.rs new file mode 100644 index 00000000..2b0882d6 --- /dev/null +++ b/crates/karva_test_semantic/src/py_attach.rs @@ -0,0 +1,71 @@ +//! Python interpreter attachment helpers. +//! +//! Wraps [`pyo3::Python::attach`] with first-time interpreter initialization +//! and optional suppression of `sys.stdout` / `sys.stderr` to `/dev/null` +//! for the duration of the callback. + +use pyo3::prelude::*; + +/// Initialize the Python interpreter (idempotent) and attach to it for the +/// duration of `f`. +fn attach(f: F) -> R +where + F: for<'py> FnOnce(Python<'py>) -> R, +{ + Python::initialize(); + Python::attach(f) +} + +/// Like [`attach`], but redirects Python's `sys.stdout` and `sys.stderr` to +/// `/dev/null` for the duration of `f` when `show_output` is `false`. +/// +/// If `/dev/null` cannot be opened we fall back to unsuppressed output rather +/// than failing the test run. +pub fn attach_with_output(show_output: bool, f: F) -> R +where + F: for<'py> FnOnce(Python<'py>) -> R, +{ + attach(|py| { + if show_output { + return f(py); + } + + let Ok(null_file) = open_devnull(py) else { + return f(py); + }; + + let _ = redirect_stdio(py, &null_file); + let result = f(py); + let _ = flush_and_mute(py, &null_file); + result + }) +} + +fn open_devnull(py: Python<'_>) -> PyResult> { + let os = py.import("os")?; + let builtins = py.import("builtins")?; + builtins + .getattr("open")? + .call1((os.getattr("devnull")?, "w")) +} + +fn redirect_stdio<'py>(py: Python<'py>, null_file: &Bound<'py, PyAny>) -> PyResult<()> { + let sys = py.import("sys")?; + for stream in ["stdout", "stderr"] { + sys.setattr(stream, null_file.clone())?; + } + Ok(()) +} + +/// Close whatever is currently on `sys.stdout`/`sys.stderr` (so pending writes +/// flush) and reset both to `null_file`. We don't restore the originals — the +/// runner doesn't emit to real stdout after the callback returns, and a test +/// may have swapped the streams itself. +fn flush_and_mute<'py>(py: Python<'py>, null_file: &Bound<'py, PyAny>) -> PyResult<()> { + let sys = py.import("sys")?; + for stream in ["stdout", "stderr"] { + sys.getattr(stream)?.call_method0("close")?; + sys.setattr(stream, null_file.clone())?; + } + Ok(()) +} diff --git a/crates/karva_test_semantic/src/python.rs b/crates/karva_test_semantic/src/python.rs index 545da879..7167bd64 100644 --- a/crates/karva_test_semantic/src/python.rs +++ b/crates/karva_test_semantic/src/python.rs @@ -1,3 +1,17 @@ +//! Registration hub for the `PyO3` surface exposed to Python as `karva._karva`. +//! +//! Each extension family owns its own Python-facing file: +//! - [`crate::extensions::tags::python`] — the `@karva.tags.*` decorator API +//! - [`crate::extensions::fixtures::python`] — the `@karva.fixture` decorator and companion types +//! - [`crate::extensions::functions::python`] — top-level `karva.{skip, fail, param}` plus their exceptions +//! - [`crate::extensions::functions::raises`] and [`crate::extensions::functions::snapshot`] +//! — cohesive `PyO3` + Rust modules that stay together because their classes +//! are tightly coupled to private state in the same file +//! +//! [`init_module`] below is the single place every `#[pyclass]`, +//! `#[pyfunction]`, and `create_exception!` gets handed to the interpreter — +//! grep here first when adding or removing a binding. + use pyo3::prelude::*; use pyo3::wrap_pymodule; diff --git a/crates/karva_test_semantic/src/runner/fixture_resolver.rs b/crates/karva_test_semantic/src/runner/fixture_resolver.rs index 82de6a79..24261e5a 100644 --- a/crates/karva_test_semantic/src/runner/fixture_resolver.rs +++ b/crates/karva_test_semantic/src/runner/fixture_resolver.rs @@ -8,7 +8,6 @@ use crate::extensions::fixtures::{ DiscoveredFixture, FixtureScope, HasFixtures, NormalizedFixture, RequiresFixtures, get_auto_use_fixtures, }; -use crate::utils::iter_with_ancestors; /// Resolves fixtures at runtime during test execution. /// @@ -150,7 +149,7 @@ fn find_fixture<'a>( return Some(fixture); } - for (parent, _ancestors) in iter_with_ancestors(parents) { + for parent in parents { if let Some(fixture) = parent.get_fixture(name) && current_fixture .is_none_or(|current_fixture| current_fixture.name() != fixture.name()) diff --git a/crates/karva_test_semantic/src/runner/package_runner.rs b/crates/karva_test_semantic/src/runner/package_runner.rs index c939500b..094bd006 100644 --- a/crates/karva_test_semantic/src/runner/package_runner.rs +++ b/crates/karva_test_semantic/src/runner/package_runner.rs @@ -307,93 +307,75 @@ impl<'ctx, 'a> PackageRunner<'ctx, 'a> { } /// Classify a test result, handling `expect_fail` logic and error reporting. - #[expect(clippy::too_many_arguments)] fn classify_test_result( &self, py: Python<'_>, test_result: PyResult>, - expect_fail: bool, - expect_fail_tag: Option, - qualified_test_name: &QualifiedTestName, - name: &QualifiedFunctionName, - test_module_path: &camino::Utf8PathBuf, - stmt_function_def: &StmtFunctionDef, - function_arguments: &FixtureArguments, fixture_call_errors: Vec, - start_time: std::time::Instant, + ctx: &VariantReportCtx<'_>, ) -> bool { - match test_result { - Ok(_) => { - if expect_fail { - let reason = expect_fail_tag.and_then(|tag| tag.reason()); - - report_test_pass_on_expect_failure( - self.context, - source_file(test_module_path), - stmt_function_def, - reason, - ); - - self.context.register_test_case_result( - qualified_test_name, - IndividualTestResultKind::Failed, - start_time.elapsed(), - ) - } else { - self.context.register_test_case_result( - qualified_test_name, - IndividualTestResultKind::Passed, - start_time.elapsed(), - ) - } - } - Err(err) => { - if is_skip_exception(py, &err) { - let reason = extract_skip_reason(py, &err); - self.context.register_test_case_result( - qualified_test_name, - IndividualTestResultKind::Skipped { reason }, - start_time.elapsed(), - ) - } else if expect_fail { - self.context.register_test_case_result( - qualified_test_name, - IndividualTestResultKind::Passed, - start_time.elapsed(), - ) - } else { - let missing_args = - missing_arguments_from_error(name.function_name(), &err.to_string()); - - if missing_args.is_empty() { - report_test_failure( - self.context, - py, - source_file(test_module_path), - stmt_function_def, - function_arguments, - &err, - ); - } else { - report_missing_fixtures( - self.context, - py, - source_file(test_module_path), - stmt_function_def, - &missing_args, - FunctionKind::Test, - fixture_call_errors, - ); - } + let register = |kind: IndividualTestResultKind| { + self.context.register_test_case_result( + ctx.qualified_test_name, + kind, + ctx.start_time.elapsed(), + ) + }; - self.context.register_test_case_result( - qualified_test_name, - IndividualTestResultKind::Failed, - start_time.elapsed(), - ) - } + let expect_fail = ctx + .expect_fail_tag + .as_ref() + .is_some_and(ExpectFailTag::should_expect_fail); + + let err = match test_result { + Ok(_) if expect_fail => { + let reason = ctx.expect_fail_tag.as_ref().and_then(ExpectFailTag::reason); + report_test_pass_on_expect_failure( + self.context, + source_file(ctx.test_module_path), + ctx.stmt_function_def, + reason, + ); + return register(IndividualTestResultKind::Failed); } + Ok(_) => return register(IndividualTestResultKind::Passed), + Err(err) => err, + }; + + if is_skip_exception(py, &err) { + return register(IndividualTestResultKind::Skipped { + reason: extract_skip_reason(py, &err), + }); + } + + if expect_fail { + return register(IndividualTestResultKind::Passed); + } + + let missing_args = missing_arguments_from_error(ctx.name.function_name(), &err.to_string()); + + if missing_args.is_empty() { + report_test_failure( + self.context, + py, + source_file(ctx.test_module_path), + ctx.stmt_function_def, + ctx.function_arguments, + &err, + ); + } else { + report_missing_fixtures( + self.context, + py, + source_file(ctx.test_module_path), + ctx.stmt_function_def, + &missing_args, + FunctionKind::Test, + fixture_call_errors, + ); } + + register(IndividualTestResultKind::Failed) } /// Run a test variant (a specific combination of parametrize values and fixtures). @@ -419,11 +401,7 @@ impl<'ctx, 'a> PackageRunner<'ctx, 'a> { } let start_time = std::time::Instant::now(); - let expect_fail_tag = tags.expect_fail_tag(); - let expect_fail = expect_fail_tag - .as_ref() - .is_some_and(ExpectFailTag::should_expect_fail); let (function_arguments, fixture_call_errors, test_finalizers) = self.setup_test_fixtures( py, @@ -482,19 +460,16 @@ impl<'ctx, 'a> PackageRunner<'ctx, 'a> { test_result = run_test(); } - let passed = self.classify_test_result( - py, - test_result, - expect_fail, + let report_ctx = VariantReportCtx { + name: &name, + qualified_test_name: &qualified_test_name, + test_module_path: &test_module_path, + stmt_function_def: &stmt_function_def, + function_arguments: &function_arguments, expect_fail_tag, - &qualified_test_name, - &name, - &test_module_path, - &stmt_function_def, - &function_arguments, - fixture_call_errors, start_time, - ); + }; + let passed = self.classify_test_result(py, test_result, fixture_call_errors, &report_ctx); for finalizer in test_finalizers.into_iter().rev() { finalizer.run(self.context, py); @@ -671,6 +646,17 @@ fn get_value_and_finalizer( } } +/// Immutable per-variant state threaded into [`PackageRunner::classify_test_result`]. +struct VariantReportCtx<'a> { + name: &'a QualifiedFunctionName, + qualified_test_name: &'a QualifiedTestName, + test_module_path: &'a camino::Utf8Path, + stmt_function_def: &'a StmtFunctionDef, + function_arguments: &'a FixtureArguments, + expect_fail_tag: Option, + start_time: std::time::Instant, +} + pub struct FixtureCallError { pub(crate) fixture_name: String, pub(crate) error: PyErr, diff --git a/crates/karva_test_semantic/src/utils.rs b/crates/karva_test_semantic/src/utils.rs index e729c324..166ab7dc 100644 --- a/crates/karva_test_semantic/src/utils.rs +++ b/crates/karva_test_semantic/src/utils.rs @@ -95,96 +95,6 @@ pub(crate) fn add_to_sys_path(py: Python<'_>, path: &Utf8Path, index: isize) -> Ok(()) } -/// Redirects Python's stdout and stderr to /dev/null if output is disabled. -/// -/// This function is used to suppress Python output during test execution -/// when the user hasn't requested to see it. It returns a handle to the -/// null file for later restoration. -fn redirect_python_output( - py: Python<'_>, - show_python_output: bool, -) -> PyResult>> { - if show_python_output { - return Ok(None); - } - let sys = py.import("sys")?; - let os = py.import("os")?; - let builtins = py.import("builtins")?; - - let devnull = os.getattr("devnull")?; - let open_file_function = builtins.getattr("open")?; - let null_file = open_file_function.call1((devnull, "w"))?; - - for output in ["stdout", "stderr"] { - sys.setattr(output, null_file.clone())?; - } - - Ok(Some(null_file)) -} - -/// Restores Python's stdout and stderr from the null file redirect. -/// -/// This function cleans up the output redirection by closing the null file -/// handles and restoring normal output streams. -fn restore_python_output<'py>(py: Python<'py>, null_file: &Bound<'py, PyAny>) -> PyResult<()> { - let sys = py.import("sys")?; - - for output in ["stdout", "stderr"] { - let current_output = sys.getattr(output)?; - let close_method = current_output.getattr("close")?; - close_method.call0()?; - sys.setattr(output, null_file.clone())?; - } - - Ok(()) -} - -/// A wrapper around `Python::attach` so we can manage the stdout and stderr redirection. -pub(crate) fn attach_with_project(show_python_output: bool, f: F) -> R -where - F: for<'py> FnOnce(Python<'py>) -> R, -{ - attach(|py| { - let null_file = redirect_python_output(py, show_python_output); - let result = f(py); - if let Ok(Some(null_file)) = null_file { - let _ = restore_python_output(py, &null_file); - } - result - }) -} - -/// A simple wrapper around `Python::attach` that initializes the Python interpreter first. -pub(crate) fn attach(f: F) -> R -where - F: for<'py> FnOnce(Python<'py>) -> R, -{ - Python::initialize(); - Python::attach(f) -} - -/// Creates an iterator that yields each item with all items after it. -/// -/// For example, given [session, package, module], -/// it yields: (module, [session, package]), (package, [session]), (session, []). -pub(crate) fn iter_with_ancestors<'a, T: ?Sized>( - items: &[&'a T], -) -> impl Iterator)> { - let mut ancestors = items.to_vec(); - let mut current_index = items.len(); - - std::iter::from_fn(move || { - if current_index > 0 { - current_index -= 1; - let current_item = items[current_index]; - ancestors.truncate(current_index); - Some((current_item, ancestors.clone())) - } else { - None - } - }) -} - pub(crate) fn full_test_name( py: Python, function: String, @@ -225,24 +135,3 @@ pub(crate) fn truncate_string(value: &str) -> String { value.to_string() } } - -#[cfg(test)] -mod tests { - use super::*; - - mod utils_tests { - use super::*; - - #[test] - fn test_iter_with_ancestors() { - let items = vec!["session", "package", "module"]; - let expected = vec![ - ("module", vec!["session", "package"]), - ("package", vec!["session"]), - ("session", vec![]), - ]; - let result: Vec<(&str, Vec<&str>)> = iter_with_ancestors(&items).collect(); - assert_eq!(result, expected); - } - } -} From 76a0ef77dd9c790b023f1bcee8732ed68384bed5 Mon Sep 17 00:00:00 2001 From: MatthewMckee4 Date: Wed, 15 Apr 2026 15:33:26 +0100 Subject: [PATCH 2/3] Drop module-level docs on python.rs files --- .../src/extensions/fixtures/python.rs | 4 ---- .../src/extensions/functions/python.rs | 8 -------- .../src/extensions/tags/python.rs | 6 ------ crates/karva_test_semantic/src/python.rs | 14 -------------- 4 files changed, 32 deletions(-) diff --git a/crates/karva_test_semantic/src/extensions/fixtures/python.rs b/crates/karva_test_semantic/src/extensions/fixtures/python.rs index 71803d0d..201d132d 100644 --- a/crates/karva_test_semantic/src/extensions/fixtures/python.rs +++ b/crates/karva_test_semantic/src/extensions/fixtures/python.rs @@ -1,7 +1,3 @@ -//! `PyO3` bindings for the `@karva.fixture` decorator and its companion types -//! (`FixtureFunctionMarker`, `FixtureFunctionDefinition`), plus the -//! `InvalidFixtureError` exception surfaced to Python. - use pyo3::prelude::*; use pyo3::types::{PyDict, PyTuple}; diff --git a/crates/karva_test_semantic/src/extensions/functions/python.rs b/crates/karva_test_semantic/src/extensions/functions/python.rs index 60b16ed1..268c899d 100644 --- a/crates/karva_test_semantic/src/extensions/functions/python.rs +++ b/crates/karva_test_semantic/src/extensions/functions/python.rs @@ -1,11 +1,3 @@ -//! `PyO3` bindings for the top-level `karva.*` test API — runtime test-control -//! functions (`skip`, `fail`, `param`), the value types they produce, and the -//! exception classes the runner understands. -//! -//! The more involved `PyO3` types for this module live next to their Rust -//! helpers in [`super::raises`] and [`super::snapshot`], where splitting them -//! would only add `pub(super)` noise. - use std::sync::Arc; use pyo3::exceptions::PyTypeError; diff --git a/crates/karva_test_semantic/src/extensions/tags/python.rs b/crates/karva_test_semantic/src/extensions/tags/python.rs index 346c48b1..1818c70c 100644 --- a/crates/karva_test_semantic/src/extensions/tags/python.rs +++ b/crates/karva_test_semantic/src/extensions/tags/python.rs @@ -1,9 +1,3 @@ -//! `PyO3` bindings for the `@karva.tags.*` decorator API. -//! -//! Exposes the `tag` enum, the `Tags` builder, the nested `tags` submodule -//! with built-in tags (`parametrize`, `use_fixtures`, `skip`, `expect_fail`), -//! and the dynamic `CustomTagBuilder` / `TestFunction` classes. - use pyo3::IntoPyObjectExt; use pyo3::exceptions::PyTypeError; use pyo3::prelude::*; diff --git a/crates/karva_test_semantic/src/python.rs b/crates/karva_test_semantic/src/python.rs index 7167bd64..545da879 100644 --- a/crates/karva_test_semantic/src/python.rs +++ b/crates/karva_test_semantic/src/python.rs @@ -1,17 +1,3 @@ -//! Registration hub for the `PyO3` surface exposed to Python as `karva._karva`. -//! -//! Each extension family owns its own Python-facing file: -//! - [`crate::extensions::tags::python`] — the `@karva.tags.*` decorator API -//! - [`crate::extensions::fixtures::python`] — the `@karva.fixture` decorator and companion types -//! - [`crate::extensions::functions::python`] — top-level `karva.{skip, fail, param}` plus their exceptions -//! - [`crate::extensions::functions::raises`] and [`crate::extensions::functions::snapshot`] -//! — cohesive `PyO3` + Rust modules that stay together because their classes -//! are tightly coupled to private state in the same file -//! -//! [`init_module`] below is the single place every `#[pyclass]`, -//! `#[pyfunction]`, and `create_exception!` gets handed to the interpreter — -//! grep here first when adding or removing a binding. - use pyo3::prelude::*; use pyo3::wrap_pymodule; From 87097fe258530c3e18d76ff4a9dabb1de292554a Mon Sep 17 00:00:00 2001 From: MatthewMckee4 Date: Wed, 15 Apr 2026 15:35:49 +0100 Subject: [PATCH 3/3] Handle missing reason consistently in skip/fail --- crates/karva_test_semantic/src/extensions/functions/python.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/karva_test_semantic/src/extensions/functions/python.rs b/crates/karva_test_semantic/src/extensions/functions/python.rs index 268c899d..19f44aba 100644 --- a/crates/karva_test_semantic/src/extensions/functions/python.rs +++ b/crates/karva_test_semantic/src/extensions/functions/python.rs @@ -51,8 +51,7 @@ impl Param { #[pyfunction] #[pyo3(signature = (reason = None))] pub fn skip(_py: Python<'_>, reason: Option) -> PyResult<()> { - let message = reason.unwrap_or_default(); - Err(SkipError::new_err(message)) + Err(SkipError::new_err(reason)) } /// Fail the current test at runtime with an optional reason.