Wrong panic message when #[timeout(...)] is not expired

[Joining7943]

In rstests with the #[timeout(...)] macro attribute, the panic messages in tests in which the timeout is not reached are replaced by Timeout ... expired.

The original panics are still readable because the timeout thread panicked, but it's very confusing to see a Timeout ... expired although the timeout didn't expire. Also, it's not possible to match against the original panic string for example with #[should_panic = "my assertion"]

Here's a short example:

The test code showing the wrong behavior

#[rstest]
#[should_panic = "my error message"]
#[timeout(Duration::from_secs(60))]
fn test_me() {
    panic!("my error message");
}

The test output of cargo test for the test_me test function:

---- test_me stdout ----
thread '<unnamed>' panicked at 'my error message', tests/test_me.rs:8:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread 'test_me' panicked at 'Timeout 60s expired', /home/lenny/.cargo/registry/src/github.com-1ecc6299db9ec823/rstest-0.16.0/src/timeout.rs:16:29
note: panic did not contain expected string
      panic message: `"Timeout 60s expired"`,
 expected substring: `"my error message"`

failures:
    test_me

The correct test output shouldn't show the panic message of the <unnamed> thread and should show thread 'test_me' panicked at 'my error message' instead of thread 'test_me' panicked at 'Timeout 60s expired'.

If interested, I can submit a pr fixing this in the execute_with_timeout_sync function in rstest/rstest/src/timeout.rs.

[la10736]

Yeah... you are right. Thanks to report it.

I don't know when I can publish the new version.... sorry

[Joining7943]

Sure. I think you should also consider a downcast for String in addition to &str. I didn't noticed it at first, but the assert! macros and panic("{}", "something") produce a String instead of a &str. Could you also remove the panic message of the unnamed thread, please?

[la10736]

Ok.

[Joining7943]

Great! Thanks.

[la10736]

Ok, I fixed the String casting but I don't found a clear and clean way to remove the unnamed thread: I should run another thread to implement timeout (timeout should be in the current thread) and this message will contain the original stack trace and correct error coordinates.

The only way that I found to catch the stack trace is to use something like described in https://stackoverflow.com/questions/69593235/how-to-get-panic-information-i-e-stack-trace-with-catch-unwind but it seams a little bit overkilled and it will not prevent the panic output.

If you are ok I'm incline to close this ticket now.... If you have some ideas of how to move the panic from other thread to test thread I would really happy to hear you.

[Joining7943]

Great thanks for your work.

Oh I see. You want to preserve the backtrace, what's pretty cool. After a lot of searching and reading through the stdlib, I found a nice and simple solution.

pub fn execute_with_timeout_sync<T: 'static + Send, F: FnOnce() -> T + Send + 'static>(
    code: F,
    timeout: Duration,
) -> T {
    let (sender, receiver) = mpsc::channel();

    let thread = if let Some(name) = thread::current().name() {
        std::thread::Builder::new().name(name.to_string())
    } else {
        std::thread::Builder::new()
    };
    let handle = thread.spawn(move || sender.send(code())).unwrap();

    match receiver.recv_timeout(timeout) {
        Ok(result) => {
            // Unwraps are safe because we got a result from the thread, which is not a `SendError`,
            // and there was no panic within the thread which caused a disconnect.
            handle.join().unwrap().unwrap();
            result
        }
        Err(RecvTimeoutError::Timeout) => {
            panic!("Timeout {:?} expired", timeout)
        }
        Err(RecvTimeoutError::Disconnected) => match handle.join() {
            Ok(_) => unreachable!(),
            Err(any) => {
                panic::resume_unwind(any);
            }
        },
    }
}

Sorry, I've written this on the base of an older commit on the main branch, but I hope you get the idea.

[la10736]

Wow!!! Seams great! Tomorrow I'll try it. Thanks Il mer 14 dic 2022, 21:18 Joining7943 ***@***.***> ha scritto:

…

Great thanks for your work. Oh I see. You want to preserve the backtrace, what's pretty cool. After a lot of searching and reading through the stdlib, I found a nice and simple solution. pub fn execute_with_timeout_sync<T: 'static + Send, F: FnOnce() -> T + Send + 'static>( code: F, timeout: Duration,) -> T { let (sender, receiver) = mpsc::channel(); let thread = if let Some(name) = thread::current().name() { std::thread::Builder::new().name(name.to_string()) } else { std::thread::Builder::new() }; let handle = thread.spawn(move || sender.send(code())).unwrap(); match receiver.recv_timeout(timeout) { Ok(result) => { // Unwraps are safe because we got a result from the thread, which is not a `SendError`, // and there was no panic within the thread which caused a disconnect. handle.join().unwrap().unwrap(); result } Err(RecvTimeoutError::Timeout) => { panic!("Timeout {:?} expired", timeout) } Err(RecvTimeoutError::Disconnected) => match handle.join() { Ok(_) => unreachable!(), Err(any) => { panic::resume_unwind(any); } }, }} Sorry, I've written this on the base of an older commit on the main branch, but I hope you get the idea. — Reply to this email directly, view it on GitHub <#171 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA5Y34IMQWR2A7QGTDGN6CLWNITRDANCNFSM6AAAAAAS5BPNUM> . You are receiving this because you modified the open/close state.Message ID: ***@***.***>

[Joining7943]

Sure :)

[la10736]

THX again