Improve parsing of fixtures

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]
 

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,
@@ -112,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);
 
@@ -127,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(error_string);
+    }
 }
 
 pub fn report_invalid_fixture_finalizer(
@@ -296,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);
     }
 }

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

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;
 

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,

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<Self, String> {
-        let function = py_module
-            .getattr(stmt_function_def.name.to_string())
-            .map_err(|e| e.to_string())?;
+    ) -> PyResult<Self> {
+        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<Self, String> {
-        let found_name = get_attribute(function.clone(), &["_fixture_function_marker", "name"])?;
+    ) -> PyResult<Self> {
+        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,23 +166,24 @@ 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()
         } else {
             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,
             QualifiedFunctionName::new(name, module_name),
             stmt_function_def.clone(),
             fixture_scope,
             auto_use.extract::<bool>().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<Self, String> {
+    ) -> PyResult<Self> {
         let py_function = function
             .clone()
-            .cast_into::<python::FixtureFunctionDefinition>()
-            .map_err(|_| "Failed to parse fixture")?;
+            .cast_into::<python::FixtureFunctionDefinition>()?;
 
-        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<Bound<'a, PyAny>, 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<Bound<'py, PyAny>> {
+    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<Bound<'py, PyAny>> {
+    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 {

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);

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::<SkipError>())?;
     m.add("FailError", py.get_type::<FailError>())?;
+    m.add("InvalidFixtureError", py.get_type::<InvalidFixtureError>())?;
     Ok(())
 }