diff --git a/.github/workflows/e2eUI.yml b/.github/workflows/e2eUI.yml index 96855ee7..89264f24 100644 --- a/.github/workflows/e2eUI.yml +++ b/.github/workflows/e2eUI.yml @@ -162,16 +162,6 @@ jobs: steps: - uses: actions/checkout@v4 - - name: Setup Build Environment (Xvfb) - run: | - sudo apt-get update - sudo apt-get install -y libxkbfile-dev pkg-config libsecret-1-dev libxss1 dbus xvfb libgtk-3-0 libgbm1 - # Use 1920x1080 so the Java Projects view (rendered inside the Explorer - # sidebar) gets enough vertical space. With 1024x768 the sticky - # pane-header overlapped tree rows and intercepted click events. - sudo /usr/bin/Xvfb :99 -screen 0 1920x1080x24 > /dev/null 2>&1 & - sleep 3 - - name: Set up JDK 21 uses: actions/setup-java@v4 with: @@ -181,7 +171,7 @@ jobs: - name: Setup Node.js environment uses: actions/setup-node@v4 with: - node-version: 20 + node-version: 22 - name: Setup autotest run: npm install -g @vscjava/vscode-autotest @@ -199,7 +189,9 @@ jobs: AZURE_OPENAI_API_KEY: ${{ secrets.AZURE_OPENAI_API_KEY }} AZURE_OPENAI_DEPLOYMENT: ${{ secrets.AZURE_OPENAI_DEPLOYMENT }} run: | - DISPLAY=:99 autotest run "test/e2e-plans/${{ matrix.plan }}.yaml" \ + # Use 1920x1080 so the Java Projects view gets enough vertical space. + xvfb-run -a -s "-screen 0 1920x1080x24" \ + autotest run "test/e2e-plans/${{ matrix.plan }}.yaml" \ --vsix "$(pwd)/vscode-java-dependency.vsix" \ --output "test-results/${{ matrix.plan }}" @@ -234,7 +226,7 @@ jobs: - name: Setup Node.js environment uses: actions/setup-node@v4 with: - node-version: 20 + node-version: 22 - name: Setup autotest run: npm install -g @vscjava/vscode-autotest @@ -272,7 +264,7 @@ jobs: - name: Setup Node.js environment uses: actions/setup-node@v4 with: - node-version: 20 + node-version: 22 - name: Setup autotest run: npm install -g @vscjava/vscode-autotest diff --git a/src/languageServerApi/languageServerApiManager.ts b/src/languageServerApi/languageServerApiManager.ts index 7affb88c..a89fe4ef 100644 --- a/src/languageServerApi/languageServerApiManager.ts +++ b/src/languageServerApi/languageServerApiManager.ts @@ -116,12 +116,16 @@ class LanguageServerApiManager { // Server is sending project data, so it's definitely running. // Mark as running so ready() returns immediately on subsequent calls. this.isServerRunning = true; - // During import, the JDTLS server is blocked by Eclipse workspace - // operations and cannot respond to queries. Instead of triggering - // a refresh (which queries the server), directly add projects to - // the tree view from the notification data. - const projectUris = uris.map(u => u.toString()); - commands.executeCommand(Commands.VIEW_PACKAGE_INTERNAL_ADD_PROJECTS, projectUris); + if (this.isServerReady) { + commands.executeCommand(Commands.VIEW_PACKAGE_INTERNAL_REFRESH, /* debounce = */true); + } else { + // During import, the JDTLS server is blocked by Eclipse workspace + // operations and cannot respond to queries. Instead of triggering + // a refresh (which queries the server), directly add projects to + // the tree view from the notification data. + const projectUris = uris.map(u => u.toString()); + commands.executeCommand(Commands.VIEW_PACKAGE_INTERNAL_ADD_PROJECTS, projectUris); + } syncHandler.updateFileWatcher(Settings.autoRefresh()); })); } diff --git a/src/views/dependencyDataProvider.ts b/src/views/dependencyDataProvider.ts index 626af103..a8b192d6 100644 --- a/src/views/dependencyDataProvider.ts +++ b/src/views/dependencyDataProvider.ts @@ -2,6 +2,7 @@ // Licensed under the MIT license. import * as _ from "lodash"; +import * as path from "path"; import { commands, Event, EventEmitter, ExtensionContext, ProviderResult, RelativePattern, TreeDataProvider, TreeItem, Uri, window, workspace, @@ -226,7 +227,10 @@ export class DependencyDataProvider implements TreeDataProvider { */ public addProgressiveProjects(projectUris: string[]): void { const folders = workspace.workspaceFolders; - if (!folders || !folders.length) { + // Multi-root workspaces use WorkspaceNode roots. Those roots can remain + // cached briefly after switching to a single folder, so wait for the + // full refresh rather than creating a mixed root structure. + if (!folders || folders.length !== 1 || this._rootItems?.some(root => root instanceof WorkspaceNode)) { return; } @@ -238,11 +242,14 @@ export class DependencyDataProvider implements TreeDataProvider { this._rootItems .filter((n): n is ProjectNode => n instanceof ProjectNode) .map((n) => n.uri) + .filter((uri): uri is string => Boolean(uri)) + .map(getProjectUriKey) ); let added = false; for (const uriStr of projectUris) { - if (existingUris.has(uriStr)) { + const uriKey = getProjectUriKey(uriStr); + if (existingUris.has(uriKey)) { continue; } // Extract project name from URI (last non-empty path segment) @@ -253,7 +260,7 @@ export class DependencyDataProvider implements TreeDataProvider { kind: NodeKind.Project, }; this._rootItems.push(new ProjectNode(nodeData, undefined)); - existingUris.add(uriStr); + existingUris.add(uriKey); added = true; } @@ -308,3 +315,16 @@ export class DependencyDataProvider implements TreeDataProvider { } } } + +function getProjectUriKey(uriString: string): string { + const uri = Uri.parse(uriString); + if (uri.scheme !== "file") { + return uri.toString(); + } + + let fsPath = path.normalize(uri.fsPath); + if (fsPath !== path.parse(fsPath).root) { + fsPath = fsPath.replace(/[\\\/]+$/, ""); + } + return process.platform === "win32" ? fsPath.toLowerCase() : fsPath; +} diff --git a/test/e2e-plans/java-dep-project-explorer.yaml b/test/e2e-plans/java-dep-project-explorer.yaml index 7e198f6b..73854cf6 100644 --- a/test/e2e-plans/java-dep-project-explorer.yaml +++ b/test/e2e-plans/java-dep-project-explorer.yaml @@ -152,3 +152,79 @@ steps: name: "App" exact: true timeout: 15 + + # ── Test 5: mixed multi-root project attribution (#1060) ── + # First establish a mixed workspace and refresh it into WorkspaceNode roots. + # The smoke-test driver renders folder pickers as an internal quick input: + # entering an absolute folder path opens it, then the Add button confirms it. + - id: "invoke-add-non-java-root" + action: "executeVSCodeCommand workbench.action.addRootFolder" + + - id: "type-non-java-root" + action: "fillQuickInput ${workspaceParent}/non-java" + + - id: "confirm-non-java-root" + action: "tryClickButton Add" + + - id: "wait-non-java-root-ready" + action: "waitForLanguageServer" + timeout: 120 + skipLlmVerify: true + + - id: "refresh-mixed-workspace" + action: "executeVSCodeCommand java.view.package.refresh" + waitBefore: 2 + + - id: "collapse-multi-root-explorer" + action: "collapseSidebarSection UNTITLED (WORKSPACE)" + + - id: "focus-mixed-workspace" + action: "executeVSCodeCommand javaProjectExplorer.focus" + waitBefore: 2 + + - id: "verify-non-java-workspace-root" + action: "wait 1 seconds" + verifyTreeItem: + name: "non-java" + timeout: 15 + + # Add a Java folder after the mixed multi-root structure already exists and + # refresh it into the expected WorkspaceNode -> ProjectNode hierarchy. + - id: "invoke-add-java-root" + action: "executeVSCodeCommand workbench.action.addRootFolder" + + - id: "type-java-root" + action: "fillQuickInput ${workspaceParent}/simple" + + - id: "confirm-java-root" + action: "tryClickButton Add" + + - id: "wait-java-root-ready" + action: "waitForLanguageServer" + timeout: 120 + skipLlmVerify: true + + - id: "refresh-added-java-root" + action: "executeVSCodeCommand java.view.package.refresh" + + - id: "verify-java-workspace-root" + action: "executeVSCodeCommand javaProjectExplorer.focus" + verifyTreeItem: + name: "simple" + exact: true + count: 1 + level: 1 + timeout: 15 + + # Deterministically simulate the onDidProjectsImport path. Before #1060 this + # command appended a second top-level ProjectNode named "simple" beside the + # existing WorkspaceNode. The fixed provider ignores progressive insertion + # in multi-root workspaces, so exactly one level-1 row remains. + - id: "simulate-progressive-project-import" + action: 'executeVSCodeCommand _java.view.package.internal.addProjects ["${workspaceParentUri}/simple"]' + verifyTreeItem: + name: "simple" + exact: true + count: 1 + level: 1 + timeout: 15 diff --git a/test/index.ts b/test/index.ts index f9db1d22..a08d7358 100644 --- a/test/index.ts +++ b/test/index.ts @@ -98,6 +98,17 @@ async function main(): Promise { ], }); + // Run multi-root workspace test + await runTests({ + vscodeExecutablePath, + extensionDevelopmentPath, + extensionTestsPath: path.resolve(__dirname, "./multiple-suite"), + launchArgs: [ + path.join(__dirname, "..", "..", "test", "multiple", "multiple-project.code-workspace"), + `--user-data-dir=${userDir}`, + ], + }); + // Run test for non-Java Gradle project (regression test for #921) await runTests({ vscodeExecutablePath, diff --git a/test/maven-suite/projectView.test.ts b/test/maven-suite/projectView.test.ts index c3999802..f5c545f4 100644 --- a/test/maven-suite/projectView.test.ts +++ b/test/maven-suite/projectView.test.ts @@ -277,6 +277,24 @@ suite("Maven Project View Tests", () => { assert.equal(mavenChildren[1].getDisplayName(), "junit:junit:4.13.1"); }); + test("Does not add duplicate progressive projects for equivalent URIs", async function() { + const explorer = DependencyExplorer.getInstance(contextManager.context); + await vscode.commands.executeCommand(Commands.VIEW_PACKAGE_REFRESH); + + const roots = await explorer.dataProvider.getChildren(); + assert.equal(roots?.length, 1, "Number of root nodes should be 1"); + const projectNode = roots![0] as ProjectNode; + assert.ok(projectNode.uri, "Project node should have a URI"); + + const equivalentUri = projectNode.uri!.endsWith("/") + ? projectNode.uri!.replace(/\/+$/, "") + : `${projectNode.uri}/`; + explorer.dataProvider.addProgressiveProjects([equivalentUri]); + + const updatedRoots = await explorer.dataProvider.getChildren(); + assert.equal(updatedRoots?.length, 1, "Equivalent project URIs should be deduplicated"); + }); + teardown(async () => { // Restore default settings. Some tests might alter them and others depend on a specific setting. // Not resetting to the default settings will also show the file as changed in the source control view. diff --git a/test/multiple-suite/index.ts b/test/multiple-suite/index.ts new file mode 100644 index 00000000..cfc957e9 --- /dev/null +++ b/test/multiple-suite/index.ts @@ -0,0 +1,38 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. + +import * as glob from "glob"; +import * as Mocha from "mocha"; +import * as path from "path"; + +export function run(): Promise { + const mocha = new Mocha({ + ui: "tdd", + color: true, + timeout: 1 * 60 * 1000, + }); + + const testsRoot = __dirname; + + return new Promise((c, e) => { + glob("**/**.test.js", { cwd: testsRoot }, (err, files) => { + if (err) { + return e(err); + } + + files.forEach((f) => mocha.addFile(path.resolve(testsRoot, f))); + + try { + mocha.run((failures) => { + if (failures > 0) { + e(new Error(`${failures} tests failed.`)); + } else { + c(); + } + }); + } catch (err) { + e(err); + } + }); + }); +} diff --git a/test/multiple-suite/projectView.test.ts b/test/multiple-suite/projectView.test.ts new file mode 100644 index 00000000..2e10c3db --- /dev/null +++ b/test/multiple-suite/projectView.test.ts @@ -0,0 +1,99 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. + +import * as assert from "assert"; +import * as vscode from "vscode"; +import { + Commands, contextManager, DependencyExplorer, ProjectNode, WorkspaceNode, +} from "../../extension.bundle"; +import { setupTestEnv } from "../shared"; + +// tslint:disable: only-arrow-functions +suite("Multiple Project View Tests", () => { + + suiteSetup(async () => { + await setupTestEnv(); + const javaExtension = vscode.extensions.getExtension("redhat.java"); + assert.ok(javaExtension, "Language Support for Java should be installed"); + const javaApi = await javaExtension!.activate(); + await javaApi.serverReady(); + await vscode.commands.executeCommand(Commands.VIEW_PACKAGE_REFRESH); + }); + + test("Does not add project roots progressively in a multi-root workspace", async function() { + const explorer = DependencyExplorer.getInstance(contextManager.context); + const roots = await explorer.dataProvider.getChildren(); + const expectedRootCount = vscode.workspace.workspaceFolders?.length || 0; + + assert.equal(roots?.length, expectedRootCount, "Each workspace folder should have one root node"); + assert.ok(roots?.every(root => root instanceof WorkspaceNode), "All roots should be workspace nodes"); + const nonJavaRoot = roots?.find(root => + root instanceof WorkspaceNode && root.name === "non-java") as WorkspaceNode | undefined; + assert.ok(nonJavaRoot, "The non-Java workspace folder should have a root node"); + assert.equal((await nonJavaRoot!.getChildren()).length, 0, "The non-Java root should not contain Java projects"); + + const projects = await explorer.dataProvider.getRootProjects(); + const project = projects.find((node): node is ProjectNode => + node instanceof ProjectNode && Boolean(node.uri)); + assert.ok(project?.uri, "At least one Java project should be available"); + + explorer.dataProvider.addProgressiveProjects([project!.uri!]); + + const updatedRoots = await explorer.dataProvider.getChildren(); + assert.equal(updatedRoots?.length, expectedRootCount, "Progressive updates should not add project roots"); + assert.ok(updatedRoots?.every(root => root instanceof WorkspaceNode), "All roots should remain workspace nodes"); + }); + + test("Does not add project roots while cached multi-root roots are stale", async function() { + const explorer = DependencyExplorer.getInstance(contextManager.context); + const roots = await explorer.dataProvider.getChildren(); + const folders = vscode.workspace.workspaceFolders; + assert.ok(folders && folders.length > 1, "The test requires a multi-root workspace"); + assert.ok(roots?.every(root => root instanceof WorkspaceNode), "All cached roots should be workspace nodes"); + + const projects = await explorer.dataProvider.getRootProjects(); + const project = projects.find((node): node is ProjectNode => + node instanceof ProjectNode && Boolean(node.uri)); + assert.ok(project?.uri, "At least one Java project should be available"); + + const removedFolders = folders!.slice(1); + const workspaceFoldersChanged = updateWorkspaceFoldersAndWait(1, removedFolders.length, [], + "The workspace should switch to a single folder"); + + try { + assert.equal(vscode.workspace.workspaceFolders?.length, 1, "The workspace should have one folder"); + explorer.dataProvider.addProgressiveProjects([project!.uri!]); + + const updatedRoots = await explorer.dataProvider.getChildren(); + assert.equal(updatedRoots?.length, roots?.length, "Stale cached roots should not be mixed with project roots"); + assert.ok(updatedRoots?.every(root => root instanceof WorkspaceNode), + "Cached workspace roots should remain unchanged until refresh"); + } finally { + await workspaceFoldersChanged; + await updateWorkspaceFoldersAndWait(1, 0, + removedFolders.map(folder => ({ uri: folder.uri })), + "The removed workspace folders should be restored"); + } + }); +}); + +async function updateWorkspaceFoldersAndWait( + start: number, + deleteCount: number, + foldersToAdd: { uri: vscode.Uri; name?: string }[], + failureMessage: string, +): Promise { + let resolveChange: () => void; + const changed = new Promise((resolve) => resolveChange = resolve); + const listener = vscode.workspace.onDidChangeWorkspaceFolders(() => { + listener.dispose(); + resolveChange(); + }); + + if (!vscode.workspace.updateWorkspaceFolders(start, deleteCount, ...foldersToAdd)) { + listener.dispose(); + assert.fail(failureMessage); + } + + await changed; +} diff --git a/test/multiple/multiple-project.code-workspace b/test/multiple/multiple-project.code-workspace index 19610b08..6189dcb6 100644 --- a/test/multiple/multiple-project.code-workspace +++ b/test/multiple/multiple-project.code-workspace @@ -1,13 +1,16 @@ { "folders": [ { - "path": "..\\simple" + "path": "../simple" }, { - "path": "..\\maven" + "path": "../maven" }, { - "path": "..\\gradle" + "path": "../gradle" + }, + { + "path": "../non-java" } ], "settings": {} diff --git a/test/non-java/package.json b/test/non-java/package.json new file mode 100644 index 00000000..030de1b7 --- /dev/null +++ b/test/non-java/package.json @@ -0,0 +1,4 @@ +{ + "name": "non-java", + "private": true +}