Re-raise internal errors instead of swallowing#173
Draft
Liam-DeVoe wants to merge 13 commits intomainfrom
Draft
Conversation
When hegel-internal code panics during test execution (e.g., invalid
generator config, server protocol error), the panic was caught by
catch_unwind, treated as a test failure, and either lost entirely
("Property test failed: unknown") or unnecessarily shrunk.
Now we check the panic location against CARGO_MANIFEST_DIR to detect
hegel-internal panics and propagate them immediately with the original
error message and backtrace, following Hypothesis's pattern.
Three fixes for CI failures: 1. Make thread ID optional in panic output regex: `(?: \(\d+\))?` Rust 1.86 doesn't include thread IDs in panic output, but nightly does. The regex now handles both. 2. Bump regex dev-dependency from "1.0" to "1.5" to avoid bugs in regex 1.0.0 with complex (?s) patterns in the minimal-versions CI job. 3. Add in-process test (test_internal_error_message) that exercises the internal error code path directly, providing coverage for the new InternalError handling in runner.rs. The subprocess-based tests (TempRustProject) don't contribute to coverage instrumentation.
Previously, is_hegel_file would treat any file existing under CARGO_MANIFEST_DIR as hegel-internal, including test files. Paths like tests/test_find_quality/../common/utils.rs resolved to tests/common/utils.rs which exists in the crate, causing test panics (e.g. HEGEL_MINIMAL_FOUND) to be re-raised as internal errors instead of treated as test failures. Now the function normalizes ".." components and checks the path starts with src/ or hegel-macros/src/, so only actual library source files are considered internal.
- Add codepoints field to text conformance metrics (required by updated TextConformance validator) - Add test for CurDir path component in is_hegel_file (coverage gap) - Fix backtrace regex to handle generic params after ::draw on nightly
Kept main's version which removed the redundant `length` field from Metrics, since it's derivable from codepoints.len().
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Claude-written description
Previously, when hegel-internal code panicked during test execution (e.g. invalid generator config like
integers().min_value(100).max_value(10), or an unexpected server error response), the panic was caught bycatch_unwind, treated as a test failure, and reported as "Property test failed: unknown" — completely losing the actual error. This follows the same pattern as Hypothesis's_execute_once_for_engine, which re-raises exceptions that originate from Hypothesis's own source files.Now we check the panic location's file path against
CARGO_MANIFEST_DIRto detect hegel-internal panics and propagate them immediately on the first test case — no shrinking, no wrapping. The error message includes the original source location and backtrace so users can trace where things went wrong.Fixes #47.