kornia/kornia-rs

[Bug]: Test Infrastructure Fragility: data-driven AprilTag tests have silent failures and panic-on-error paths

Open

#732 opened on Feb 22, 2026

View on GitHub
 (11 comments) (0 reactions) (1 assignee)Rust (188 forks)auto 404
bughelp wantedtriage

Repository metrics

Stars
 (675 stars)
PR merge metrics
 (PR metrics pending)

Description

🐛 Describe the bug

The data-driven test infrastructure in crates/kornia-apriltag/tests/data_driven.rs has multiple fragility points that can cause silent failures or misleading test results:

  1. partial_cmp().unwrap() panics on NaN (line 170) — If the detector produces NaN coordinates, f32::partial_cmp returns None and .unwrap() crashes with an unhelpful "called unwrap() on a None value" instead of reporting the actual geometric issue.

  2. expect() panics instead of returning Err (line 225) — The run_data_driven_tests function returns Result<()>, but the Edge test arm uses .expect("Edge test must have min_detections") which panics instead of propagating an error through ?. A manifest authoring mistake produces a panic, not a diagnostic.

  3. Golden tests silently pass with zero validation — If a Golden test case has neither expect_ids nor expect_detections, both if let Some(...) branches are skipped. The detector runs, but nothing is asserted. The test reports success while validating nothing.

  4. No finite-value validation on geometric coordinates — NaN or Infinity in manifest coordinates or detector output makes assert!((a - b).abs() <= tol) comparisons meaningless (NaN comparisons always return false, but the assertion message is misleading).

🔄 Steps to Reproduce

1. Clone the repo and check out main

2. Add a Golden test case to crates/kornia-apriltag/tests/data/manifest.json with no expect_ids and no expect_detections — it silently passes

3. Add an Edge test case without min_detections — it panics instead of returning an error

4. Inspect line 170: da.partial_cmp(&db).unwrap() — any NaN from the detector causes an opaque panic

💻 Minimal Code Example

// Fragility 1: Panic on NaN (line 170)
let da: f32 = f32::NAN;
let db: f32 = 1.0;
da.partial_cmp(&db).unwrap(); // panics: "called `Option::unwrap()` on a `None` value"
// Fix: da.total_cmp(&db)  — total ordering, no panic, already used in quad.rs

// Fragility 2: Panic instead of Err (line 225)
let min_detections: Option<usize> = None;
min_detections.expect("Edge test must have min_detections"); // panics
// Fix: min_detections.ok_or_else(|| format!(...))?

// Fragility 3: Silent pass — this Golden arm does nothing
let expect_ids: Option<Vec<usize>> = None;
let expect_detections: Option<Vec<ExpectedDetection>> = None;
if let Some(ref ids) = expect_ids { /* skipped */ }
if let Some(ref dets) = expect_detections { /* skipped */ }
// Test passes having validated zero assertions

✅ Expected behavior

~ All test code paths either validate something meaningful or return a clear error ~ Manifest authoring mistakes are caught with descriptive error messages, not panics ~ NaN/Infinity in coordinates is detected and reported, not silently swallowed ~ A Golden test that validates nothing is treated as a configuration error

❌ Actual behavior

~ .unwrap() on partial_cmp panics with an opaque message when NaN is encountered ~ .expect() on min_detections panics instead of returning Err through the Result return type ~ Golden tests with no expectations silently pass — the detector runs but nothing is asserted ~ Non-finite coordinates produce meaningless comparison results with misleading assertion messages

🔧 Environment

- kornia-rs version: 0.1.11-rc.1
- Rust version (`rustc -V`): 1.92.0
- Cargo version (`cargo -V`): 1.92.0
- OS (e.g., Linux, macOS, Windows): macOS
- Target architecture (if cross-compiling): aarch64
- Python version (if using Python bindings):  N/A

📝 Additional context

~ The crate already uses f32::total_cmp() in quad.rs:259 and quad.rs:534 — the fix for fragility 1 is just aligning the test with existing crate patterns.

~ The ok_or_else + format! pattern is already used at line 173 of the same test file — fix 2 is consistent with existing code.

~ is_finite() is used throughout kornia-algebra, kornia-imgproc, and kornia-3d

All fixes are confined to a single file: crates/kornia-apriltag/tests/data_driven.rs

🤝 Contribution Intent

  • I plan to submit a PR to fix this bug
  • I'm reporting this bug but not planning to fix it

Contributor guide