Do not double panic in FakeParentArgs::drop #1856
Conversation
src/utils/process.rs
Outdated
| fn test_process_testing_assert_never_used() { | ||
| let _args = FakeParentArgs::once("never used"); | ||
|
|
||
| // causes a panic while panicking, so can't test: | ||
| // let _args = FakeParentArgs::for_scope(&"never used"); | ||
| // let _args = FakeParentArgs::once(&"never used"); | ||
| let _args = FakeParentArgs::for_scope(&"never used"); | ||
| let _args = FakeParentArgs::once(&"never used"); | ||
| } | ||
|
|
There was a problem hiding this comment.
Nice. Now that this is testable, this test needs to be split into separate cases. once() should panic on drop, even stand-alone as before, because it is never used.
But calling for_scope twice should panic in the constructor because the previous state (in the thread local variable) was not cleared out properly (and previously the drop also panicked because it encountered bad state as well).
Actually, couldn't FakeParentArgs::error just set the state so that drop doesn't panic again? And to ensure the correct panic is hit maybe the more precise #[should_panic(expected = "msg here")] can be used.
There was a problem hiding this comment.
I think all FakeParentArgs will go through drop before error being called in this construct.
if FAKE_ARGS.with(|a| a.replace(initial(string_vec))) != TlsState::None {
Self::error(from_);
}
Thus, we'll only get panics from the drop method and none from the constructors
dvermd
force-pushed
the
do_not_double_panic
branch
2 times, most recently
from
4f852b2
to
e8cf280
Compare
Signed-off-by: dvermd <[email protected]>
dvermd
force-pushed
the
do_not_double_panic
branch
from
e8cf280
to
5c5da90
Compare
Check if we're already panicking before calling error which will also panic.