zed-industries / zed

Code at the speed of thought – Zed is a high-performance, multiplayer code editor from the creators of Atom and Tree-sitter.
https://zed.dev
Other
50k stars 3.06k forks source link

Rust tasks do not work in lib.rs files due to use of ZED_STEM #19161

Open bitfield opened 1 month ago

bitfield commented 1 month ago

Check for existing issues

Describe the bug / provide steps to reproduce it

When editing a Rust lib.rs with a mod tests section (I'm guessing any item with a #[cfg(test)] attribute), there's a little clickable arrow next to it to run the tests. Very nice. But no tests actually get run, because they're all filtered out:

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 4 filtered out; finished in 0.00s

That's because Zed runs:

⏵ Task `cargo test -p my_crate lib` finished successfully
⏵ Command: /bin/zsh -i -c 'cargo test -p my_crate lib'

The command here should (I think) actually be:

 /bin/zsh -i -c 'cargo test -p my_crate --lib'

The lib that Zed is passing is interpreted by Cargo as a filter for the test names it should run, and since no test names happen to contain lib, none of them are run. The flag should be --lib instead ("run all tests in this library"). I'm not sure where this is actually defined: it doesn't seem to be in Zed's default settings.

Is this even Zed's fault, or is it a rust-analyzer issue?

Environment

Zed: v0.156.1 (Zed) OS: macOS 15.0.1 Memory: 8 GiB Architecture: aarch64

If applicable, add mockups / screenshots to help explain present your vision of the feature

Screenshot 2024-10-13 at 19 39 15

If applicable, attach your Zed.log file to this issue.

Zed.log


notpeter commented 1 month ago

I do not believe Zed supports LSP runnables yet:

Here's where it is defined in the code:

https://github.com/zed-industries/zed/blob/cdead5760a2c200179e6a51d5a396c33a3e06e3d/crates/languages/src/rust.rs#L478-L495

As a work around, I think you could create creating a task with the alternate --lib form and then adding "tags": ["rust-test"] to it and it would show up as an option when you click the play button in the sidebar.

See: Binding runnable tags to task templates in the docs.

osiewicz commented 1 month ago

Do you have any custom tasks defined? I don't think we include that lib bit anywhere ourselves.

bitfield commented 1 month ago

Do you have any custom tasks defined?

No. From the snippet posted by @notpeter, the lib is coming from VariableName::Stem.template_value(). Stem is documented as:

/// Stem (filename without extension) of the currently opened file.

...which is lib because I'm editing lib.rs.

But any argument to cargo test that's not a flag is interpreted as a filter for test names to run, per the Cargo book. The lib isn't needed and stops the tests running at all, while cargo test -p my_crate works just fine.

I think this should simply be:

            TaskTemplate {
                label: format!(
                    "cargo test -p {}",
                    RUST_PACKAGE_TASK_VARIABLE.template_value(),
                ),
                command: "cargo".into(),
                args: vec![
                    "test".into(),
                    "-p".into(),
                    RUST_PACKAGE_TASK_VARIABLE.template_value(),
                ],
                tags: vec!["rust-mod-test".to_owned()],
                cwd: Some("$ZED_DIRNAME".to_owned()),
                ..TaskTemplate::default()
            },

I don't see how it could ever have worked otherwise.

osiewicz commented 1 month ago

Well, that was the point - that the stem is a filter expression. We should probably special-case lib.rs. Thanks for the report!

bitfield commented 1 month ago

the stem is a filter expression

I'm confused about this, because as far as I can tell, Cargo doesn't apply filter expressions to file names. They apply only to test names. Are we talking at cross-purposes somehow? What is adding this filename stem supposed to do?

osiewicz commented 1 month ago

You're right. We're using stem as more often than not the stem is also the module name (although it doesn't have to be the case) to limit the # of executed tests.

bitfield commented 4 weeks ago

Shall I put in a PR with the suggested fix?