WS7 - Test coverage & Tooling Enforcement

Author: tzanko-matev

Justfile

@@ -42,6 +42,14 @@ cargo-test:
 
 py-test:
     uv run --group dev --group test pytest codetracer-python-recorder/tests/python codetracer-pure-python-recorder
+
+lint: lint-rust lint-errors
+
+lint-rust:
+    uv run cargo clippy --manifest-path codetracer-python-recorder/Cargo.toml --workspace --no-default-features -- -D clippy::panic
+
+lint-errors:
+    uv run python3 codetracer-python-recorder/scripts/lint_no_unwraps.py
 
 # Run tests only on the pure recorder
 test-pure:

codetracer-python-recorder/crates/recorder-errors/src/lib.rs

@@ -621,6 +621,26 @@ mod tests {
         assert_eq!(err.code, ErrorCode::FrameIntrospectionFailed);
     }
 
+    #[test]
+    fn target_macro_marks_target_error() {
+        let err = target!(ErrorCode::TraceIncomplete, "target callback failed");
+        assert_eq!(err.kind, ErrorKind::Target);
+        assert_eq!(err.code, ErrorCode::TraceIncomplete);
+    }
+
+    #[test]
+    fn ensure_internal_marks_internal_failures() {
+        fn guarded(assert_ok: bool) -> RecorderResult<()> {
+            ensure_internal!(assert_ok, ErrorCode::TraceIncomplete, "invariant broken");
+            Ok(())
+        }
+
+        let err = guarded(false).expect_err("expected invariant failure");
+        assert_eq!(err.kind, ErrorKind::Internal);
+        assert_eq!(err.code, ErrorCode::TraceIncomplete);
+        guarded(true).expect("guarded success");
+    }
+
     #[test]
     fn parse_roundtrip_matches_known_codes() {
         for code in [

codetracer-python-recorder/scripts/lint_no_unwraps.py

@@ -0,0 +1,61 @@
+#!/usr/bin/env python3
+"""Fail the build when unchecked `.unwrap(` usage appears outside the allowlist."""
+from __future__ import annotations
+
+import argparse
+import pathlib
+import sys
+from typing import Iterable
+
+DEFAULT_ALLOWLIST = {
+    pathlib.Path("codetracer-python-recorder/src/runtime/value_encoder.rs"),
+    pathlib.Path("codetracer-python-recorder/src/runtime/mod.rs"),
+    pathlib.Path("codetracer-python-recorder/src/monitoring/mod.rs"),
+    pathlib.Path("codetracer-python-recorder/src/monitoring/tracer.rs"),
+}
+
+
+def scan_for_unsafe_unwraps(root: pathlib.Path, allowlist: Iterable[pathlib.Path]) -> list[pathlib.Path]:
+    repo_root = root.resolve()
+    src_root = repo_root / "codetracer-python-recorder" / "src"
+    allowed = {path.resolve() for path in (repo_root / entry for entry in allowlist)}
+
+    failures: list[pathlib.Path] = []
+    for candidate in src_root.rglob("*.rs"):
+        if candidate in allowed:
+            continue
+        text = candidate.read_text()
+        if ".unwrap(" in text:
+            failures.append(candidate.relative_to(repo_root))
+    return failures
+
+
+def main() -> int:
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument(
+        "--repo-root",
+        type=pathlib.Path,
+        default=pathlib.Path(__file__).resolve().parents[2],
+        help="Path to the repository root (auto-detected by default)",
+    )
+    parser.add_argument(
+        "--allow",
+        action="append",
+        dest="allow",
+        default=[str(path) for path in DEFAULT_ALLOWLIST],
+        help="Additional relative paths that may contain unwrap usage",
+    )
+    args = parser.parse_args()
+
+    allowlist = [pathlib.Path(entry) for entry in args.allow]
+    failures = scan_for_unsafe_unwraps(args.repo_root, allowlist)
+    if failures:
+        print("Found disallowed `.unwrap(` usage in the recorder crate:", file=sys.stderr)
+        for failure in sorted(failures):
+            print(f" - {failure}", file=sys.stderr)
+        return 1
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())

codetracer-python-recorder/src/ffi.rs

@@ -136,3 +136,105 @@ where
         Err(panic_payload) => Err(handle_panic(label, panic_payload)),
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use recorder_errors::{enverr, target, usage};
+
+    #[test]
+    fn map_recorder_error_sets_python_attributes() {
+        Python::with_gil(|py| {
+            let err = usage!(
+                ErrorCode::UnsupportedFormat,
+                "invalid trace format"
+            )
+            .with_context("format", "yaml")
+            .with_source(std::io::Error::new(std::io::ErrorKind::Other, "boom"));
+            let pyerr = map_recorder_error(err);
+            let ty = pyerr.get_type(py);
+            assert!(ty.is(py.get_type::<PyUsageError>()));
+            let value = pyerr.value(py);
+            assert_eq!(
+                value
+                    .getattr("code")
+                    .expect("error code attribute")
+                    .extract::<String>()
+                    .expect("code string"),
+                "ERR_UNSUPPORTED_FORMAT"
+            );
+            assert_eq!(
+                value
+                    .getattr("kind")
+                    .expect("error kind attribute")
+                    .extract::<String>()
+                    .expect("kind string"),
+                "Usage"
+            );
+            let context_obj = value.getattr("context").expect("context attribute");
+            let ctx = context_obj
+                .downcast::<PyDict>()
+                .expect("context attribute downcast");
+            let format_value = ctx
+                .get_item("format")
+                .expect("context lookup failed")
+                .expect("context map missing 'format'");
+            assert_eq!(
+                format_value
+                    .extract::<String>()
+                    .expect("format value extraction"),
+                "yaml"
+            );
+        });
+    }
+
+    #[test]
+    fn dispatch_converts_recorder_error_to_pyerr() {
+        Python::with_gil(|py| {
+            let result: PyResult<()> = dispatch("dispatch_env", || {
+                Err(enverr!(ErrorCode::Io, "disk full"))
+            });
+            let err = result.expect_err("expected PyErr");
+            let ty = err.get_type(py);
+            assert!(ty.is(py.get_type::<PyEnvironmentError>()));
+        });
+    }
+
+    #[test]
+    fn dispatch_converts_panic_into_internal_error() {
+        Python::with_gil(|py| {
+            let result: PyResult<()> = dispatch("dispatch_panic", || panic!("boom"));
+            let err = result.expect_err("expected panic to map into PyErr");
+            let ty = err.get_type(py);
+            assert!(ty.is(py.get_type::<PyInternalError>()));
+            assert!(err.to_string().contains("panic in dispatch_panic"));
+        });
+    }
+
+    #[test]
+    fn wrap_pyfunction_passes_through_success() {
+        let result = wrap_pyfunction("wrap_ok", || Ok::<_, PyErr>(42));
+        assert_eq!(result.expect("expected success"), 42);
+    }
+
+    #[test]
+    fn wrap_pyfunction_converts_errors_and_panics() {
+        Python::with_gil(|py| {
+            let err = wrap_pyfunction("wrap_error", || -> PyResult<()> {
+                Err(map_recorder_error(target!(
+                    ErrorCode::TraceIncomplete,
+                    "target failure"
+                )))
+            })
+            .expect_err("expected error");
+            assert!(err.get_type(py).is(py.get_type::<PyTargetError>()));
+
+            let panic_err = wrap_pyfunction("wrap_panic", || -> PyResult<()> {
+                panic!("boom");
+            })
+            .expect_err("expected panic");
+            assert!(panic_err.get_type(py).is(py.get_type::<PyInternalError>()));
+            assert!(panic_err.to_string().contains("panic in wrap_panic"));
+        });
+    }
+}

