Skip to content

Commit 55f0327

Browse files
DRMacIverclaude
andcommitted
Refactor for testability: extract functions and deduplicate panic_message
- Move panic_message to control.rs, remove duplicates from runner.rs and stateful.rs - Extract filter_backtrace from format_backtrace in runner.rs - Extract install_hegel_server from ensure_hegel_installed in runner.rs - Extract handle_antithesis_reporting from Hegel::run() in runner.rs - Add RequestErrorKind enum and classify_request_error to test_case.rs - Change PROTOCOL_DEBUG from LazyLock static to plain function - Add ENV_TEST_MUTEX to lib.rs for test serialization - Add lcov.info to .gitignore Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
1 parent 93e8298 commit 55f0327

File tree

7 files changed

+110
-70
lines changed

7 files changed

+110
-70
lines changed

.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ result
1515
.claude-reliability/
1616
.claude/reliability-config.yml
1717

18+
lcov.info
19+
1820
# claude-reliability managed
1921
.claude/bin/
2022
.claude/*.local.md

RELEASE.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
RELEASE_TYPE: patch
2+
3+
Internal refactoring for testability. No user-visible changes.

src/control.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,17 @@ pub(crate) fn with_test_context<R>(f: impl FnOnce() -> R) -> R {
1212
result
1313
}
1414

15+
/// Extract a message from a panic payload.
16+
pub(crate) fn panic_message(payload: &Box<dyn std::any::Any + Send>) -> String {
17+
if let Some(s) = payload.downcast_ref::<&str>() {
18+
s.to_string()
19+
} else if let Some(s) = payload.downcast_ref::<String>() {
20+
s.clone()
21+
} else {
22+
"Unknown panic".to_string()
23+
}
24+
}
25+
1526
/// Returns `true` if we are currently inside a Hegel test context.
1627
///
1728
/// This can be used to conditionally execute code that depends on a

src/lib.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,3 +221,8 @@ pub use hegel_macros::test;
221221
#[doc(hidden)]
222222
pub use runner::hegel;
223223
pub use runner::{HealthCheck, Hegel, Settings, Verbosity};
224+
225+
/// Mutex for serializing tests that modify environment variables.
226+
/// Used across runner, antithesis, and integration test modules to prevent races.
227+
#[doc(hidden)]
228+
pub static ENV_TEST_MUTEX: std::sync::Mutex<()> = std::sync::Mutex::new(());

src/runner.rs

Lines changed: 37 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use crate::antithesis::{TestLocation, is_running_in_antithesis};
2-
use crate::control::{currently_in_test_context, with_test_context};
2+
use crate::control::{currently_in_test_context, panic_message, with_test_context};
33
use crate::protocol::{Channel, Connection, HANDSHAKE_STRING, SERVER_CRASHED_MESSAGE};
44
use crate::test_case::{ASSUME_FAIL_STRING, STOP_TEST_STRING, TestCase};
55
use ciborium::Value;
@@ -145,11 +145,17 @@ fn take_panic_info() -> Option<(String, String, String, Backtrace)> {
145145
/// Frame numbers are renumbered to start at 0.
146146
fn format_backtrace(bt: &Backtrace, full: bool) -> String {
147147
let backtrace_str = format!("{}", bt);
148-
149148
if full {
150149
return backtrace_str;
151150
}
151+
filter_backtrace(&backtrace_str)
152+
}
152153

154+
/// Filter a backtrace string to "short" format.
155+
///
156+
/// Keeps only frames between `__rust_end_short_backtrace` and
157+
/// `__rust_begin_short_backtrace` markers, renumbering from 0.
158+
fn filter_backtrace(backtrace_str: &str) -> String {
153159
// Filter to short backtrace: keep lines between the markers
154160
// Frame groups look like:
155161
// N: function::name
@@ -266,20 +272,27 @@ fn init_panic_hook() {
266272
}
267273

268274
fn ensure_hegel_installed() -> Result<String, String> {
269-
let venv_dir = format!("{HEGEL_SERVER_DIR}/venv");
275+
install_hegel_server(HEGEL_SERVER_DIR, HEGEL_SERVER_VERSION)
276+
}
277+
278+
/// Install the hegel server into a directory using `uv`.
279+
///
280+
/// Returns the path to the installed `hegel` binary on success.
281+
fn install_hegel_server(server_dir: &str, version: &str) -> Result<String, String> {
282+
let venv_dir = format!("{server_dir}/venv");
270283
let version_file = format!("{venv_dir}/hegel-version");
271284
let hegel_bin = format!("{venv_dir}/bin/hegel");
272-
let install_log = format!("{HEGEL_SERVER_DIR}/install.log");
285+
let install_log = format!("{server_dir}/install.log");
273286

274287
// Check cached version
275288
if let Ok(cached) = std::fs::read_to_string(&version_file) {
276-
if cached.trim() == HEGEL_SERVER_VERSION && std::path::Path::new(&hegel_bin).is_file() {
289+
if cached.trim() == version && std::path::Path::new(&hegel_bin).is_file() {
277290
return Ok(hegel_bin);
278291
}
279292
}
280293

281-
std::fs::create_dir_all(HEGEL_SERVER_DIR)
282-
.map_err(|e| format!("Failed to create {HEGEL_SERVER_DIR}: {e}"))?;
294+
std::fs::create_dir_all(server_dir)
295+
.map_err(|e| format!("Failed to create {server_dir}: {e}"))?;
283296

284297
let log_file = std::fs::File::create(&install_log)
285298
.map_err(|e| format!("Failed to create install log: {e}"))?;
@@ -310,7 +323,7 @@ fn ensure_hegel_installed() -> Result<String, String> {
310323
"install",
311324
"--python",
312325
&python_path,
313-
&format!("hegel-core=={HEGEL_SERVER_VERSION}"),
326+
&format!("hegel-core=={version}"),
314327
])
315328
.stderr(log_file.try_clone().unwrap())
316329
.stdout(log_file)
@@ -319,7 +332,7 @@ fn ensure_hegel_installed() -> Result<String, String> {
319332
if !status.success() {
320333
let log = std::fs::read_to_string(&install_log).unwrap_or_default();
321334
return Err(format!(
322-
"Failed to install hegel-core (version: {HEGEL_SERVER_VERSION}). \
335+
"Failed to install hegel-core (version: {version}). \
323336
Set {HEGEL_SERVER_COMMAND_ENV} to a hegel binary path to skip installation.\n\
324337
Install log:\n{log}"
325338
));
@@ -329,7 +342,7 @@ fn ensure_hegel_installed() -> Result<String, String> {
329342
return Err(format!("hegel not found at {hegel_bin} after installation"));
330343
}
331344

332-
std::fs::write(&version_file, HEGEL_SERVER_VERSION)
345+
std::fs::write(&version_file, version)
333346
.map_err(|e| format!("Failed to write version file: {e}"))?;
334347

335348
Ok(hegel_bin)
@@ -793,18 +806,7 @@ where
793806

794807
let test_failed = !passed || got_interesting.load(Ordering::SeqCst);
795808

796-
if is_running_in_antithesis() {
797-
#[cfg(not(feature = "antithesis"))]
798-
panic!(
799-
"When Hegel is run inside of Antithesis, it requires the `antithesis` feature. \
800-
You can add it with {{ features = [\"antithesis\"] }}."
801-
);
802-
803-
#[cfg(feature = "antithesis")]
804-
if let Some(ref loc) = self.test_location {
805-
crate::antithesis::emit_assertion(loc, !test_failed);
806-
}
807-
}
809+
handle_antithesis_reporting(self.test_location.as_ref(), test_failed);
808810

809811
if test_failed {
810812
let msg = match &final_result {
@@ -909,14 +911,19 @@ fn run_test_case<F: FnMut(TestCase)>(
909911
tc_result
910912
}
911913

912-
/// Extract a message from a panic payload.
913-
fn panic_message(payload: &Box<dyn std::any::Any + Send>) -> String {
914-
if let Some(s) = payload.downcast_ref::<&str>() {
915-
s.to_string()
916-
} else if let Some(s) = payload.downcast_ref::<String>() {
917-
s.clone()
918-
} else {
919-
"Unknown panic".to_string()
914+
/// Report test results to Antithesis if running in that environment.
915+
fn handle_antithesis_reporting(test_location: Option<&TestLocation>, test_failed: bool) {
916+
if is_running_in_antithesis() {
917+
#[cfg(not(feature = "antithesis"))]
918+
panic!(
919+
"When Hegel is run inside of Antithesis, it requires the `antithesis` feature. \
920+
You can add it with {{ features = [\"antithesis\"] }}."
921+
);
922+
923+
#[cfg(feature = "antithesis")]
924+
if let Some(loc) = test_location {
925+
crate::antithesis::emit_assertion(loc, !test_failed);
926+
}
920927
}
921928
}
922929

src/stateful.rs

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@
6262
6363
use crate::TestCase;
6464
use crate::cbor_utils::cbor_map;
65+
use crate::control::panic_message;
6566
use crate::generators::integers;
6667
use crate::test_case::{ASSUME_FAIL_STRING, STOP_TEST_STRING};
6768
use ciborium::Value;
@@ -179,17 +180,6 @@ pub trait StateMachine {
179180
fn invariants(&self) -> Vec<Rule<Self>>;
180181
}
181182

182-
// TODO: factor out (shared with runner.rs)
183-
fn panic_message(payload: &Box<dyn std::any::Any + Send>) -> String {
184-
if let Some(s) = payload.downcast_ref::<&str>() {
185-
s.to_string()
186-
} else if let Some(s) = payload.downcast_ref::<String>() {
187-
s.clone()
188-
} else {
189-
"Unknown panic".to_string()
190-
}
191-
}
192-
193183
fn check_invariants(m: &mut impl StateMachine, tc: &TestCase) {
194184
let invariants = m.invariants();
195185
for invariant in invariants {

src/test_case.rs

Lines changed: 51 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use crate::runner::Verbosity;
55
use ciborium::Value;
66
use std::cell::RefCell;
77
use std::rc::Rc;
8-
use std::sync::{Arc, LazyLock};
8+
use std::sync::Arc;
99

1010
use crate::generators::value;
1111

@@ -35,15 +35,38 @@ impl std::fmt::Display for StopTestError {
3535
}
3636
impl std::error::Error for StopTestError {}
3737

38-
static PROTOCOL_DEBUG: LazyLock<bool> = LazyLock::new(|| {
38+
fn protocol_debug() -> bool {
3939
matches!(
4040
std::env::var("HEGEL_PROTOCOL_DEBUG")
4141
.unwrap_or_default()
4242
.to_lowercase()
4343
.as_str(),
4444
"1" | "true"
4545
)
46-
});
46+
}
47+
48+
#[derive(Debug, PartialEq)]
49+
pub(crate) enum RequestErrorKind {
50+
StopTest,
51+
FlakyReplay,
52+
ServerCrashed,
53+
CommunicationError,
54+
}
55+
56+
pub(crate) fn classify_request_error(error_msg: &str, server_exited: bool) -> RequestErrorKind {
57+
if error_msg.contains("overflow")
58+
|| error_msg.contains("StopTest")
59+
|| error_msg.contains("channel is closed")
60+
{
61+
RequestErrorKind::StopTest
62+
} else if error_msg.contains("FlakyStrategyDefinition") || error_msg.contains("FlakyReplay") {
63+
RequestErrorKind::FlakyReplay
64+
} else if server_exited {
65+
RequestErrorKind::ServerCrashed
66+
} else {
67+
RequestErrorKind::CommunicationError
68+
}
69+
}
4770

4871
pub(crate) const ASSUME_FAIL_STRING: &str = "__HEGEL_ASSUME_FAIL";
4972

@@ -269,7 +292,7 @@ impl TestCase {
269292
if global.test_aborted {
270293
return Err(StopTestError);
271294
}
272-
let debug = *PROTOCOL_DEBUG || global.verbosity == Verbosity::Debug;
295+
let debug = protocol_debug() || global.verbosity == Verbosity::Debug;
273296

274297
let mut entries = vec![(
275298
Value::Text("command".to_string()),
@@ -300,32 +323,31 @@ impl TestCase {
300323
}
301324
Err(e) => {
302325
let error_msg = e.to_string();
303-
if error_msg.contains("overflow")
304-
|| error_msg.contains("StopTest")
305-
|| error_msg.contains("channel is closed")
306-
{
307-
if debug {
308-
eprintln!("RESPONSE: StopTest/overflow");
326+
let server_exited = self.global.borrow().connection.server_has_exited();
327+
match classify_request_error(&error_msg, server_exited) {
328+
RequestErrorKind::StopTest => {
329+
if debug {
330+
eprintln!("RESPONSE: StopTest/overflow");
331+
}
332+
let mut global = self.global.borrow_mut();
333+
global.channel.mark_closed();
334+
global.test_aborted = true;
335+
drop(global);
336+
Err(StopTestError)
337+
}
338+
RequestErrorKind::FlakyReplay => {
339+
let mut global = self.global.borrow_mut();
340+
global.channel.mark_closed();
341+
global.test_aborted = true;
342+
drop(global);
343+
Err(StopTestError)
344+
}
345+
RequestErrorKind::ServerCrashed => {
346+
panic!("{}", SERVER_CRASHED_MESSAGE);
347+
}
348+
RequestErrorKind::CommunicationError => {
349+
panic!("Failed to communicate with Hegel: {}", e);
309350
}
310-
let mut global = self.global.borrow_mut();
311-
global.channel.mark_closed();
312-
global.test_aborted = true;
313-
drop(global);
314-
Err(StopTestError)
315-
} else if error_msg.contains("FlakyStrategyDefinition")
316-
|| error_msg.contains("FlakyReplay")
317-
{
318-
// Abort the test case; the server will report the flaky
319-
// error in the test_done results, which runner.rs handles.
320-
let mut global = self.global.borrow_mut();
321-
global.channel.mark_closed();
322-
global.test_aborted = true;
323-
drop(global);
324-
Err(StopTestError)
325-
} else if self.global.borrow().connection.server_has_exited() {
326-
panic!("{}", SERVER_CRASHED_MESSAGE);
327-
} else {
328-
panic!("Failed to communicate with Hegel: {}", e);
329351
}
330352
}
331353
}

0 commit comments

Comments
 (0)