regression: tokio::test in 1.38 eats my tests

[zeenix]

Version

1.38

Platform

Linux zeeframework 6.8.10-300.fc40.x86_64 #1 SMP PREEMPT_DYNAMIC Fri May 17 21:20:54 UTC 2024 x86_64 GNU/Linux

Description

On upgrading from 1.37 to 1.38, my tests are suddenly just getting ignored. If I don't set RUSTFLAGS=-Dwarnings env, the tests are just not run at all (which is why the compiler is complaining about the functions being called from the tests, are unused).

[mox692]

After 1.38, #[tokio::test] macro now embeds #[test] attribute most inner than the other macros. (See also #6497)

On the other hand, the timeout macro you are using doesn't seem to recognize the inner #[test] attribute:

// This won't be triggered.
#[timeout]
#[test]
fn greet() {...}

So, it seems that the combination of the tokio::test's change and the current behavior of the timeout macro has made your test broken..

// Before, your `greet` test will be expanded like
#[test]
#[timeout]
fn greet() {...}

// After 1.38, your `greet` test will be expanded like
#[timeout]
#[test]
fn greet() {...}

[Darksonn]

That's unfortunate. There's a good chance we'll have to revert this, as it is a breaking change.

@becheran Ignoring users of #[tokio::test], are you able to clarify whether the current behavior of #[timeout] #[test] is a bug in ntest?

[becheran]

@Darksonn thanks for the hint. Hm... yea, the #[timeout] part in ntest seems broken now. But to be honest I don't really know how I can possibly fix this in the ntest lib. Is the #[tokio::test] macro now always putting every macro below it above the #[test] macro? If that is the case, does #[should_panic] work with #[tokio::test] or is this also broken? If it does, then I will have a look if I can do something similar in ntest.

[Darksonn]

Yes, right now it always puts #[test] at the bottom. The motivation behind putting #[test] at the end is so that if you are using multiple test macros, then they are able to "see" each other and avoid adding multiple copies of #[test].

As for #[should_panic] and #[test], those attributes work in either order, so yes it still works with #[tokio::test]. My understanding is that if #[should_panic] comes first, then it just adds another copy of itself after all other attributes so that it comes after the #[test] macro. And the #[test] macro will remove #[should_panic] attributes that come later.

Looking over your #[timeout] macro, it looks like you are completely ignoring input.attrs. You should attach all of the attributes to the test function instead. Anything that comes after #[timeout] is completely ignored.

[Darksonn]

In the case of this example from your docs:

#[test]
#[timeout(10)]
#[should_panic]
fn timeout() {
    loop {};
}

First, the #[test] macro is expanded. It removes the #[should_panic] attribute and generates a constant:

#[cfg(test)]
#[rustc_test_marker = "my_test"]
pub const timeout: test::TestDescAndFn = /* let's ignore this part */;

#[timeout(10)]
fn timeout() {
    loop {};
}

And then the #[timeout] macro gets to run. So the reason it works with #[should_panic] after #[timeout] is that by the time #[timeout] expands, there is no longer a #[should_panic] attribute after it.

---

If we consider this example:

#[should_panic]
#[test]
#[timeout(10)]
fn timeout() {
    loop {};
}

Then the #[should_panic] attribute runs first and expands to this:

#[test]
#[timeout(10)]
#[should_panic]
fn timeout() {
    loop {};
}

Which in turn expands as above.

---

Finally, this example does not work:

#[timeout(10)]
#[test]
#[should_panic]
fn timeout() {
    loop {};
}

Because here #[timeout] is expanded first, and it expands to this:

fn timeout() {
    fn ntest_callback() {
        loop {};
    }
    let ntest_timeout_now = std::time::Instant::now();
    let (sender, receiver) = std::sync::mpsc::channel();
    std::thread::spawn(move || {
        if let std::result::Result::Ok(()) = sender.send(ntest_callback()) {}
    });
    match receiver.recv_timeout(std::time::Duration::from_millis(10u64)) {
        std::result::Result::Ok(t) => return t,
        Err(std::sync::mpsc::RecvTimeoutError::Timeout) => {
            ::std::rt::panic_fmt(
                format_args!(
                    "timeout: the function call took {0} ms. Max time {1} ms",
                    ntest_timeout_now.elapsed().as_millis(),
                    10u64,
                ),
            );
        }
        Err(std::sync::mpsc::RecvTimeoutError::Disconnected) => {
            ::std::rt::begin_panic("explicit panic")
        }
    }
}

The #[timeout] attribute does not produce any attributes in the output, so no further expansion happens.

With #[timeout] fixed, the output should look like this:

#[test]
#[should_panic]
fn timeout() {
    fn ntest_callback() {
        loop {};
    }
    ...
}

[mox692]

It seems that the ntest(ntest_timeout) with a patch applied has been released, so I'll close this issue.

Please open a new issue if you encounter similar problems.