ntest icon indicating copy to clipboard operation
ntest copied to clipboard

Run & Debug links gone in VS Code 1.74.1 with Rust Analyzer v0.3.1325

Open chilabot opened this issue 3 years ago • 4 comments

Description VS Code has a pair of handy links for running or debugging individual tests. But when #[ntest::timeout(1000)] is placed, they are removed.

  • VS Code version: 1.74.1.
  • ntest version: 0.9.0
  • Rust Analyzer: v0.3.1325
  • rustc: 1.66.0 (69f9c33d7 2022-12-12)

To Reproduce Put #[ntest::timeout(1000)] over the testing function.

Expected behavior "Run" and "Debug" links should remain.

Screenshots example

Desktop (please complete the following information):

  • OS: Ubuntu 22.04.1 LTS

chilabot avatar Dec 19 '22 19:12 chilabot

Thx for reporting. I will have a look.

becheran avatar Dec 30 '22 21:12 becheran

Sorry for the very late delay. Just had a look how the timout macro expands and I can't see the problem yet.

I have a simple test:

#[test]
#[ntest::timeout(1000)]
fn timeout_test() {
    assert!(false);
}

Which expands to:

extern crate test;
#[cfg(test)]
#[rustc_test_marker = "timeout_test"]
pub const timeout_test: test::TestDescAndFn = test::TestDescAndFn {
    desc: test::TestDesc {
        name: test::StaticTestName("timeout_test"),
        ignore: false,
        ignore_message: ::core::option::Option::None,
        source_file: "tests\\end_to_end.rs",
        start_line: 12usize,
        start_col: 4usize,
        end_line: 12usize,
        end_col: 16usize,
        compile_fail: false,
        no_run: false,
        should_panic: test::ShouldPanic::No,
        test_type: test::TestType::IntegrationTest,
    },
    testfn: test::StaticTestFn(|| test::assert_test_result(timeout_test())),
};
fn timeout_test() {
    fn ntest_callback() {
        if !false {
            ::core::panicking::panic("assertion failed: false")
        }
    }
    let ntest_timeout_now = std::time::Instant::now();
    let (sender, receiver) = std::sync::mpsc::channel();
    std::thread::spawn(move || { if let Ok(()) = sender.send(ntest_callback()) {} });
    match receiver.recv_timeout(std::time::Duration::from_millis(1000u64)) {
        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(),
                    1000u64,
                ),
            );
        }
        Err(std::sync::mpsc::RecvTimeoutError::Disconnected) => {
            ::std::rt::begin_panic("explicit panic")
        }
    }
}

I see that rust analyzer fails to interprete this and does not show the > Run Test icon. But I don't know why? What is wrong with the expansion?

Without using the timeout macro it would have expanded to:

extern crate test;
#[cfg(test)]
#[rustc_test_marker = "timeout_test"]
pub const timeout_test: test::TestDescAndFn = test::TestDescAndFn {
    desc: test::TestDesc {
        name: test::StaticTestName("timeout_test"),
        ignore: false,
        ignore_message: ::core::option::Option::None,
        source_file: "tests\\end_to_end.rs",
        start_line: 11usize,
        start_col: 4usize,
        end_line: 11usize,
        end_col: 16usize,
        compile_fail: false,
        no_run: false,
        should_panic: test::ShouldPanic::No,
        test_type: test::TestType::IntegrationTest,
    },
    testfn: test::StaticTestFn(|| test::assert_test_result(timeout_test())),
};
fn timeout_test() {
    if !false {
        ::core::panicking::panic("assertion failed: false")
    }
}

becheran avatar Jul 18 '23 19:07 becheran

Just a guess but I think the macros are expanded differently when this is performed by RA. I have no other sensible expalnation for panic happening in https://github.com/becheran/ntest/pull/23 when using RA but not when building code with Cargo.

I hope to look into this issue from RA side but cannot give any ETA.

mati865 avatar Jul 18 '23 20:07 mati865

Given the discussion in https://github.com/rust-lang/rust-analyzer/issues/11109 I do believe there is nothing left to fix in this crate and missing test/debug button is RA bug. When RA expands the macros it seems to consider each attribute separately so when expanding #[ntest::timeout(1000)] it "sees" only pub fn ... part and doesn't consider that function as a test.

As a workaround I think it should be possible to provide workaround attribute like that but I'd have to experiment with it first:

#[ntest::timeout_some_name(test, 1000)]
fn timeout_test() {
    assert!(false);
}

#[ntest::timeout_some_name(tokio::test, 1000)]
async fn timeout_test() {
    assert!(false);
}

timeout_some_name (with proper name ofc) would take test macro and timeout, add or expand that macro (don't know yet how RA will behave) over code generated by ntest and then return it to RA.

mati865 avatar Aug 11 '23 00:08 mati865