Skip to content

Commit 558d444

Browse files
committed
perf: shorten commetn
1 parent 34fdf4c commit 558d444

4 files changed

Lines changed: 33 additions & 70 deletions

File tree

java-extension/com.microsoft.java.test.plugin/src/main/java/com/microsoft/java/test/plugin/launchers/JUnitLaunchConfigurationDelegate.java

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -146,16 +146,9 @@ private void addTestItemArgs(List<String> arguments) throws CoreException {
146146
} else if (this.args.testLevel == TestLevel.METHOD) {
147147
if (this.args.testNames.length > 1) {
148148
if (!JUnitLaunchUtils.supportsMultiMethodLaunch()) {
149-
// The Class:method protocol is parsed by RemoteTestRunner inside
150-
// org.eclipse.jdt.junit.runtime, which ships with the Eclipse Java
151-
// Language Server (Language Support for Java(TM) by Red Hat). When
152-
// that bundle predates eclipse.jdt.ui#2975, batching multiple
153-
// methods into a single JVM would surface as a ClassNotFoundException
154-
// at test time. Fail fast here with a marker the TypeScript side
155-
// recognises so it can transparently fall back to launching each
156-
// method in its own JVM (the legacy per-method path). The actionable
157-
// text after the marker is preserved as a defensive fallback in case
158-
// the fallback path itself also fails for an unrelated reason.
149+
// Bundled org.eclipse.jdt.junit.runtime predates eclipse.jdt.ui#2975.
150+
// Fail fast with a marker so the TypeScript side can fall back to
151+
// per-method launches transparently.
159152
throw new CoreException(new Status(IStatus.ERROR, JUnitPlugin.PLUGIN_ID, IStatus.ERROR,
160153
JUnitLaunchUtils.MULTI_METHOD_LAUNCH_UNSUPPORTED_PREFIX
161154
+ "Running multiple test methods together in a single JVM requires a newer "
@@ -164,11 +157,9 @@ private void addTestItemArgs(List<String> arguments) throws CoreException {
164157
+ "extension and retry, or run the selected methods one at a time.",
165158
null));
166159
}
167-
// Multi-method launch: hand the full selection to RemoteTestRunner via
168-
// -testNameFile using the new "Class:method" line format. The runner
169-
// will then load every selected method inside a single test JVM, so
170-
// per-class @BeforeAll/@AfterAll and any cached Spring
171-
// ApplicationContext are reused across the selection.
160+
// Multi-method launch via "Class:method" lines: all selected methods share
161+
// one JVM so per-class @BeforeAll/@AfterAll and cached fixtures (e.g.
162+
// Spring ApplicationContext) are reused. See issue #1836.
172163
final String fileName = createMethodTestNamesFile(this.args.testNames);
173164
arguments.add("-testNameFile");
174165
arguments.add(fileName);

java-extension/com.microsoft.java.test.plugin/src/main/java/com/microsoft/java/test/plugin/launchers/JUnitLaunchUtils.java

Lines changed: 10 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -55,30 +55,24 @@ public class JUnitLaunchUtils {
5555
private static final String JUNIT4_LOADER = "org.eclipse.jdt.junit.loader.junit4";
5656

5757
/**
58-
* Bundle that hosts {@code RemoteTestRunner}, the consumer of the
59-
* {@code -testNameFile} content. This jar is shipped by the Eclipse Java
60-
* Language Server (i.e. the "Language Support for Java(TM) by Red Hat"
61-
* extension), not by vscode-java-test itself.
58+
* Bundle that hosts {@code RemoteTestRunner}, the consumer of {@code -testNameFile}.
59+
* Shipped by the Eclipse Java Language Server, not by vscode-java-test itself.
6260
*/
6361
private static final String JUNIT_RUNTIME_BUNDLE = "org.eclipse.jdt.junit.runtime";
6462

6563
/**
66-
* Stable prefix prepended to the {@link CoreException} thrown when the
67-
* resolved {@code org.eclipse.jdt.junit.runtime} bundle is too old to
68-
* understand the {@code Class:method} multi-method launch protocol. The
69-
* TypeScript side detects this prefix and silently falls back to launching
70-
* every selected method in its own JVM (the legacy per-method path),
71-
* keeping the user experience identical to the pre-batching behaviour on
72-
* older Eclipse Java Language Server releases. Do NOT change the value
73-
* without updating the corresponding constant on the client side.
64+
* Marker prepended to the launch-resolution error when the bundled
65+
* {@code org.eclipse.jdt.junit.runtime} is too old for the
66+
* {@code Class:method} multi-method launch protocol. Detected by the
67+
* TypeScript side to fall back to per-method launches. Keep in sync with
68+
* the constant on the client side.
7469
*/
7570
public static final String MULTI_METHOD_LAUNCH_UNSUPPORTED_PREFIX =
7671
"MULTI_METHOD_LAUNCH_UNSUPPORTED: ";
7772

7873
/**
79-
* Minimum {@code org.eclipse.jdt.junit.runtime} version that recognises the
80-
* {@code Class:method} multi-method launch protocol introduced by
81-
* <a href="https://github.com/eclipse-jdt/eclipse.jdt.ui/pull/2975">eclipse.jdt.ui#2975</a>.
74+
* Minimum {@code org.eclipse.jdt.junit.runtime} version that supports the
75+
* {@code Class:method} multi-method launch protocol (eclipse.jdt.ui#2975).
8276
*/
8377
private static final Version MIN_JDT_JUNIT_RUNTIME_VERSION_FOR_MULTI_METHOD =
8478
Version.parseVersion("3.8.100");
@@ -87,11 +81,7 @@ private JUnitLaunchUtils() {}
8781

8882
/**
8983
* @return {@code true} when the resolved {@code org.eclipse.jdt.junit.runtime}
90-
* bundle is new enough to parse the {@code Class:method} multi-method launch
91-
* protocol; {@code false} otherwise. When this returns {@code false}, callers
92-
* must not batch multiple methods into a single JVM via the
93-
* {@code -testNameFile} mechanism — the legacy per-method launch path should
94-
* be used instead.
84+
* supports batching multiple methods into a single JVM via {@code -testNameFile}.
9585
*/
9686
public static boolean supportsMultiMethodLaunch() {
9787
final Bundle bundle = Platform.getBundle(JUNIT_RUNTIME_BUNDLE);

src/constants.ts

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -68,16 +68,11 @@ export namespace Context {
6868

6969
export namespace JUnitLaunchProtocol {
7070
/**
71-
* Stable prefix prepended to the launch-resolution error thrown by the
72-
* Java plugin when the bundled {@code org.eclipse.jdt.junit.runtime}
71+
* Marker prepended to the launch-resolution error when the bundled JDT-LS
7372
* predates the {@code Class:method} multi-method launch protocol
74-
* introduced by eclipse.jdt.ui#2975. The TypeScript runner detects this
75-
* prefix and silently re-launches every selected method in its own JVM
76-
* (the legacy per-method path), keeping the user experience identical to
77-
* the pre-batching behaviour on older Eclipse Java Language Server
78-
* releases. Keep in sync with
79-
* {@code JUnitLaunchUtils.MULTI_METHOD_LAUNCH_UNSUPPORTED_PREFIX} on the
80-
* Java side.
73+
* (eclipse.jdt.ui#2975). Detected by the runner to fall back silently to
74+
* per-method launches. Must match
75+
* {@code JUnitLaunchUtils.MULTI_METHOD_LAUNCH_UNSUPPORTED_PREFIX} on the Java side.
8176
*/
8277
export const MULTI_METHOD_LAUNCH_UNSUPPORTED_PREFIX: string = 'MULTI_METHOD_LAUNCH_UNSUPPORTED: ';
8378
}

src/controller/testController.ts

Lines changed: 13 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -265,22 +265,17 @@ export const runTests: (request: TestRunRequest, option: IRunOption) => any = in
265265
if (typeof error?.message === 'string'
266266
&& error.message.startsWith(JUnitLaunchProtocol.MULTI_METHOD_LAUNCH_UNSUPPORTED_PREFIX)
267267
&& testContext.testItems.length > 1) {
268-
// Silent fallback for older Eclipse Java Language Server
269-
// releases (predating eclipse.jdt.ui#2975): re-launch every
270-
// selected method in its own JVM. From here on the cancel
271-
// handler must defer to the per-item debug sessions, just
272-
// as it would for a normal multi-group run.
268+
// Silent fallback for legacy JDT-LS (pre eclipse.jdt.ui#2975):
269+
// re-launch every selected method in its own JVM.
273270
delegatedToDebugger = true;
274271
const itemsToRetry: TestItem[] = [...testContext.testItems];
275272
for (const item of itemsToRetry) {
276273
if (token.isCancellationRequested) {
277274
break;
278275
}
279-
// Each per-item launch hands its progress to the debugger
280-
// via __progressId, and the debugger calls done() when the
281-
// session ends. The next iteration must therefore obtain a
282-
// fresh progress reporter — mirroring the same isCancelled
283-
// reset that the outer per-kind loop performs at line ~240.
276+
// Each per-item launch hands progress to the debugger via
277+
// __progressId; the debugger dones the reporter on session
278+
// end. Reset like the outer per-kind loop does (~line 240).
284279
if (option.progressReporter?.isCancelled()) {
285280
option.progressReporter = progressProvider?.createProgressReporter(option.isDebug ? 'Debug Tests' : 'Run Tests');
286281
}
@@ -682,11 +677,10 @@ export function mergeTestMethods(testItems: TestItem[]): TestItem[][] { // expor
682677
&& !([...methods].some((m: TestItem) => dataCache.get(m)?.uniqueId))) {
683678
classMapping.set(clazz.id, clazz);
684679
} else {
685-
// Methods restricted to a single invocation (uniqueId) must still run in their
686-
// own launch since the underlying protocol carries at most one uniqueId per JVM.
687-
// Every other method of the same class can share one launch so that
688-
// @BeforeAll / @AfterAll and any cached fixture (e.g. Spring ApplicationContext)
689-
// are reused across the selection. See issue #1836.
680+
// uniqueId methods must run alone (the protocol carries at most one
681+
// uniqueId per JVM); the rest can share a JVM so @BeforeAll/@AfterAll
682+
// and cached fixtures (e.g. Spring ApplicationContext) are reused.
683+
// See issue #1836.
690684
const groupable: TestItem[] = [];
691685
for (const method of methods.values()) {
692686
if (dataCache.get(method)?.uniqueId) {
@@ -753,14 +747,9 @@ function getRunnerByContext(testContext: IRunTestContext): BaseRunner | undefine
753747
}
754748

755749
/**
756-
* Run a single test item through its own setup → resolve → run → tearDown
757-
* cycle. Used as a silent fallback when the bundled JDT-LS does not yet
758-
* understand the {@code Class:method} multi-method launch protocol
759-
* (eclipse.jdt.ui#2975) — every selected method is re-launched in its own JVM,
760-
* matching the pre-batching behaviour. Errors during the fallback are
761-
* surfaced per-item so unrelated failures (e.g. compilation errors) are still
762-
* reported to the user; the multi-method marker itself is filtered out so the
763-
* scary "requires a newer..." text never reaches the popup.
750+
* Run a single test item through its own setup → resolve → run → tearDown cycle.
751+
* Used as the silent fallback when the bundled JDT-LS lacks the
752+
* {@code Class:method} multi-method protocol (eclipse.jdt.ui#2975).
764753
*/
765754
async function runItemInIsolatedLaunch(
766755
item: TestItem,
@@ -787,9 +776,7 @@ async function runItemInIsolatedLaunch(
787776
await runner.run(resolvedConfiguration, token, option.progressReporter);
788777
} catch (error) {
789778
const rawMessage: string = error?.message || 'Failed to run tests.';
790-
// Strip the internal marker if it ever bubbles up here (e.g. the user
791-
// somehow ends up with a single-method batch that still hits the
792-
// capability gate) so the popup stays user-readable.
779+
// Strip the internal marker if it ever bubbles up here so the popup stays user-readable.
793780
const message: string = rawMessage.startsWith(JUnitLaunchProtocol.MULTI_METHOD_LAUNCH_UNSUPPORTED_PREFIX)
794781
? rawMessage.substring(JUnitLaunchProtocol.MULTI_METHOD_LAUNCH_UNSUPPORTED_PREFIX.length)
795782
: rawMessage;

0 commit comments

Comments
 (0)