Add features handle-panics and backtrace
#421
Conversation
proptest/src/test_runner/runner.rs
Outdated
|
|
||
| static INIT_ONCE: Once = Once::new(); | ||
| // NB: no need for external sync, value is mutated only once, when init is performed | ||
| static mut DEF_HOOK: Option<Box<dyn Fn(&PanicInfo<'_>) + Send + Sync>> = |
There was a problem hiding this comment.
please expand this to DEFAULT_HOOK for clarity, i'm concerned that "DEF" could be mistaken for something else
| @@ -210,6 +212,96 @@ where | |||
| }) | |||
| } | |||
|
|
|||
| #[cfg(feature = "handle-panics")] | |||
| mod panicky { | |||
There was a problem hiding this comment.
given the unsafe usage, this section could use a bit more documentation i think. possibly specifying what the responsibility of each static variable and each function as well as clarifying the order of operations. i don't think it needs to be long but just enough to provide more clarity
There was a problem hiding this comment.
Done. Added more comments, doc comments for funcs and variables, and module-level doc comment with overall description of whole process
proptest/src/test_runner/errors.rs
Outdated
| (Self::Fail(l0, _, l2), Self::Fail(r0, _, r2)) => { | ||
| l0 == r0 && l2 == r2 | ||
| } | ||
| _ => false, |
There was a problem hiding this comment.
i think i'd prefer if we exhaustively match here, even if it's more verbose
|
My biggest concern is that I had to change |
|
I had started going through this last night. will finish up later tonight |
Ah right, thank you for raising this. I don't think we can merge then until we move to a new major version unfortunately, or without making it a separate feature like you suggested, and yes I can see how that's unappealing. |
rexmas
added
the
2.0-wishlist
|
Ok, then I guess the best course is to take out |
|
we're looking at a 2.0 release so we can include this and a couple other things that are pending. it'd probably take 1-2 months to get everything together if those timelines work for you. also happy to accept changes with no backwards compatibility issues in the meantime if you want a faster turnaround |
handle-panicsattempts to capture panic backtrace when one happens, instead of spitting it to consolebacktraceenables actual backtraces usingstd::backtraceBacktrace::frames()andBacktraceFrameare still experimental.P.S: Sorry for "PR spam", prev one #419 was closed prematurely due to accident
closes #356