codetracer-python-recorder/src/policy.rs

@@ -351,6 +351,19 @@ mod tests {
         reset_policy();
     }
 
+    #[test]
+    fn configure_policy_from_env_rejects_invalid_boolean() {
+        reset_policy();
+        let env_guard = env_lock();
+        env::set_var(ENV_REQUIRE_TRACE, "sometimes");
+
+        let err = configure_policy_from_env().expect_err("invalid bool should error");
+        assert_eq!(err.code, ErrorCode::InvalidPolicyValue);
+
+        drop(env_guard);
+        reset_policy();
+    }
+
     fn env_lock() -> EnvGuard {
         EnvGuard
     }

codetracer-python-recorder/src/runtime/mod.rs

@@ -25,7 +25,7 @@ use std::sync::OnceLock;
 use pyo3::prelude::*;
 use pyo3::types::PyAny;
 
-use recorder_errors::{bug, enverr, usage, ErrorCode, RecorderResult};
+use recorder_errors::{bug, enverr, target, usage, ErrorCode, RecorderResult};
 use runtime_tracing::NonStreamingTraceWriter;
 use runtime_tracing::{Line, TraceEventsFileFormat, TraceWriter};
 
@@ -97,6 +97,8 @@ impl FailureStage {
 enum FailureMode {
     Stage(FailureStage),
     SuppressEvents,
+    TargetArgs,
+    Panic,
 }
 
 #[cfg(feature = "integration-test")]
@@ -116,26 +118,46 @@ fn configured_failure_mode() -> Option<FailureMode> {
             "line" => Some(FailureMode::Stage(FailureStage::Line)),
             "finish" => Some(FailureMode::Stage(FailureStage::Finish)),
             "suppress-events" | "suppress_events" | "suppress" => Some(FailureMode::SuppressEvents),
+            "target" | "target-args" | "target_args" => Some(FailureMode::TargetArgs),
+            "panic" | "panic-callback" | "panic_callback" => Some(FailureMode::Panic),
             _ => None,
         })
     })
 }
 
 #[cfg(feature = "integration-test")]
 fn should_inject_failure(stage: FailureStage) -> bool {
-    match configured_failure_mode() {
-        Some(FailureMode::Stage(mode)) if mode == stage => {
-            !FAILURE_TRIGGERED.swap(true, Ordering::SeqCst)
-        }
-        _ => false,
-    }
+    matches!(configured_failure_mode(), Some(FailureMode::Stage(mode)) if mode == stage)
+        && mark_failure_triggered()
 }
 
 #[cfg(not(feature = "integration-test"))]
 fn should_inject_failure(_stage: FailureStage) -> bool {
     false
 }
 
