Conversation
Agent-Logs-Url: https://github.com/camshaft/bolero/sessions/731c4998-23c5-4218-9917-18218f0649c5 Co-authored-by: camshaft <799311+camshaft@users.noreply.github.com>
Agent-Logs-Url: https://github.com/camshaft/bolero/sessions/571b86bb-5e6f-4f78-be55-2947940c4b53 Co-authored-by: camshaft <799311+camshaft@users.noreply.github.com>
Agent-Logs-Url: https://github.com/camshaft/bolero/sessions/571b86bb-5e6f-4f78-be55-2947940c4b53 Co-authored-by: camshaft <799311+camshaft@users.noreply.github.com>
camshaft
left a comment
There was a problem hiding this comment.
I think we'll need to make the input path a Rc instead if we require that the application clones out the state every time. That or we'll need a way for them to pass a callback and inspect the context and return something from that. That's probably cheaper than the clone
| /// iteration counter without reconstructing the context guard. Has no effect | ||
| /// when called outside a bolero test harness. | ||
| #[doc(hidden)] | ||
| pub fn set_iteration(iteration: u64) { |
There was a problem hiding this comment.
Maybe instead of individual setters we just had an update function where the caller could mutate it themselves. That way you don't have to deref the thread local over and over again.
There was a problem hiding this comment.
Done in 02ec219. Replaced the three individual setters with a single update(FnOnce(&mut TestRunContext)) that acquires one TLS borrow and lets the caller mutate any fields they need in one shot.
… API Agent-Logs-Url: https://github.com/camshaft/bolero/sessions/425a5660-68af-4169-9815-68b4e58e807a Co-authored-by: camshaft <799311+camshaft@users.noreply.github.com>
Added |
RunPhase and in-place context setters to TestRunContextRunPhase, update(), and with_context() to TestRunContext
| bolero_engine::test_context::update(|ctx| { | ||
| ctx.run_phase = bolero_engine::RunPhase::Shrink; | ||
| }); |
There was a problem hiding this comment.
instead of making every engine set the shrink phase would it be better to have the shrinking logic set the phase? and i think that would actually be better cause it would need to run the input one last time with the Failure run phase set so the application could actually capture the log. otherwise the failure is set but we didn't actually rerun anything so the log output will be empty if the application is filtering based on run_phase == Failure
There was a problem hiding this comment.
Done in 0807b78.
RunPhase::Shrink and RunPhase::Failure are now set entirely inside Shrinker::shrink() in bolero-engine/src/shrink.rs — the individual engines no longer touch the phase during shrinking:
RunPhase::Shrinkis set at the start of the shrink loop (after the zero-shrink-time early return)RunPhase::Failureis set immediately before the final confirmed-failureexecute()call (line ~149), so the application actually runs the minimal failing input withFailurephase and can capture diagnostic output
For the "no shrink" path where test.shrink() returns None (shrink time=0 or no improvement), each call site now sets RunPhase::Failure and re-runs the original input before formatting the error message — covering the File, Rng, exhaustive, and libfuzzer cases.
…rink paths Agent-Logs-Url: https://github.com/camshaft/bolero/sessions/cc35feda-db54-4d7e-a531-3af1a3c5142c Co-authored-by: camshaft <799311+camshaft@users.noreply.github.com>
| // Set the failure phase before the final confirmed-failure execution so | ||
| // the application can capture diagnostic output for the minimal failing input. | ||
| crate::test_context::update(|ctx| { | ||
| ctx.run_phase = crate::RunPhase::Failure; | ||
| }); |
There was a problem hiding this comment.
yeah this is great! exactly what i was thinking. that being said, we need to figure out what to do when shrinking is disabled. I think we probably add another field to the context to indicate that it is and that the application will need to buffer differently. and it would be more about the last input that was ran instead of getting an explicit signal that there was a failure. the problem is by that point, bolero panics and there's no way for the application to get a signal that we're about to panic and it needs to dump its buffered log. so i think we'll probably also need to add a way for the application to register a Box<dyn Fn()> that triggers right before we panic and fail the test.
There was a problem hiding this comment.
Done in b001508.
Added two things:
shrink_enabled: bool on TestRunContext — defaults to true. The test engine sets it correctly in run_tests (based on rng_options.shrink_time_or_default().is_zero()) and in run_exhaustive (always false). Fuzzer engines leave it at the default true since their shrinking is handled externally.
bolero::on_failure(f: impl FnOnce() + 'static) — stored in a separate ON_FAILURE TLS cell (kept separate from TestRunContext since FnOnce isn't Clone). The callback is:
- Cleared at the start of each iteration via
clear_on_failure()(prevents a stale callback from a passing iteration firing on a later failure) - Invoked just before
panic!("test failed")inrun_tests/run_exhaustiveand just beforeabort()in libfuzzer
The intended flow: during the RunPhase::Failure re-run, the application registers bolero::on_failure(|| flush_logs()). When the re-run ends and the harness is about to panic, invoke_on_failure() fires the callback and the buffered logs are flushed before the process exits.
Agent-Logs-Url: https://github.com/camshaft/bolero/sessions/3443e0a3-6789-40f0-a1ce-cffb264b6db0 Co-authored-by: camshaft <799311+camshaft@users.noreply.github.com>
Agent-Logs-Url: https://github.com/camshaft/bolero/sessions/3443e0a3-6789-40f0-a1ce-cffb264b6db0 Co-authored-by: camshaft <799311+camshaft@users.noreply.github.com>
RunPhase, update(), and with_context() to TestRunContextRunPhase, update(), with_context(), shrink_enabled, and on_failure() to TestRunContext
There was a problem hiding this comment.
Pull request overview
This PR expands TestRunContext so applications can distinguish normal runs vs shrinking vs confirmed failures, read context without cloning, and run a per-failure callback before the harness panics/aborts. It also updates the built-in test engine and several fuzzing engines to populate the new context fields and propagate phase transitions through shrinking.
Changes:
- Added
RunPhase,iteration,shrink_enabled,with_context(),update(), and anon_failure()callback mechanism tobolero-engine’s test context. - Updated the
bolerotest engine to enter the TLS context once andupdate()it per-iteration; invokeson_failurejust before panicking. - Propagated shrink/failure phases from
Shrinkerand updated multiple fuzzing engines for the newTestRunContext::new(...)signature.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/bolero/src/test/mod.rs | Enters context once, updates per iteration, clears/invokes failure callbacks, and replays failing inputs with RunPhase::Failure. |
| lib/bolero/src/lib.rs | Re-exports new public context APIs (RunPhase, with_context, on_failure). |
| lib/bolero-libfuzzer/src/lib.rs | Updates context construction; adds failure-phase replay on no-shrink path and invokes failure callback before abort (in one path). |
| lib/bolero-honggfuzz/src/lib.rs | Updates context construction for new TestRunContext::new(...) signature. |
| lib/bolero-afl/src/lib.rs | Updates context construction for new TestRunContext::new(...) signature. |
| lib/bolero-engine/src/test_context.rs | Introduces RunPhase, extends TestRunContext, adds update()/with_context()/on_failure() + TLS callback plumbing. |
| lib/bolero-engine/src/shrink.rs | Sets RunPhase::Shrink during shrinking and RunPhase::Failure for the final confirmed-failure execution. |
| lib/bolero-engine/src/lib.rs | Re-exports new public context types/functions from test_context. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #[cfg(not(kani))] | ||
| mod std_impl { | ||
| use super::TestRunContext; | ||
| use super::{RunPhase, TestInput, TestRunContext}; |
| #[doc(hidden)] | ||
| pub fn new(engine: EngineKind, input: TestInput) -> Self { | ||
| Self { engine, input } | ||
| pub fn new(engine: EngineKind, input: TestInput, iteration: u64, run_phase: RunPhase) -> Self { |
| pub use test::*; | ||
| pub use test_context::{current_context, is_active, EngineKind, TestInput, TestRunContext}; | ||
| pub use test_context::{ | ||
| current_context, is_active, on_failure, with_context, EngineKind, RunPhase, TestInput, | ||
| TestRunContext, | ||
| }; |
| let shrunken = test.shrink(slice.to_vec(), None, options); | ||
|
|
||
| if let Some(shrunken) = shrunken { | ||
| // shrink.rs already ran the final confirmed-failure execution | ||
| // with RunPhase::Failure set | ||
| eprintln!("{shrunken:#}"); | ||
| } else { | ||
| // Shrinking was skipped or made no progress. | ||
| // Set failure phase and re-run the original input so the | ||
| // application can capture diagnostic output. | ||
| bolero_engine::test_context::update(|ctx| { | ||
| ctx.run_phase = bolero_engine::RunPhase::Failure; | ||
| }); | ||
| let mut replay = input::cache::Bytes::new(slice, options, &mut cache); | ||
| let _ = test.test(&mut replay); |
| panic::set_hook(); | ||
| panic::forward_panic(false); | ||
|
|
||
| let _ctx_guard = | ||
| bolero_engine::test_context::enter(bolero_engine::TestRunContext::new( | ||
| bolero_engine::EngineKind::LibFuzzer, | ||
| bolero_engine::TestInput::default(), | ||
| 0, | ||
| bolero_engine::RunPhase::Normal, | ||
| )); |
| let _ctx_guard = | ||
| bolero_engine::test_context::enter(bolero_engine::TestRunContext::new( | ||
| bolero_engine::EngineKind::Afl, | ||
| bolero_engine::TestInput::default(), | ||
| 0, | ||
| bolero_engine::RunPhase::Normal, | ||
| )); |
| let _ctx_guard = | ||
| bolero_engine::test_context::enter(bolero_engine::TestRunContext::new( | ||
| bolero_engine::EngineKind::Honggfuzz, | ||
| bolero_engine::TestInput::default(), | ||
| 0, | ||
| bolero_engine::RunPhase::Normal, | ||
| )); |
TestRunContextonly trackediterationandinput, giving logging/tracing filters no way to distinguish normal test runs from shrinking or confirmed failures, and no way to mutate the live context without paying the cost of a full guard reconstruction each iteration.Changes
bolero-engine:RunPhaseenum +update()+with_context()RunPhase { Normal, Shrink, Failure }(derivesDefault = Normal) and arun_phasefield onTestRunContextupdate(FnOnce(&mut TestRunContext))— acquires a single TLS borrow and lets the caller mutate any fields in one shot, replacing the need for multiple individual setter callswith_context<F, R>(FnOnce(&TestRunContext) -> R) -> Option<R>— a zero-clone callback API for read-only inspection that avoids thePathBufclone thatcurrent_context()requiresshrink_enabled: boolfield onTestRunContext— lets applications know whether the harness will produceRunPhase::Shrinkiterations beforeRunPhase::Failure; defaults totrueon_failure(f: impl FnOnce() + 'static)— registers a callback invoked just before the harness panics or aborts to report a confirmed failure, allowing applications to flush buffered diagnostic outputlib.rsre-exports to includeRunPhase,update,with_context, andon_failurebolerotest engine: one guard,update()per iterationrun_testsandrun_exhaustivenowenter()once before the loop and callupdate(|ctx| { ... })at the top of each iteration to setinput,iteration, andrun_phasein a single TLS borrowshrink_enabledis set in the initial context based onoptions.shrink_time_or_default()inrun_tests, andfalseinrun_exhaustive(exhaustive mode never shrinks)clear_on_failure()is called at the start of each iteration to prevent stale callbacks from firing on a later failureinvoke_on_failure()is called just beforepanic!("test failed")in bothrun_testsandrun_exhaustiveShrink phase propagation (managed by
Shrinker)RunPhase::ShrinkandRunPhase::Failureare set entirely insideShrinker::shrink()inbolero-engine/src/shrink.rs— individual engines no longer manage these transitionsRunPhase::Shrinkis set at the start of the shrink loop (after the zero-shrink-time early return)RunPhase::Failureis set immediately before the final confirmed-failureexecute()call, so the application actually re-runs the minimal failing input withFailurephase and can capture diagnostic outputtest.shrink()returnsNone— shrink time is zero or no improvement found), each call site setsRunPhase::Failureand re-runs the original input before formatting the error message, covering theFile,Rng,exhaustive, andlibfuzzercasesFuzzer engines (libfuzzer, afl, honggfuzz, kani)
TestRunContext::new()call sites updated to passRunPhase::Normalkani_implforupdate,on_failure,invoke_on_failure, andclear_on_failureinvoke_on_failure()called beforestd::process::abort()in libfuzzerUsage
When a test fails, the application will observe:
RunPhase::NormalRunPhase::Shrink(ifshrink_enabled)RunPhase::Failure— this is the run where diagnostic output should be emittedon_failurecallback fires immediately after step 3, before the harness panics or aborts