fix bug

Author: Song Gao

editors/code/src/test_explorer/README.md

@@ -8,7 +8,7 @@ An obvious choice to to load all tests for all projects at the first time, then
 Another choice is only to load test cases laziness. Only when we open a file or click expand button of the case in test explorer, we load itself and its parents(if they are not loaded yet).
 
 1. Where should user go when they click "open file" for test module, definition or the declaration?
-For now, I choose definition
+For now, I choose declaration
 
 1. How to know whether a test case start? When run the whole test suite, how to know the test case in it is queued or started?
 1. How to support multi select test cases, especially when the target is differnt(part of them might be --lib, and part of them might be --test)?

editors/code/src/test_explorer/TestControllerHelper.ts

@@ -1,85 +1,10 @@
 import * as vscode from "vscode";
 import { testController } from ".";
-import { assert } from "../util";
-
-interface TestItemWithParentCollection {
-    testItem: vscode.TestItem;
-
-    collection: vscode.TestItemCollection;
-}
 
 /**
  * General helper functions for VSCode TestItemController
  */
 export abstract class TestControllerHelper {
-    /**
-     * This is feasible for the id we are using is in order, like the parent of "A::B::C::D" is "A::B::C"
-     *
-     * Return `undefined`, if the package level item is not created.
-     */
-    static findNeartestTestItemById(id: string): vscode.TestItem | undefined {
-        const idPaths = id.split("::");
-        const indexLength = idPaths.length;
-        let currentId = idPaths[0];
-        let candidate: vscode.TestItem | undefined;
-        candidate = testController!.items.get(currentId);
-        for (let index = 1; index < indexLength; index++) {
-            currentId = currentId + `::${idPaths[index]}`;
-            const next = candidate?.children.get(currentId);
-            if (next === undefined) {
-                return candidate;
-            }
-            candidate = next;
-        }
-
-        return candidate;
-    }
-
-    /**
-     * This is feasible for the id of each item is unique and in regular.
-     *
-     * Unique -- could compare id directly
-     * Regular -- not need to traversal the tree
-     */
-    static findTestItemById(id: string): vscode.TestItem | undefined {
-        const nearestItem = TestControllerHelper.findNeartestTestItemById(id);
-        if (nearestItem?.id === id) {
-            return nearestItem;
-        }
-        return undefined;
-    }
-
-    static findTestItemPathsByUri(
-        uri: vscode.Uri,
-        testItemCollection: vscode.TestItemCollection = testController!.items
-    ): TestItemWithParentCollection[] {
-        const results: TestItemWithParentCollection[] = [];
-        const curTestItemPaths: vscode.TestItem[] = [];
-        const curTestItemCollectionPaths: vscode.TestItemCollection[] = [];
-
-        TestControllerHelper.visitTestItemTreePreOrder((item, collection) => {
-            curTestItemPaths.push(item);
-            curTestItemCollectionPaths.push(collection);
-            if (item.uri?.toString() === uri.toString()) {
-                assert(curTestItemPaths.length === curTestItemCollectionPaths.length, "each item must be contained in a collection.");
-                for (let index = 0; index < curTestItemPaths.length; index++) {
-                    const testItem = curTestItemPaths[index];
-                    const collection = curTestItemCollectionPaths[index];
-                    results.push({
-                        collection,
-                        testItem,
-                    });
-                }
-                return true;
-            };
-            return;
-        }, testItemCollection, () => {
-            curTestItemPaths.pop();
-            curTestItemCollectionPaths.pop();
-        });
-        return results;
-    }
-
     /**
      *
      * @param cb Stop serach the current subtree if returning non-falsy value. But the rest subtree will continute to search.
@@ -100,14 +25,4 @@ export abstract class TestControllerHelper {
             exitCb?.(item, collection);
         });
     }
-
-    static visitTestItemTreePostOrder(
-        cb: (item: vscode.TestItem, collection: vscode.TestItemCollection) => void,
-        root: vscode.TestItemCollection = testController!.items
-    ) {
-        root.forEach((item, collection) => {
-            TestControllerHelper.visitTestItemTreePostOrder(cb, item.children);
-            cb(item, collection);
-        });
-    }
 }

editors/code/src/test_explorer/TestMetadata.ts

@@ -3,11 +3,6 @@ import * as ra from "../lsp_ext";
 import { assert, assertNever } from "../util";
 import { TargetKind, NodeKind, TestLikeNodeKind, TestLocation } from "./test_model_tree";
 
-export enum TestKind {
-    TestModule,
-    Test
-}
-
 export class RunnableFacde {
     public readonly origin: ra.Runnable;
 

editors/code/src/test_explorer/discover_and_update.ts

@@ -35,7 +35,7 @@ export function registerWatcherForWorkspaces() {
         }
 
         console.log("onDidChangeTextDocument callback");
-        debounceUpToDownUpdateTestItems(e.document.uri);
+        debounceHandleFileChangeCore(e.document.uri);
 
         function isRustFile(document: vscode.TextDocument): boolean {
             return document.fileName.toLowerCase().endsWith('.rs');
@@ -62,17 +62,21 @@ function watchWorkspace(workspaceFolder: vscode.WorkspaceFolder) {
 function debounce(fn: Function, ms: number) {
     let timeout: NodeJS.Timeout | undefined = undefined;
     return (...params: any[]) => {
-        console.log(performance.now());
+        console.log("debounce debug: " + performance.now());
         clearTimeout(timeout);
         timeout = setTimeout(() => {
             fn(...params);
         }, ms);
     };
 }
 
-const deboundeRefresh = debounce(refreshAllThings, 500);
+// Why choose 2s:
+// when auto save is enabled, there seems to be 2 events for workspace.onDidChangeTextDocument
+// the first one is for the change of the file, the second one is for the save of the file
+// And usually it takes about 1s between on my machine between the two events
+const deboundeRefresh = debounce(refreshAllThings, 2000);
 // FIXME: if there is changes in two files, we will lost the first chagne
-const debounceUpToDownUpdateTestItems = debounce(updateModelByChangeOfFile, 500);
+const debounceHandleFileChangeCore = debounce(handleFileChangeCore, 2000);
 
 export async function refreshAllThings() {
     if (!isInitilized) return;
@@ -133,10 +137,14 @@ async function handleFileCreate(uri: vscode.Uri) {
     updateTestItemsByModel();
 }
 
+async function handleFileChangeCore(uri: vscode.Uri) {
+    await updateModelByChangeOfFile(uri);
+    updateTestItemsByModel();
+}
+
 async function handleFileChange(uri: vscode.Uri) {
     console.log(`handleFileChange triggered for ${uri}`);
-    await debounceUpToDownUpdateTestItems(uri);
-    updateTestItemsByModel();
+    await debounceHandleFileChangeCore(uri);
 }
 
 async function handleFileDelete(uri: vscode.Uri) {
@@ -148,24 +156,43 @@ async function handleFileDelete(uri: vscode.Uri) {
 function updateTestItemsByModel() {
     testController!.items.replace([]);
     const rootTestItems = VscodeTestTreeBuilder.build();
-    // const mockItem = testController!.createTestItem('123', '456');
-    // testController!.items.add(mockItem);
     testController!.items.replace(rootTestItems);
 }
 
+async function getNormalizedTestRunnablesInFile(uri: vscode.Uri) {
+    const rawRunables = await RaApiHelper.getTestRunnablesInFile(uri);
+
+    assert(!!rawRunables);
+
+    const runnables = rawRunables.map(it => new RunnableFacde(it));
+
+    // User might copy and past test, and then there might be same name test or test module
+    // Although it's wrong, we need to tollerate it.
+    // We only choose the first one.
+    return uniqueRunnables(runnables);
+
+    function uniqueRunnables(runnables: RunnableFacde[]) {
+        const map = new Map<string, RunnableFacde>();
+        runnables.forEach(runnable => {
+            const key = `${runnable.workspaceRoot}|${runnable.packageName}|${runnable.targetKind}|${runnable.targetName}|${runnable.origin.label}`;
+            if (!map.has(key)) {
+                map.set(key,runnable);
+            }
+        });
+        return Array.from(map.values());
+    }
+}
+
 async function updateModelByChangeOfFile(uri: vscode.Uri) {
-    const testInfos = await RaApiHelper.getTestRunnablesInFile(uri);
+    const runnables = await getNormalizedTestRunnablesInFile(uri);
 
     // Maybe from some to none
     // need to recursively clean the parent, until there is at least one test cases.
-    if (testInfos === null || testInfos.length === 0) {
+    if (runnables.length === 0) {
         testModelTree.removeTestItemsRecusivelyByUri(uri);
         return;
     }
 
-    // Ensure parent is created in the test-model-tree
-    const runnables = testInfos.map(it => new RunnableFacde(it));
-
     const testModuelRunnables = runnables.filter(it =>
         it.testKind === NodeKind.TestModule)
         .sort(RunnableFacde.sortByLabel);
@@ -175,6 +202,7 @@ async function updateModelByChangeOfFile(uri: vscode.Uri) {
 
     assert(testModuelRunnables.length + testItemRunnables.length === runnables.length);
 
+    // FIXME: should be file test modules
     const rootTestModuleRunnbale = testModuelRunnables[0];
 
     // Now, we know the root test module
@@ -231,13 +259,13 @@ async function updateModelByChangeOfFile(uri: vscode.Uri) {
     //       but this is overkill, we could reuse the test modules
     //       if definition are same(and not in the same file)
     if (nearestNode.kind === NodeKind.TestModule
-        && nearestNode.declarationInfo.uri.toString() === rootTestModuleRunnbale.uri.toString()
+        && nearestNode.definitionUri.toString() === rootTestModuleRunnbale.uri.toString()
         && nearestNode.name === rootTestModuleRunnbale.testOrSuiteName) {
         nearestNode.testChildren.clear();
     }
 
     // collect nodes which children need to be fetched
-    const nodeNeededFetched = FalsyLeavesCollector.collect();
+    const nodeNeededFetched = FalsyLeavesCollector.collect(nearestNode);
 
     // fetch the children for nodes
     await fetchChildrenForFalsyLeaves(nodeNeededFetched);
@@ -258,11 +286,8 @@ async function fetchChildrenForTestModuleNode(testModuleNode: TestModuleNode) {
         , "if the test module is not a declaration module, it must be the root module of some target node");
 
     const definitionUri = testModuleNode.definitionUri;
-    const rawRunnables = await RaApiHelper.getTestRunnablesInFile(definitionUri);
-
-    assert(!!rawRunnables);
 
-    const runnables = rawRunnables.map(it => new RunnableFacde(it));
+    const runnables = await getNormalizedTestRunnablesInFile(definitionUri);
 
     await updateModelByRunnables(testModuleNode, runnables);
 }
@@ -287,7 +312,8 @@ async function updateModelByRunnables(parentNode: TestModuleNode, runnables: Run
     // which means, when find the test item in test explorer, will refirect to declaration rather than the file
 
     // Handle testRunnables and test modules which have items, which are in the same test file
-    addTestModuleWithItemsRunnablesToTestModule(parentNode, [...withItemsModuleRunnables, ...testRunnables]);
+    addTestModuleWithItemsRunnablesToTestModule(parentNode, withItemsModuleRunnables);
+    addTestModuleWithItemsRunnablesToTestModule(parentNode, testRunnables);
 
     // Handle declarationModules
     // TODO: maybe concurrent?
@@ -305,7 +331,7 @@ async function addAndFetchDeclarationModuleRunnableToTestModule(parentNode: Test
         parentNode,
         declarationModuleRunnable.testOrSuiteName,
         declarationModuleRunnable.toTestLocation(),
-        vscode.Uri.file(definition.targetUri));
+        vscode.Uri.parse(definition.targetUri));
     parentNode.testChildren.add(testModule);
 
     // Fetch and update their definitions
@@ -320,7 +346,7 @@ function addTestModuleWithItemsRunnablesToTestModule(parentNode: TestModuleNode,
             const parentNode = testModelTree.findNearestNodeByRunnable(runnable);
             assert(parentNode.kind === NodeKind.TestModule, "Runable should be inserted into TestModule/Test, we create mock runnable for target/workspace node");
             if (!parentNode.isRootTestModule()) {
-                assert(parentNode.name === runnable.testPaths[runnable.testPaths.length - 1]);
+                assert(parentNode.name === runnable.testPaths[runnable.testPaths.length - 2]);
             }
 
             switch (runnable.testKind) {

editors/code/src/test_explorer/test_model_tree.ts

@@ -168,7 +168,7 @@ export class WorkspacesRoot {
 
         for (let index = 0; index < testPaths.length; index += 1) {
             const testModuleNmae = testPaths[index];
-            const targetKind = index !== testPaths.length - 1 ? testLevel : NodeKind.TestModule;
+            const targetKind = index === testPaths.length - 1 ? testLevel : NodeKind.TestModule;
 
             const candidate = Array.from(testModuleNode.testChildren).find((t) =>
                 t.kind === targetKind &&
@@ -344,7 +344,7 @@ export class TargetNode implements Node {
 
     static from(targetMetadata: CargoTargetMetadata, parent: CargoPackageNode): TargetNode | undefined {
         const targetKind = TargetKind.from(targetMetadata.kind);
-        if (!targetKind) return undefined;
+        if (targetKind === undefined) return undefined;
 
         const res = new TargetNode(parent, targetKind, targetMetadata.name, targetMetadata.src_path);
         return res;
@@ -362,7 +362,7 @@ export class TargetNode implements Node {
                 uri: this.srcPath,
                 range: new vscode.Range(0, 0, 0, 0),
             },
-            this.srcPath,);
+            this.srcPath);
     }
 }