+#[cfg(feature = "integration-test")]
+fn should_inject_target_error() -> bool {
+    matches!(configured_failure_mode(), Some(FailureMode::TargetArgs))
+        && mark_failure_triggered()
+}
+
+#[cfg(not(feature = "integration-test"))]
+fn should_inject_target_error() -> bool {
+    false
+}
+
+#[cfg(feature = "integration-test")]
+fn should_panic_in_callback() -> bool {
+    matches!(configured_failure_mode(), Some(FailureMode::Panic)) && mark_failure_triggered()
+}
+
+#[cfg(not(feature = "integration-test"))]
+#[allow(dead_code)]
+fn should_panic_in_callback() -> bool {
+    false
+}
+
 #[cfg(feature = "integration-test")]
 fn suppress_events() -> bool {
     matches!(configured_failure_mode(), Some(FailureMode::SuppressEvents))
@@ -146,6 +168,17 @@ fn suppress_events() -> bool {
     false
 }
 
+#[cfg(feature = "integration-test")]
+fn mark_failure_triggered() -> bool {
+    !FAILURE_TRIGGERED.swap(true, Ordering::SeqCst)
+}
+
+#[cfg(not(feature = "integration-test"))]
+#[allow(dead_code)]
+fn mark_failure_triggered() -> bool {
+    false
+}
+
 #[cfg(feature = "integration-test")]
 fn injected_failure_err(stage: FailureStage) -> PyErr {
     let err = bug!(
@@ -340,6 +373,16 @@ impl Tracer for RuntimeTracer {
             return Err(injected_failure_err(FailureStage::PyStart));
         }
 
+        if should_inject_target_error() {
+            return Err(ffi::map_recorder_error(
+                target!(
+                    ErrorCode::TraceIncomplete,
+                    "test-injected target error from capture_call_arguments"
+                )
+                .with_context("injection_stage", "capture_call_arguments"),
+            ));
+        }
+
         log_event(py, code, "on_py_start", None);
 
         if let Ok(fid) = self.ensure_function_id(py, code) {
@@ -381,6 +424,13 @@ impl Tracer for RuntimeTracer {
             return Err(injected_failure_err(FailureStage::Line));
         }
 
+        #[cfg(feature = "integration-test")]
+        {
+            if should_panic_in_callback() {
+                panic!("test-injected panic in on_line");
+            }
+        }
+
         log_event(py, code, "on_line", Some(lineno));
 
         if let Ok(filename) = code.filename(py) {