Auto merge of #16880 - HKalbasi:test-explorer, r=HKalbasi

Use `--workspace` and `--no-fail-fast` in test explorer

This PR contains:
* Using `--workspace` in `cargo test` command, to running all tests even when there is a crate in the root of a workspace
* Using `--no-fail-fast` to run all requested tests
* Excluding bench in the test explorer
* Fixing a bug in the `hack_recover_crate_name`

fix #16874

Author: bors

crates/flycheck/src/test_runner.rs

@@ -55,13 +55,16 @@ pub struct CargoTestHandle {
 }
 
 // Example of a cargo test command:
-// cargo test -- module::func -Z unstable-options --format=json
+// cargo test --workspace --no-fail-fast -- module::func -Z unstable-options --format=json
 
 impl CargoTestHandle {
     pub fn new(path: Option<&str>) -> std::io::Result<Self> {
         let mut cmd = Command::new(Tool::Cargo.path());
         cmd.env("RUSTC_BOOTSTRAP", "1");
         cmd.arg("test");
+        cmd.arg("--workspace");
+        // --no-fail-fast is needed to ensure that all requested tests will run
+        cmd.arg("--no-fail-fast");
         cmd.arg("--");
         if let Some(path) = path {
             cmd.arg(path);

crates/rust-analyzer/src/handlers/request.rs

@@ -248,7 +248,7 @@ pub(crate) fn handle_discover_test(
     Ok(lsp_ext::DiscoverTestResults {
         tests: tests
             .into_iter()
-            .map(|t| {
+            .filter_map(|t| {
                 let line_index = t.file.and_then(|f| snap.file_line_index(f).ok());
                 to_proto::test_item(&snap, t, line_index.as_ref())
             })

crates/rust-analyzer/src/lsp/to_proto.rs

@@ -1516,8 +1516,8 @@ pub(crate) fn test_item(
     snap: &GlobalStateSnapshot,
     test_item: ide::TestItem,
     line_index: Option<&LineIndex>,
-) -> lsp_ext::TestItem {
-    lsp_ext::TestItem {
+) -> Option<lsp_ext::TestItem> {
+    Some(lsp_ext::TestItem {
         id: test_item.id,
         label: test_item.label,
         kind: match test_item.kind {
@@ -1532,9 +1532,9 @@ pub(crate) fn test_item(
                     | project_model::TargetKind::Example
                     | project_model::TargetKind::BuildScript
                     | project_model::TargetKind::Other => lsp_ext::TestItemKind::Package,
-                    project_model::TargetKind::Test | project_model::TargetKind::Bench => {
-                        lsp_ext::TestItemKind::Test
-                    }
+                    project_model::TargetKind::Test => lsp_ext::TestItemKind::Test,
+                    // benches are not tests needed to be shown in the test explorer
+                    project_model::TargetKind::Bench => return None,
                 }
             }
             ide::TestItemKind::Module => lsp_ext::TestItemKind::Module,
@@ -1550,7 +1550,7 @@ pub(crate) fn test_item(
             .map(|f| lsp_types::TextDocumentIdentifier { uri: url(snap, f) }),
         range: line_index.and_then(|l| Some(range(l, test_item.text_range?))),
         runnable: test_item.runnable.and_then(|r| runnable(snap, r).ok()),
-    }
+    })
 }
 
 pub(crate) mod command {

crates/rust-analyzer/src/main_loop.rs

@@ -552,7 +552,7 @@ impl GlobalState {
                 Task::DiscoverTest(lsp_ext::DiscoverTestResults {
                     tests: tests
                         .into_iter()
-                        .map(|t| {
+                        .filter_map(|t| {
                             let line_index = t.file.and_then(|f| snapshot.file_line_index(f).ok());
                             to_proto::test_item(&snapshot, t, line_index.as_ref())
                         })

editors/code/src/test_explorer.ts

@@ -105,8 +105,9 @@ export const prepareTestExplorer = (
             testSet.add(test.id);
         }
         // FIXME(hack_recover_crate_name): We eagerly resolve every test if we got a lazy top level response (detected
-        // by `!scope`). ctx is not a good thing and wastes cpu and memory unnecessarily, so we should remove it.
-        if (!scope) {
+        // by checking that `scope` is empty). This is not a good thing and wastes cpu and memory unnecessarily, so we
+        // should remove it.
+        if (scope.length === 0) {
             for (const test of tests) {
                 void testController.resolveHandler!(idToTestMap.get(test.id));
             }