diff --git a/java-extension/com.microsoft.java.test.plugin/src/main/java/com/microsoft/java/test/plugin/launchers/JUnitLaunchConfigurationDelegate.java b/java-extension/com.microsoft.java.test.plugin/src/main/java/com/microsoft/java/test/plugin/launchers/JUnitLaunchConfigurationDelegate.java index ddbfb262..65cfffbb 100644 --- a/java-extension/com.microsoft.java.test.plugin/src/main/java/com/microsoft/java/test/plugin/launchers/JUnitLaunchConfigurationDelegate.java +++ b/java-extension/com.microsoft.java.test.plugin/src/main/java/com/microsoft/java/test/plugin/launchers/JUnitLaunchConfigurationDelegate.java @@ -144,50 +144,75 @@ private void addTestItemArgs(List arguments) throws CoreException { arguments.add("-testNameFile"); arguments.add(fileName); } else if (this.args.testLevel == TestLevel.METHOD) { - arguments.add("-test"); - final IMethod method = (IMethod) JavaCore.create(this.args.testNames[0]); - String testName = method.getElementName(); - if ((this.args.testKind == TestKind.JUnit5 || this.args.testKind == TestKind.JUnit6) && - method.getParameters().length > 0) { - final ICompilationUnit unit = method.getCompilationUnit(); - if (unit == null) { + if (this.args.testNames.length > 1) { + if (!JUnitLaunchUtils.supportsMultiMethodLaunch()) { + // Bundled org.eclipse.jdt.junit.runtime predates eclipse.jdt.ui#2975. + // Fail fast with a marker so the TypeScript side can fall back to + // per-method launches transparently. throw new CoreException(new Status(IStatus.ERROR, JUnitPlugin.PLUGIN_ID, IStatus.ERROR, - "Cannot get compilation unit of method" + method.getElementName(), null)); //$NON-NLS-1$ + JUnitLaunchUtils.MULTI_METHOD_LAUNCH_UNSUPPORTED_PREFIX + + "Running multiple test methods together in a single JVM requires a newer " + + "Eclipse Java Language Server (org.eclipse.jdt.junit.runtime). " + + "Please update the 'Language Support for Java(TM) by Red Hat' " + + "extension and retry, or run the selected methods one at a time.", + null)); } - final CompilationUnit root = (CompilationUnit) TestSearchUtils.parseToAst(unit, - false /*fromCache*/, new NullProgressMonitor()); - final MethodDeclaration methodDeclaration = ASTNodeSearchUtil.getMethodDeclarationNode(method, root); - if (methodDeclaration == null) { - throw new CoreException(new Status(IStatus.ERROR, JUnitPlugin.PLUGIN_ID, IStatus.ERROR, - "Cannot get method declaration of method" + method.getElementName(), null)); //$NON-NLS-1$ + // Multi-method launch via "Class:method" lines: all selected methods share + // one JVM so per-class @BeforeAll/@AfterAll and cached fixtures (e.g. + // Spring ApplicationContext) are reused. See issue #1836. + final String fileName = createMethodTestNamesFile(this.args.testNames); + arguments.add("-testNameFile"); + arguments.add(fileName); + } else { + arguments.add("-test"); + arguments.add(resolveMethodTestName(this.args.testNames[0])); + + if (StringUtils.isNotBlank(this.args.uniqueId)) { + arguments.add("-uniqueId"); + arguments.add(this.args.uniqueId); } + } + } + } - final List parameters = new LinkedList<>(); - for (final Object obj : methodDeclaration.parameters()) { - if (obj instanceof SingleVariableDeclaration) { - final ITypeBinding paramTypeBinding = ((SingleVariableDeclaration) obj) - .getType().resolveBinding(); - if (paramTypeBinding == null) { - throw new CoreException(new Status(IStatus.ERROR, JUnitPlugin.PLUGIN_ID, IStatus.ERROR, - "Cannot set set argument for method" + methodDeclaration.toString(), null)); - } else if (paramTypeBinding.isPrimitive()) { - parameters.add(paramTypeBinding.getQualifiedName()); - } else { - parameters.add(paramTypeBinding.getBinaryName()); - } + private String resolveMethodTestName(String handleId) throws CoreException { + final IMethod method = (IMethod) JavaCore.create(handleId); + String testName = method.getElementName(); + if ((this.args.testKind == TestKind.JUnit5 || this.args.testKind == TestKind.JUnit6) && + method.getParameters().length > 0) { + final ICompilationUnit unit = method.getCompilationUnit(); + if (unit == null) { + throw new CoreException(new Status(IStatus.ERROR, JUnitPlugin.PLUGIN_ID, IStatus.ERROR, + "Cannot get compilation unit of method" + method.getElementName(), null)); //$NON-NLS-1$ + } + final CompilationUnit root = (CompilationUnit) TestSearchUtils.parseToAst(unit, + false /*fromCache*/, new NullProgressMonitor()); + final MethodDeclaration methodDeclaration = ASTNodeSearchUtil.getMethodDeclarationNode(method, root); + if (methodDeclaration == null) { + throw new CoreException(new Status(IStatus.ERROR, JUnitPlugin.PLUGIN_ID, IStatus.ERROR, + "Cannot get method declaration of method" + method.getElementName(), null)); //$NON-NLS-1$ + } + + final List parameters = new LinkedList<>(); + for (final Object obj : methodDeclaration.parameters()) { + if (obj instanceof SingleVariableDeclaration) { + final ITypeBinding paramTypeBinding = ((SingleVariableDeclaration) obj) + .getType().resolveBinding(); + if (paramTypeBinding == null) { + throw new CoreException(new Status(IStatus.ERROR, JUnitPlugin.PLUGIN_ID, IStatus.ERROR, + "Cannot set argument for method" + methodDeclaration.toString(), null)); + } else if (paramTypeBinding.isPrimitive()) { + parameters.add(paramTypeBinding.getQualifiedName()); + } else { + parameters.add(paramTypeBinding.getBinaryName()); } } - if (parameters.size() > 0) { - testName += "(" + String.join(",", parameters) + ")"; - } } - arguments.add(method.getDeclaringType().getFullyQualifiedName() + ':' + testName); - - if (StringUtils.isNotBlank(this.args.uniqueId)) { - arguments.add("-uniqueId"); - arguments.add(this.args.uniqueId); + if (parameters.size() > 0) { + testName += "(" + String.join(",", parameters) + ")"; } } + return method.getDeclaringType().getFullyQualifiedName() + ':' + testName; } private String createTestNamesFile(String[] testNames) throws CoreException { @@ -207,4 +232,22 @@ private String createTestNamesFile(String[] testNames) throws CoreException { IStatus.ERROR, JUnitPlugin.PLUGIN_ID, IStatus.ERROR, "", e)); //$NON-NLS-1$ } } + + private String createMethodTestNamesFile(String[] testNames) throws CoreException { + try { + final File file = File.createTempFile("testNames", ".txt"); //$NON-NLS-1$ //$NON-NLS-2$ + file.deleteOnExit(); + try (BufferedWriter bw = new BufferedWriter(new OutputStreamWriter( + new FileOutputStream(file), StandardCharsets.UTF_8));) { + for (final String handleId : testNames) { + bw.write(resolveMethodTestName(handleId)); + bw.newLine(); + } + } + return file.getAbsolutePath(); + } catch (IOException e) { + throw new CoreException(new Status( + IStatus.ERROR, JUnitPlugin.PLUGIN_ID, IStatus.ERROR, "", e)); //$NON-NLS-1$ + } + } } diff --git a/java-extension/com.microsoft.java.test.plugin/src/main/java/com/microsoft/java/test/plugin/launchers/JUnitLaunchUtils.java b/java-extension/com.microsoft.java.test.plugin/src/main/java/com/microsoft/java/test/plugin/launchers/JUnitLaunchUtils.java index a98dcc03..f09703a3 100644 --- a/java-extension/com.microsoft.java.test.plugin/src/main/java/com/microsoft/java/test/plugin/launchers/JUnitLaunchUtils.java +++ b/java-extension/com.microsoft.java.test.plugin/src/main/java/com/microsoft/java/test/plugin/launchers/JUnitLaunchUtils.java @@ -22,6 +22,7 @@ import org.eclipse.core.runtime.CoreException; import org.eclipse.core.runtime.IPath; import org.eclipse.core.runtime.IProgressMonitor; +import org.eclipse.core.runtime.Platform; import org.eclipse.debug.core.DebugPlugin; import org.eclipse.debug.core.ILaunchConfiguration; import org.eclipse.jdt.core.IJavaElement; @@ -34,6 +35,8 @@ import org.eclipse.jdt.launching.IRuntimeClasspathEntry; import org.eclipse.jdt.launching.JavaRuntime; import org.eclipse.jdt.ls.core.internal.ProjectUtils; +import org.osgi.framework.Bundle; +import org.osgi.framework.Version; import java.net.URISyntaxException; import java.util.ArrayList; @@ -51,8 +54,43 @@ public class JUnitLaunchUtils { private static final String JUNIT5_LOADER = "org.eclipse.jdt.junit.loader.junit5"; private static final String JUNIT4_LOADER = "org.eclipse.jdt.junit.loader.junit4"; + /** + * Bundle that hosts {@code RemoteTestRunner}, the consumer of {@code -testNameFile}. + * Shipped by the Eclipse Java Language Server, not by vscode-java-test itself. + */ + private static final String JUNIT_RUNTIME_BUNDLE = "org.eclipse.jdt.junit.runtime"; + + /** + * Marker prepended to the launch-resolution error when the bundled + * {@code org.eclipse.jdt.junit.runtime} is too old for the + * {@code Class:method} multi-method launch protocol. Detected by the + * TypeScript side to fall back to per-method launches. Keep in sync with + * the constant on the client side. + */ + public static final String MULTI_METHOD_LAUNCH_UNSUPPORTED_PREFIX = + "MULTI_METHOD_LAUNCH_UNSUPPORTED: "; + + /** + * Minimum {@code org.eclipse.jdt.junit.runtime} version that supports the + * {@code Class:method} multi-method launch protocol (eclipse.jdt.ui#2975). + */ + private static final Version MIN_JDT_JUNIT_RUNTIME_VERSION_FOR_MULTI_METHOD = + Version.parseVersion("3.8.100"); + private JUnitLaunchUtils() {} + /** + * @return {@code true} when the resolved {@code org.eclipse.jdt.junit.runtime} + * supports batching multiple methods into a single JVM via {@code -testNameFile}. + */ + public static boolean supportsMultiMethodLaunch() { + final Bundle bundle = Platform.getBundle(JUNIT_RUNTIME_BUNDLE); + if (bundle == null) { + return false; + } + return bundle.getVersion().compareTo(MIN_JDT_JUNIT_RUNTIME_VERSION_FOR_MULTI_METHOD) >= 0; + } + /** * Resolve the arguments to launch the Eclipse test runner * @param arguments diff --git a/src/constants.ts b/src/constants.ts index b23bf197..c0fc7c8d 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -66,6 +66,17 @@ export namespace Context { export const ACTIVATION_CONTEXT_KEY: string = 'java:testRunnerActivated'; } +export namespace JUnitLaunchProtocol { + /** + * Marker prepended to the launch-resolution error when the bundled JDT-LS + * predates the {@code Class:method} multi-method launch protocol + * (eclipse.jdt.ui#2975). Detected by the runner to fall back silently to + * per-method launches. Must match + * {@code JUnitLaunchUtils.MULTI_METHOD_LAUNCH_UNSUPPORTED_PREFIX} on the Java side. + */ + export const MULTI_METHOD_LAUNCH_UNSUPPORTED_PREFIX: string = 'MULTI_METHOD_LAUNCH_UNSUPPORTED: '; +} + /** * The different part keys returned by the JUnit test runner, * which are used to identify the test cases. diff --git a/src/controller/testController.ts b/src/controller/testController.ts index cdacd518..a2c20713 100644 --- a/src/controller/testController.ts +++ b/src/controller/testController.ts @@ -12,6 +12,7 @@ import { testSourceProvider } from '../provider/testSourceProvider'; import { BaseRunner } from '../runners/baseRunner/BaseRunner'; import { JUnitRunner } from '../runners/junitRunner/JunitRunner'; import { TestNGRunner } from '../runners/testngRunner/TestNGRunner'; +import { JUnitLaunchProtocol } from '../constants'; import { IJavaTestItem } from '../types'; import { loadRunConfig } from '../utils/configUtils'; import { resolveLaunchConfigurationForRunner } from '../utils/launchUtils'; @@ -260,8 +261,34 @@ export const runTests: (request: TestRunRequest, option: IRunOption) => any = in trackTestFrameworkVersion(testContext.kind, resolvedConfiguration.classPaths, resolvedConfiguration.modulePaths); await runner.run(resolvedConfiguration, token, option.progressReporter); } catch (error) { - window.showErrorMessage(error.message || 'Failed to run tests.'); - option.progressReporter?.done(); + const message: string = error?.message || 'Failed to run tests.'; + if (typeof error?.message === 'string' + && error.message.startsWith(JUnitLaunchProtocol.MULTI_METHOD_LAUNCH_UNSUPPORTED_PREFIX) + && testContext.testItems.length > 1) { + // Silent fallback for legacy JDT-LS (pre eclipse.jdt.ui#2975): + // re-launch every selected method in its own JVM. + delegatedToDebugger = true; + const itemsToRetry: TestItem[] = [...testContext.testItems]; + for (const item of itemsToRetry) { + if (token.isCancellationRequested) { + break; + } + // Each per-item launch hands progress to the debugger via + // __progressId; the debugger calls done() on the reporter + // on session end. Reset like the outer per-kind loop does (~line 240). + if (option.progressReporter?.isCancelled()) { + option.progressReporter = progressProvider?.createProgressReporter(option.isDebug ? 'Debug Tests' : 'Run Tests'); + } + await runItemInIsolatedLaunch(item, testContext, option, run, token); + } + } else { + const testMessage: TestMessage = new TestMessage(message); + for (const item of testContext.testItems) { + run.errored(item, testMessage); + } + window.showErrorMessage(message); + option.progressReporter?.done(); + } } finally { await runner.tearDown(); } @@ -595,11 +622,18 @@ function removeNonRerunTestInvocations(testItems: TestItem[]): void { } /** - * Eliminate the test methods if they are contained in the test class. - * Because the current test runner cannot run class and methods for the same time, - * in the returned array, all the classes are in one group and each method is a group. + * Eliminate the test methods if they are contained in the test class, then group the + * remaining method-level selections so they can share JVM launches where possible. + * + * The returned array is structured as: + * - The first group contains all class-level selections (run together). + * - Each subsequent group contains methods from the same parent class that can + * be launched in a single JVM, so per-class @BeforeAll/@AfterAll and cached + * fixtures (e.g. Spring ApplicationContext) are reused. See issue #1836. + * - Methods restricted to a single invocation (uniqueId) are kept in their own + * group, since the underlying protocol carries at most one uniqueId per JVM. */ -function mergeTestMethods(testItems: TestItem[]): TestItem[][] { +export function mergeTestMethods(testItems: TestItem[]): TestItem[][] { // export for unit test if (testItems.length <= 1) { return [testItems]; } @@ -650,8 +684,20 @@ function mergeTestMethods(testItems: TestItem[]): TestItem[][] { && !([...methods].some((m: TestItem) => dataCache.get(m)?.uniqueId))) { classMapping.set(clazz.id, clazz); } else { + // uniqueId methods must run alone (the protocol carries at most one + // uniqueId per JVM); the rest can share a JVM so @BeforeAll/@AfterAll + // and cached fixtures (e.g. Spring ApplicationContext) are reused. + // See issue #1836. + const groupable: TestItem[] = []; for (const method of methods.values()) { - testMethods.push([method]); + if (dataCache.get(method)?.uniqueId) { + testMethods.push([method]); + } else { + groupable.push(method); + } + } + if (groupable.length > 0) { + testMethods.push(groupable); } } } @@ -707,6 +753,47 @@ function getRunnerByContext(testContext: IRunTestContext): BaseRunner | undefine } } +/** + * Run a single test item through its own setup → resolve → run → tearDown cycle. + * Used as the silent fallback when the bundled JDT-LS lacks the + * {@code Class:method} multi-method protocol (eclipse.jdt.ui#2975). + */ +async function runItemInIsolatedLaunch( + item: TestItem, + parentContext: IRunTestContext, + option: IRunOption, + run: TestRun, + token: CancellationToken, +): Promise { + const singleContext: IRunTestContext = { + ...parentContext, + testItems: [item], + }; + const runner: BaseRunner | undefined = getRunnerByContext(singleContext); + if (!runner) { + run.errored(item, new TestMessage(`Failed to get suitable runner for the test kind: ${singleContext.kind}.`)); + return; + } + try { + await runner.setup(); + const resolvedConfiguration: DebugConfiguration = mergeConfigurations(option.launchConfiguration, singleContext.testConfig) + ?? await resolveLaunchConfigurationForRunner(runner, singleContext, singleContext.testConfig); + resolvedConfiguration.__progressId = option.progressReporter?.getId(); + trackTestFrameworkVersion(singleContext.kind, resolvedConfiguration.classPaths, resolvedConfiguration.modulePaths); + await runner.run(resolvedConfiguration, token, option.progressReporter); + } catch (error) { + const rawMessage: string = error?.message || 'Failed to run tests.'; + // Strip the internal marker if it ever bubbles up here so the popup stays user-readable. + const message: string = rawMessage.startsWith(JUnitLaunchProtocol.MULTI_METHOD_LAUNCH_UNSUPPORTED_PREFIX) + ? rawMessage.substring(JUnitLaunchProtocol.MULTI_METHOD_LAUNCH_UNSUPPORTED_PREFIX.length) + : rawMessage; + run.errored(item, new TestMessage(message)); + window.showErrorMessage(message); + } finally { + await runner.tearDown(); + } +} + function trackTestFrameworkVersion(testKind: TestKind, classpaths: string[], modulepaths: string[]) { let artifactPattern: RegExp; switch (testKind) { diff --git a/test/suite/testController.mergeTestMethods.test.ts b/test/suite/testController.mergeTestMethods.test.ts new file mode 100644 index 00000000..0f9ecb14 --- /dev/null +++ b/test/suite/testController.mergeTestMethods.test.ts @@ -0,0 +1,170 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. + +import * as assert from 'assert'; +import { TestController, TestItem, tests } from 'vscode'; +import { mergeTestMethods } from '../../src/controller/testController'; +import { dataCache } from '../../src/controller/testItemDataCache'; +import { setupTestEnv } from './utils'; +import { TestKind, TestLevel } from '../../src/java-test-runner.api'; + +function generateTestItem(testController: TestController, id: string, testLevel: TestLevel, uniqueId?: string): TestItem { + const testItem = testController.createTestItem(id, id + '_label'); + dataCache.set(testItem, { + jdtHandler: id + '_jdtHandler', + fullName: id + '_fullName', + projectName: id + '_projectName', + testLevel, + testKind: TestKind.JUnit5, + uniqueId, + }); + return testItem; +} + +suite('testController - mergeTestMethods', () => { + + let testController: TestController; + + suiteSetup(async function () { + await setupTestEnv(); + }); + + setup(() => { + testController = tests.createTestController('mergeTestMethodsTestController', 'mergeTestMethodsTestController'); + }); + + teardown(() => { + testController.dispose(); + }); + + test('should return the input untouched when empty', () => { + assert.deepStrictEqual(mergeTestMethods([]), [[]]); + }); + + test('should return the input untouched when a single item is selected', () => { + const method = generateTestItem(testController, 'id_1', TestLevel.Method); + assert.deepStrictEqual(mergeTestMethods([method]), [[method]]); + }); + + test('should batch methods of the same class into one launch group', () => { + const clazz = generateTestItem(testController, 'class_1', TestLevel.Class); + testController.items.add(clazz); + const method1 = generateTestItem(testController, 'method_1', TestLevel.Method); + const method2 = generateTestItem(testController, 'method_2', TestLevel.Method); + const method3 = generateTestItem(testController, 'method_3', TestLevel.Method); + clazz.children.add(method1); + clazz.children.add(method2); + clazz.children.add(method3); + + // select only 2 of 3 methods - they must share one launch (the core change for issue #1836) + const result = mergeTestMethods([method1, method2]); + + assert.deepStrictEqual(result, [[], [method1, method2]]); + }); + + test('should upgrade to a class launch when all methods of the class are selected', () => { + const clazz = generateTestItem(testController, 'class_1', TestLevel.Class); + testController.items.add(clazz); + const method1 = generateTestItem(testController, 'method_1', TestLevel.Method); + const method2 = generateTestItem(testController, 'method_2', TestLevel.Method); + clazz.children.add(method1); + clazz.children.add(method2); + + const result = mergeTestMethods([method1, method2]); + + assert.deepStrictEqual(result, [[clazz]]); + }); + + test('should not upgrade to a class launch when any selected method is restricted to a single invocation', () => { + const clazz = generateTestItem(testController, 'class_1', TestLevel.Class); + testController.items.add(clazz); + const method1 = generateTestItem(testController, 'method_1', TestLevel.Method, 'unique_1'); + const method2 = generateTestItem(testController, 'method_2', TestLevel.Method); + clazz.children.add(method1); + clazz.children.add(method2); + + const result = mergeTestMethods([method1, method2]); + + // method1 is invocation-restricted -> isolated; method2 batched alone in its own group + assert.deepStrictEqual(result, [[], [method1], [method2]]); + }); + + test('should isolate uniqueId methods but still batch the remaining methods of the same class', () => { + const clazz = generateTestItem(testController, 'class_1', TestLevel.Class); + testController.items.add(clazz); + const method1 = generateTestItem(testController, 'method_1', TestLevel.Method, 'unique_1'); + const method2 = generateTestItem(testController, 'method_2', TestLevel.Method); + const method3 = generateTestItem(testController, 'method_3', TestLevel.Method); + const method4 = generateTestItem(testController, 'method_4', TestLevel.Method); + clazz.children.add(method1); + clazz.children.add(method2); + clazz.children.add(method3); + clazz.children.add(method4); + + const result = mergeTestMethods([method1, method2, method3]); + + assert.deepStrictEqual(result, [[], [method1], [method2, method3]]); + }); + + test('should keep methods of different parent classes in their own launch groups', () => { + const classA = generateTestItem(testController, 'class_A', TestLevel.Class); + const classB = generateTestItem(testController, 'class_B', TestLevel.Class); + testController.items.add(classA); + testController.items.add(classB); + const a1 = generateTestItem(testController, 'a_1', TestLevel.Method); + const a2 = generateTestItem(testController, 'a_2', TestLevel.Method); + const b1 = generateTestItem(testController, 'b_1', TestLevel.Method); + const b2 = generateTestItem(testController, 'b_2', TestLevel.Method); + classA.children.add(a1); + classA.children.add(a2); + // make sure classA still has unselected children so it's not upgraded + classA.children.add(generateTestItem(testController, 'a_3', TestLevel.Method)); + classB.children.add(b1); + classB.children.add(b2); + classB.children.add(generateTestItem(testController, 'b_3', TestLevel.Method)); + + const result = mergeTestMethods([a1, b1, a2, b2]); + + assert.deepStrictEqual(result, [[], [a1, a2], [b1, b2]]); + }); + + test('should drop methods whose parent class is also selected', () => { + const clazz = generateTestItem(testController, 'class_1', TestLevel.Class); + testController.items.add(clazz); + const method1 = generateTestItem(testController, 'method_1', TestLevel.Method); + const method2 = generateTestItem(testController, 'method_2', TestLevel.Method); + clazz.children.add(method1); + clazz.children.add(method2); + + const result = mergeTestMethods([clazz, method1]); + + assert.deepStrictEqual(result, [[clazz]]); + }); + + test('should keep multiple selected classes together in the first group', () => { + const classA = generateTestItem(testController, 'class_A', TestLevel.Class); + const classB = generateTestItem(testController, 'class_B', TestLevel.Class); + testController.items.add(classA); + testController.items.add(classB); + + const result = mergeTestMethods([classA, classB]); + + assert.deepStrictEqual(result, [[classA, classB]]); + }); + + test('should mix class-level and unrelated method-level selections correctly', () => { + const classA = generateTestItem(testController, 'class_A', TestLevel.Class); + const classB = generateTestItem(testController, 'class_B', TestLevel.Class); + testController.items.add(classA); + testController.items.add(classB); + const b1 = generateTestItem(testController, 'b_1', TestLevel.Method); + const b2 = generateTestItem(testController, 'b_2', TestLevel.Method); + classB.children.add(b1); + classB.children.add(b2); + classB.children.add(generateTestItem(testController, 'b_3', TestLevel.Method)); + + const result = mergeTestMethods([classA, b1, b2]); + + assert.deepStrictEqual(result, [[classA], [b1, b2]]); + }); +});