From 4a12af7a97c8bfa177e797606d3f281fe0a68ed7 Mon Sep 17 00:00:00 2001 From: Matthew Mckee Date: Tue, 2 Dec 2025 23:01:57 +0000 Subject: [PATCH 1/4] Clean up, code docs --- crates/karva_core/src/diagnostic/mod.rs | 20 ++++++++++++++++++- crates/karva_core/src/diagnostic/result.rs | 5 ++++- .../src/discovery/models/function.rs | 1 - crates/karva_project/src/path/test_path.rs | 1 - 4 files changed, 23 insertions(+), 4 deletions(-) diff --git a/crates/karva_core/src/diagnostic/mod.rs b/crates/karva_core/src/diagnostic/mod.rs index 7fd53c63..fa57cbe9 100644 --- a/crates/karva_core/src/diagnostic/mod.rs +++ b/crates/karva_core/src/diagnostic/mod.rs @@ -33,7 +33,7 @@ use crate::{ declare_diagnostic_type! { /// ## Invalid path /// - /// User gave an invalid path + /// User has provided an invalid path that we cannot resolve. pub static INVALID_PATH = { summary: "User provided an invalid path", severity: Severity::Error, @@ -42,6 +42,9 @@ declare_diagnostic_type! { declare_diagnostic_type! { /// ## Failed to import module + /// + /// This comes from when we try to import tests or fixtures. + /// If we try to import a module and it fails, we will raise this error. pub static FAILED_TO_IMPORT_MODULE = { summary: "Failed to import python module", severity: Severity::Error, @@ -50,6 +53,9 @@ declare_diagnostic_type! { declare_diagnostic_type! { /// ## Invalid fixture + /// + /// There are several reasons a fixture may be invalid, + /// we raise this error when we detect one. pub static INVALID_FIXTURE = { summary: "Discovered an invalid fixture", severity: Severity::Error, @@ -58,6 +64,9 @@ declare_diagnostic_type! { declare_diagnostic_type! { /// ## Invalid fixture finalizer + /// + /// If a finalizer raises an exception, we will raise this error. + /// If a finalizer tries to yield another value, we will raise this error. pub static INVALID_FIXTURE_FINALIZER = { summary: "Tried to run an invalid fixture finalizer", severity: Severity::Warning, @@ -66,6 +75,9 @@ declare_diagnostic_type! { declare_diagnostic_type! { /// ## Missing fixtures + /// + /// If we try to run a test or function without all the required fixtures, + /// we will raise this error. pub static MISSING_FIXTURES = { summary: "Missing fixtures", severity: Severity::Error, @@ -74,6 +86,8 @@ declare_diagnostic_type! { declare_diagnostic_type! { /// ## Failed Fixture + /// + /// If we call a fixture and it raises an exception, we will raise this error. pub static FIXTURE_FAILURE = { summary: "Fixture raises exception when run", severity: Severity::Error, @@ -82,6 +96,8 @@ declare_diagnostic_type! { declare_diagnostic_type! { /// ## Test Passes when expected to fail + /// + /// If a test marked as `expect_failure` passes, we will raise this error. pub static TEST_PASS_ON_EXPECT_FAILURE = { summary: "Test passes when expected to fail", severity: Severity::Error, @@ -90,6 +106,8 @@ declare_diagnostic_type! { declare_diagnostic_type! { /// ## Failed Test + /// + /// If a test raises an exception, we will raise this error. pub static TEST_FAILURE = { summary: "Test raises exception when run", severity: Severity::Error, diff --git a/crates/karva_core/src/diagnostic/result.rs b/crates/karva_core/src/diagnostic/result.rs index 5c78cf07..f745f990 100644 --- a/crates/karva_core/src/diagnostic/result.rs +++ b/crates/karva_core/src/diagnostic/result.rs @@ -12,6 +12,9 @@ pub struct TestRunResultDisplayOptions { pub(crate) diagnostic_format: DiagnosticFormat, } +/// Represents the result of a test run. +/// +/// This is held in the test context and updated throughout the test run. #[derive(Debug, Clone)] pub struct TestRunResult { /// Diagnostics generated during test discovery. @@ -28,7 +31,7 @@ pub struct TestRunResult { /// Current working directory. /// - /// This is used + /// This is used to resolve file paths in diagnostics. cwd: std::path::PathBuf, /// Display options diff --git a/crates/karva_core/src/discovery/models/function.rs b/crates/karva_core/src/discovery/models/function.rs index 6eb81153..b24c5655 100644 --- a/crates/karva_core/src/discovery/models/function.rs +++ b/crates/karva_core/src/discovery/models/function.rs @@ -64,7 +64,6 @@ impl RequiresFixtures for TestFunction { #[cfg(test)] mod tests { - use karva_project::project::Project; use karva_test::TestContext; diff --git a/crates/karva_project/src/path/test_path.rs b/crates/karva_project/src/path/test_path.rs index c9a90de1..832dc01d 100644 --- a/crates/karva_project/src/path/test_path.rs +++ b/crates/karva_project/src/path/test_path.rs @@ -76,7 +76,6 @@ impl TestPath { Err(TestPathError::InvalidUtf8Path(path)) } } else { - // Original file/directory logic let value = Utf8PathBuf::from(value); let path = try_convert_to_py_path(&value)?; From 47a8914e178e801290968ea161513614df154d85 Mon Sep 17 00:00:00 2001 From: Matthew Mckee Date: Wed, 3 Dec 2025 00:51:56 +0000 Subject: [PATCH 2/4] Update fixture parsing --- crates/karva_core/src/diagnostic/mod.rs | 9 +- crates/karva_core/src/discovery/visitor.rs | 1 + .../karva_core/src/extensions/fixtures/mod.rs | 96 ++++++++++++------- .../src/extensions/fixtures/python.rs | 3 + crates/karva_core/src/python.rs | 4 +- 5 files changed, 78 insertions(+), 35 deletions(-) diff --git a/crates/karva_core/src/diagnostic/mod.rs b/crates/karva_core/src/diagnostic/mod.rs index fa57cbe9..7d7f5d1a 100644 --- a/crates/karva_core/src/diagnostic/mod.rs +++ b/crates/karva_core/src/diagnostic/mod.rs @@ -130,9 +130,10 @@ pub fn report_failed_to_import_module(context: &Context, module_name: &str, erro pub fn report_invalid_fixture( context: &Context, + py: Python, source_file: SourceFile, stmt_function_def: &StmtFunctionDef, - reason: &str, + error: &PyErr, ) { let builder = context.report_diagnostic(&INVALID_FIXTURE); @@ -145,7 +146,11 @@ pub fn report_invalid_fixture( diagnostic.annotate(Annotation::primary(primary_span)); - diagnostic.info(reason); + let error_string = error.value(py).to_string(); + + if !error_string.is_empty() { + diagnostic.info(format!("Error message: {error_string}")); + } } pub fn report_invalid_fixture_finalizer( diff --git a/crates/karva_core/src/discovery/visitor.rs b/crates/karva_core/src/discovery/visitor.rs index ac0f80b2..382aa2cc 100644 --- a/crates/karva_core/src/discovery/visitor.rs +++ b/crates/karva_core/src/discovery/visitor.rs @@ -223,6 +223,7 @@ impl SourceOrderVisitor<'_> for FunctionDefinitionVisitor<'_, '_, '_, '_, '_> { Err(e) => { report_invalid_fixture( self.context, + self.py, self.module.source_file(), stmt_function_def, &e, diff --git a/crates/karva_core/src/extensions/fixtures/mod.rs b/crates/karva_core/src/extensions/fixtures/mod.rs index 16739f2b..ac0e7b0b 100644 --- a/crates/karva_core/src/extensions/fixtures/mod.rs +++ b/crates/karva_core/src/extensions/fixtures/mod.rs @@ -1,6 +1,6 @@ use std::sync::Arc; -use pyo3::prelude::*; +use pyo3::{exceptions::PyAttributeError, prelude::*}; use ruff_python_ast::{Expr, StmtFunctionDef}; mod builtins; @@ -23,7 +23,9 @@ use crate::{ ModulePath, QualifiedFunctionName, discovery::DiscoveredPackage, extensions::{ - fixtures::{scope::fixture_scope, utils::handle_custom_fixture_params}, + fixtures::{ + python::InvalidFixtureError, scope::fixture_scope, utils::handle_custom_fixture_params, + }, tags::Parametrization, }, }; @@ -100,10 +102,10 @@ impl Fixture { py_module: &Bound<'_, PyModule>, module_path: &ModulePath, is_generator_function: bool, - ) -> Result { - let function = py_module - .getattr(stmt_function_def.name.to_string()) - .map_err(|e| e.to_string())?; + ) -> PyResult { + tracing::debug!("Trying to parse `{}` as a fixture", stmt_function_def.name); + + let function = py_module.getattr(stmt_function_def.name.to_string())?; let try_karva = Self::try_from_karva_function( py, @@ -115,7 +117,10 @@ impl Fixture { let try_karva_err = match try_karva { Ok(fixture) => return Ok(fixture), - Err(e) => Some(e), + Err(e) => { + tracing::debug!("Failed to create fixture from Karva function: {}", e); + Some(e) + } }; let try_pytest = Self::try_from_pytest_function( @@ -128,7 +133,10 @@ impl Fixture { match try_pytest { Ok(fixture) => Ok(fixture), - Err(e) => Err(try_karva_err.unwrap_or(e)), + Err(e) => { + tracing::debug!("Failed to create fixture from Pytest function: {}", e); + Err(try_karva_err.unwrap_or(e)) + } } } @@ -138,14 +146,17 @@ impl Fixture { function: &Bound<'_, PyAny>, module_name: ModulePath, is_generator_function: bool, - ) -> Result { - let found_name = get_attribute(function.clone(), &["_fixture_function_marker", "name"])?; + ) -> PyResult { + let fixture_function_marker = get_fixture_function_marker(function)?; + + let found_name = fixture_function_marker.getattr("name")?; - let scope = get_attribute(function.clone(), &["_fixture_function_marker", "scope"])?; + let scope = fixture_function_marker.getattr("scope")?; - let auto_use = get_attribute(function.clone(), &["_fixture_function_marker", "autouse"])?; + let auto_use = fixture_function_marker.getattr("autouse")?; - let params = get_attribute(function.clone(), &["_fixture_function_marker", "params"]) + let params = fixture_function_marker + .getattr("params") .ok() .and_then(|p| { if p.is_none() { @@ -155,7 +166,7 @@ impl Fixture { } }); - let function = get_attribute(function.clone(), &["_fixture_function"])?; + let fixture_function = get_fixture_function(function)?; let name = if found_name.is_none() { stmt_function_def.name.to_string() @@ -163,7 +174,8 @@ impl Fixture { found_name.to_string() }; - let fixture_scope = fixture_scope(py, &scope, &name)?; + let fixture_scope = + fixture_scope(py, &scope, &name).map_err(InvalidFixtureError::new_err)?; Ok(Self::new( py, @@ -171,7 +183,7 @@ impl Fixture { stmt_function_def.clone(), fixture_scope, auto_use.extract::().unwrap_or(false), - function.into(), + fixture_function.into(), is_generator_function, params, )) @@ -183,22 +195,20 @@ impl Fixture { function: &Bound<'_, PyAny>, module_path: ModulePath, is_generator_function: bool, - ) -> Result { + ) -> PyResult { let py_function = function .clone() - .cast_into::() - .map_err(|_| "Failed to parse fixture")?; + .cast_into::()?; - let py_function_borrow = py_function - .try_borrow_mut() - .map_err(|err| err.to_string())?; + let py_function_borrow = py_function.try_borrow_mut()?; let scope_obj = py_function_borrow.scope.clone(); let name = py_function_borrow.name.clone(); let auto_use = py_function_borrow.auto_use; let params = py_function_borrow.params.clone(); - let fixture_scope = fixture_scope(py, scope_obj.bind(py), &name)?; + let fixture_scope = + fixture_scope(py, scope_obj.bind(py), &name).map_err(InvalidFixtureError::new_err)?; Ok(Self::new( py, @@ -213,16 +223,38 @@ impl Fixture { } } -fn get_attribute<'a>( - function: Bound<'a, PyAny>, - attributes: &[&str], -) -> Result, String> { - let mut current = function; - for attribute in attributes { - let current_attr = current.getattr(attribute).map_err(|err| err.to_string())?; - current = current_attr; +/// Get the fixture function marker from a function. +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", + )) +} + +/// Get the fixture function from a function. +fn get_fixture_function<'py>(function: &Bound<'py, PyAny>) -> PyResult> { + if let Ok(attr) = function.getattr("_fixture_function") { + return Ok(attr); } - Ok(current.clone()) + + // Older versions of pytest + if let Ok(attr) = function.getattr("__pytest_wrapped__") { + if let Ok(attr) = attr.getattr("obj") { + return Ok(attr); + } + } + + Err(PyAttributeError::new_err( + "Could not find fixture information", + )) } impl std::fmt::Debug for Fixture { diff --git a/crates/karva_core/src/extensions/fixtures/python.rs b/crates/karva_core/src/extensions/fixtures/python.rs index db479e89..5e14bfe1 100644 --- a/crates/karva_core/src/extensions/fixtures/python.rs +++ b/crates/karva_core/src/extensions/fixtures/python.rs @@ -130,3 +130,6 @@ pub fn fixture_decorator( Ok(Py::new(py, marker)?.into_any()) } } + +// InvalidFixtureError exception that can be raised when a fixture is invalid +pyo3::create_exception!(karva, InvalidFixtureError, pyo3::exceptions::PyException); diff --git a/crates/karva_core/src/python.rs b/crates/karva_core/src/python.rs index 7e4b371e..d8ad586e 100644 --- a/crates/karva_core/src/python.rs +++ b/crates/karva_core/src/python.rs @@ -4,7 +4,8 @@ use crate::extensions::{ fixtures::{ Mock, python::{ - FixtureFunctionDefinition, FixtureFunctionMarker, FixtureRequest, fixture_decorator, + FixtureFunctionDefinition, FixtureFunctionMarker, FixtureRequest, InvalidFixtureError, + fixture_decorator, }, }, functions::{FailError, SkipError, fail, param, skip}, @@ -28,5 +29,6 @@ pub fn init_module(py: Python<'_>, m: &Bound<'_, PyModule>) -> PyResult<()> { m.add("SkipError", py.get_type::())?; m.add("FailError", py.get_type::())?; + m.add("InvalidFixtureError", py.get_type::())?; Ok(()) } From 8ff28aa9de6101f712d18a527df8e53da7db5091 Mon Sep 17 00:00:00 2001 From: Matthew Mckee Date: Wed, 3 Dec 2025 00:53:02 +0000 Subject: [PATCH 3/4] Add invalid fixture error to stubs --- python/karva/_karva/__init__.pyi | 3 +++ 1 file changed, 3 insertions(+) diff --git a/python/karva/_karva/__init__.pyi b/python/karva/_karva/__init__.pyi index 6a3e1dee..2457ce21 100644 --- a/python/karva/_karva/__init__.pyi +++ b/python/karva/_karva/__init__.pyi @@ -82,6 +82,9 @@ class SkipError(Exception): class FailError(Exception): """Raised when `karva.fail` is called.""" +class InvalidFixtureError(Exception): + """Raised when an invalid fixture is encountered.""" + class Mock: """Helper to conveniently monkeypatch attributes/items/environment variables/syspath. From 51657805c0a5a7d878d60dbdc3218ea054cc4304 Mon Sep 17 00:00:00 2001 From: Matthew Mckee Date: Wed, 3 Dec 2025 00:59:06 +0000 Subject: [PATCH 4/4] Fix tests --- crates/karva_cli/tests/cli.rs | 10 +++++----- crates/karva_core/src/diagnostic/mod.rs | 4 ++-- .../tests/integration/extensions/fixtures/invalid.rs | 8 ++++---- .../tests/integration/extensions/functions.rs | 8 ++++---- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/crates/karva_cli/tests/cli.rs b/crates/karva_cli/tests/cli.rs index 8438236b..4d7abff2 100644 --- a/crates/karva_cli/tests/cli.rs +++ b/crates/karva_cli/tests/cli.rs @@ -209,7 +209,7 @@ fn test_two_test_fails() { 6 | assert False, 'Test failed' | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - info: Error message: Test failed + info: Test failed test result: FAILED. 0 passed; 2 failed; 0 skipped; finished in [TIME] @@ -267,7 +267,7 @@ fn test_file_importing_another_file() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 5 | return True | - info: Error message: Data validation failed + info: Data validation failed test result: FAILED. 0 passed; 1 failed; 0 skipped; finished in [TIME] @@ -933,7 +933,7 @@ fn test_failfast() { 4 | 5 | def test_second(): | - info: Error message: First test fails + info: First test fails test result: FAILED. 0 passed; 1 failed; 0 skipped; finished in [TIME] @@ -1271,7 +1271,7 @@ def test_normal(): 6 | 7 | def test_normal(): | - info: Error message: This is a custom failure message + info: This is a custom failure message test result: FAILED. 1 passed; 1 failed; 0 skipped; finished in [TIME] @@ -1348,7 +1348,7 @@ def test_1(): 6 | 7 | def test_1(): | - info: Error message: bar + info: bar test result: FAILED. 0 passed; 1 failed; 0 skipped; finished in [TIME] diff --git a/crates/karva_core/src/diagnostic/mod.rs b/crates/karva_core/src/diagnostic/mod.rs index 7d7f5d1a..787347e6 100644 --- a/crates/karva_core/src/diagnostic/mod.rs +++ b/crates/karva_core/src/diagnostic/mod.rs @@ -149,7 +149,7 @@ pub fn report_invalid_fixture( let error_string = error.value(py).to_string(); if !error_string.is_empty() { - diagnostic.info(format!("Error message: {error_string}")); + diagnostic.info(error_string); } } @@ -319,6 +319,6 @@ fn handle_failed_function_call( let error_string = error.value(py).to_string(); if !error_string.is_empty() { - diagnostic.info(format!("Error message: {error_string}")); + diagnostic.info(error_string); } } diff --git a/crates/karva_core/tests/integration/extensions/fixtures/invalid.rs b/crates/karva_core/tests/integration/extensions/fixtures/invalid.rs index 64814da3..d8d46eb5 100644 --- a/crates/karva_core/tests/integration/extensions/fixtures/invalid.rs +++ b/crates/karva_core/tests/integration/extensions/fixtures/invalid.rs @@ -34,7 +34,7 @@ fn test_invalid_pytest_fixture_scope() { | ^^^^^^^^^^^^ 6 | return 1 | - info: Failed to parse fixture + info: 'FixtureFunctionDefinition' object cannot be cast as 'FixtureFunctionDefinition' error[missing-fixtures]: Test `test_all_scopes` has missing fixtures --> /test.py:8:5 @@ -126,7 +126,7 @@ fn test_fixture_fails_to_run() { 7 | 8 | def test_failing_fixture(failing_fixture): | - info: Error message: Fixture failed + info: Fixture failed error[missing-fixtures]: Test `test_failing_fixture` has missing fixtures --> /test.py:8:5 @@ -223,7 +223,7 @@ fn missing_arguments_in_nested_function() { 6 | inner() | ^^^^^^^ | - info: Error message: test_failing_fixture..inner() missing 1 required positional argument: 'missing_fixture' + info: test_failing_fixture..inner() missing 1 required positional argument: 'missing_fixture' test result: FAILED. 0 passed; 1 failed; 0 skipped; finished in [TIME] "); @@ -270,7 +270,7 @@ fn test_failing_yield_fixture() { | ^^^^^^^^^^^^^^^^^^^^^^^ 8 | yield foo() | - info: Error message: foo + info: foo error[missing-fixtures]: Test `test_failing_fixture` has missing fixtures --> /test.py:10:5 diff --git a/crates/karva_core/tests/integration/extensions/functions.rs b/crates/karva_core/tests/integration/extensions/functions.rs index 783539b1..9b7efef4 100644 --- a/crates/karva_core/tests/integration/extensions/functions.rs +++ b/crates/karva_core/tests/integration/extensions/functions.rs @@ -46,7 +46,7 @@ def test_with_fail_with_keyword_reason(): 6 | 7 | def test_with_fail_with_no_reason(): | - info: Error message: This is a custom failure message + info: This is a custom failure message error[test-failure]: Test `test_with_fail_with_no_reason` failed --> /test.py:7:5 @@ -83,7 +83,7 @@ def test_with_fail_with_keyword_reason(): 11 | karva.fail(reason='This is a custom failure message') | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - info: Error message: This is a custom failure message + info: This is a custom failure message test result: FAILED. 0 passed; 3 failed; 0 skipped; finished in [TIME] "); @@ -128,7 +128,7 @@ def test_conditional_fail(): | ^^^^^^^^^^^^^^^^^^^^^^^^^^ 8 | assert True | - info: Error message: failing test + info: failing test test result: FAILED. 0 passed; 1 failed; 0 skipped; finished in [TIME] "); @@ -167,7 +167,7 @@ def test_raise_fail_error(): 5 | raise karva.FailError('Manually raised FailError') | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - info: Error message: Manually raised FailError + info: Manually raised FailError test result: FAILED. 0 passed; 1 failed; 0 skipped; finished in [TIME] ");