Copilot commented on code in PR #2517:
URL: https://github.com/apache/groovy/pull/2517#discussion_r3191605010


##########
subprojects/groovy-test-junit6/src/main/java/groovy/junit6/plugin/ExpectedToFailExtension.java:
##########
@@ -0,0 +1,146 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package groovy.junit6.plugin;
+
+import org.junit.jupiter.api.extension.ExtensionContext;
+import org.junit.jupiter.api.extension.InvocationInterceptor;
+import org.junit.jupiter.api.extension.ReflectiveInvocationContext;
+import org.opentest4j.TestAbortedException;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Method;
+
+/**
+ * JUnit 5 {@link InvocationInterceptor} backing the {@link ExpectedToFail}
+ * annotation. Inverts a test's pass/fail outcome with optional exception-type
+ * and message-substring filters.
+ * <p>
+ * Composes with {@link ForkedJvm}: the inversion runs in whichever JVM layer
+ * is the natural outermost one for the method's annotation order (see
+ * {@link ExpectedToFail} javadoc).
+ *
+ * @since 6.0.0
+ */
+public class ExpectedToFailExtension implements InvocationInterceptor {
+
+    @Override
+    public void interceptTestMethod(Invocation<Void> invocation,
+                                    ReflectiveInvocationContext<Method> 
invocationContext,
+                                    ExtensionContext extensionContext) throws 
Throwable {
+        ExpectedToFail config = findAnnotation(extensionContext);
+        if (config == null) {
+            invocation.proceed();
+            return;
+        }
+        Method method = invocationContext.getExecutable();
+        if (shouldDeferToParent(method)) {
+            // We're in a forked child whose parent JVM has @ExpectedToFail
+            // wrapping @ForkedJvm; the parent will see the propagated outcome
+            // and do the inversion. Pass through honestly here.
+            invocation.proceed();
+            return;
+        }
+        Throwable thrown = null;
+        try {
+            invocation.proceed();
+        } catch (TestAbortedException tae) {
+            // Assumption failures never count as the expected failure.
+            throw tae;
+        } catch (Throwable t) {
+            thrown = t;
+        }
+        evaluateOutcome(thrown, config);
+    }
+
+    /**
+     * Returns true iff the inversion should be performed by the parent JVM's
+     * instance of this extension (rather than at the current layer).
+     * <p>
+     * That happens when all of:
+     * <ul>
+     *   <li>we're running inside a {@link ForkedJvm} child JVM (the
+     *       {@code FORKED_FLAG} sentinel is set),</li>
+     *   <li>the test method (or its declaring class) carries a
+     *       {@code @ForkedJvm} annotation, and</li>
+     *   <li>{@code @ExpectedToFail} is declared <em>before</em>
+     *       {@code @ForkedJvm} in source — i.e., it wraps
+     *       {@code @ForkedJvm}'s interceptor in the parent and will see the
+     *       propagated outcome.</li>
+     * </ul>
+     */
+    private static boolean shouldDeferToParent(Method method) {
+        if 
(!Boolean.parseBoolean(System.getProperty(ForkedJvmTestRunner.FORKED_FLAG))) {
+            return false;
+        }
+        if (!annotationOnMethodOrClass(method, ForkedJvm.class)) {
+            return false;
+        }
+        return isExpectedToFailDeclaredBeforeForkedJvm(method);
+    }
+
+    private static boolean annotationOnMethodOrClass(Method method, Class<? 
extends Annotation> a) {
+        return method.isAnnotationPresent(a) || 
method.getDeclaringClass().isAnnotationPresent(a);
+    }
+
+    private static boolean isExpectedToFailDeclaredBeforeForkedJvm(Method 
method) {
+        // Method-level annotations take precedence; check those first.
+        Boolean methodOrder = orderOf(method.getAnnotations());
+        if (methodOrder != null) return methodOrder;
+        // Otherwise consult the declaring class.
+        Boolean classOrder = 
orderOf(method.getDeclaringClass().getAnnotations());
+        return classOrder != null && classOrder;
+    }
+
+    private static Boolean orderOf(Annotation[] annotations) {
+        for (Annotation a : annotations) {
+            Class<?> type = a.annotationType();
+            if (type == ExpectedToFail.class) return true;
+            if (type == ForkedJvm.class) return false;
+        }
+        return null;
+    }

Review Comment:
   This logic relies on the iteration order of `Method#getAnnotations()` / 
`Class#getAnnotations()` to reflect source declaration order, but the Java 
reflection API does not guarantee annotation ordering. This can make the 
forked-vs-parent inversion decision non-deterministic across JVMs/compilers. 
Consider replacing the ordering-based behavior with a deterministic rule (e.g., 
always invert in the parent when both annotations are present), or introduce an 
explicit attribute/config flag to select where inversion occurs when combined 
with `@ForkedJvm`.



##########
subprojects/groovy-test-junit6/src/main/java/groovy/junit6/plugin/ForkedJvmTestRunner.java:
##########
@@ -0,0 +1,125 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package groovy.junit6.plugin;
+
+import org.junit.platform.engine.discovery.DiscoverySelectors;
+import org.junit.platform.launcher.Launcher;
+import org.junit.platform.launcher.LauncherDiscoveryRequest;
+import org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder;
+import org.junit.platform.launcher.core.LauncherFactory;
+import org.junit.platform.launcher.listeners.SummaryGeneratingListener;
+import org.junit.platform.launcher.listeners.TestExecutionSummary;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.ObjectOutputStream;
+import java.io.OutputStream;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+
+/**
+ * Child-JVM entry point for {@link ForkedJvm}-annotated tests.
+ * <p>
+ * Invoked by {@link ForkedJvmExtension} with the qualified test class name
+ * and method name as command-line arguments. Runs exactly that one test method
+ * via the JUnit Platform {@link Launcher}, then reports outcome to the parent
+ * via the file referenced by the system property
+ * {@code groovy.junit6.forked.result}: empty file on success, serialised
+ * {@link Throwable} (with text fallback) on failure.
+ *
+ * @since 6.0.0
+ */
+public final class ForkedJvmTestRunner {
+
+    /** Set on the child JVM so {@link ForkedJvmExtension} doesn't re-fork. */
+    public static final String FORKED_FLAG = "groovy.junit6.forked";
+
+    /** Path of the result file the child writes into. */
+    public static final String RESULT_FILE_PROP = 
"groovy.junit6.forked.result";
+
+    private ForkedJvmTestRunner() {}
+
+    public static void main(String[] args) throws Exception {
+        if (args.length != 2) {
+            System.err.println("Usage: ForkedJvmTestRunner <testClass> 
<methodName>");
+            System.exit(2);
+        }
+        String className = args[0];
+        String methodName = args[1];
+        Path resultPath = Paths.get(System.getProperty(RESULT_FILE_PROP));

Review Comment:
   `RESULT_FILE_PROP` is required for correct operation, but if it is missing 
`System.getProperty(...)` returns null and `Paths.get(null)` will throw a 
`NullPointerException` with a low-signal stack trace. Consider validating that 
the system property is present and exiting with a clear message and distinct 
exit code when it is not.



##########
subprojects/groovy-test-junit6/src/main/java/groovy/junit6/plugin/ForkedJvmExtension.java:
##########
@@ -0,0 +1,145 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package groovy.junit6.plugin;
+
+import org.junit.jupiter.api.extension.ExtensionContext;
+import org.junit.jupiter.api.extension.InvocationInterceptor;
+import org.junit.jupiter.api.extension.ReflectiveInvocationContext;
+
+import java.io.ByteArrayInputStream;
+import java.io.File;
+import java.io.IOException;
+import java.io.ObjectInputStream;
+import java.lang.reflect.Method;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * JUnit 5 {@link InvocationInterceptor} backing the {@link ForkedJvm}
+ * annotation. When applied, the test method is skipped in the current JVM and
+ * re-run in a freshly forked JVM via {@link ForkedJvmTestRunner}, with any
+ * declared system properties and JVM args.
+ * <p>
+ * Recursion is avoided by setting the system property
+ * {@link ForkedJvmTestRunner#FORKED_FLAG} on the child; when the extension
+ * sees that flag set in the current JVM it just proceeds with the normal
+ * invocation (i.e. the child JVM actually runs the test body).
+ *
+ * @since 6.0.0
+ */
+public class ForkedJvmExtension implements InvocationInterceptor {
+
+    @Override
+    public void interceptTestMethod(Invocation<Void> invocation,
+                                    ReflectiveInvocationContext<Method> 
invocationContext,
+                                    ExtensionContext extensionContext) throws 
Throwable {
+        if 
(Boolean.parseBoolean(System.getProperty(ForkedJvmTestRunner.FORKED_FLAG))) {
+            // Already in the child JVM — run the body for real.
+            invocation.proceed();
+            return;
+        }
+
+        ForkedJvm config = findAnnotation(extensionContext);
+        if (config == null) {
+            invocation.proceed();
+            return;
+        }
+
+        // Skip in this (parent) JVM; we'll run the same method in a child.
+        invocation.skip();
+
+        Class<?> testClass = invocationContext.getTargetClass();
+        Method testMethod = invocationContext.getExecutable();
+        runInForkedJvm(testClass, testMethod, config);
+    }
+
+    private static ForkedJvm findAnnotation(ExtensionContext context) {
+        return context.getElement()
+                .map(el -> el.getAnnotation(ForkedJvm.class))
+                .orElseGet(() -> context.getTestClass()
+                        .map(c -> c.getAnnotation(ForkedJvm.class))
+                        .orElse(null));
+    }
+
+    private static void runInForkedJvm(Class<?> testClass, Method testMethod, 
ForkedJvm config) throws Throwable {
+        Path resultFile = Files.createTempFile("groovy-forked-jvm-result", 
".bin");
+        try {
+            List<String> command = buildCommand(testClass, testMethod, config, 
resultFile);
+            ProcessBuilder pb = new ProcessBuilder(command).inheritIO();
+            int exit = pb.start().waitFor();
+            propagateOutcome(exit, resultFile, testClass, testMethod);
+        } finally {
+            try { Files.deleteIfExists(resultFile); } catch (IOException 
ignore) {}
+        }
+    }
+
+    private static List<String> buildCommand(Class<?> testClass, Method 
testMethod,
+                                             ForkedJvm config, Path 
resultFile) {
+        List<String> cmd = new ArrayList<>();
+        String javaHome = System.getProperty("java.home");
+        cmd.add(javaHome + File.separator + "bin" + File.separator + "java");
+        cmd.add("-D" + ForkedJvmTestRunner.FORKED_FLAG + "=true");
+        cmd.add("-D" + ForkedJvmTestRunner.RESULT_FILE_PROP + "=" + 
resultFile);
+        for (String sp : config.systemProperties()) {
+            int eq = sp.indexOf('=');
+            if (eq < 0) {
+                throw new IllegalArgumentException(
+                        "@ForkedJvm system property must be 'key=value', got: 
" + sp);
+            }
+            cmd.add("-D" + sp);
+        }
+        for (String arg : config.jvmArgs()) {
+            cmd.add(arg);
+        }
+        cmd.add("-cp");
+        cmd.add(System.getProperty("java.class.path"));
+        cmd.add(ForkedJvmTestRunner.class.getName());
+        cmd.add(testClass.getName());
+        cmd.add(testMethod.getName());
+        return cmd;
+    }
+
+    private static void propagateOutcome(int exit, Path resultFile,
+                                         Class<?> testClass, Method 
testMethod) throws Throwable {
+        byte[] bytes = Files.readAllBytes(resultFile);
+        if (exit == 0 && bytes.length == 0) return;
+
+        String location = testClass.getName() + "#" + testMethod.getName();

Review Comment:
   If the child JVM crashes before creating/writing the result file, 
`Files.readAllBytes(resultFile)` will throw (e.g., `NoSuchFileException`) and 
the parent will fail with an unhelpful I/O stack trace. Consider handling 
missing/unreadable result files explicitly and throwing an `AssertionError` 
that includes the forked command location and the child exit code to aid 
debugging.
   



##########
subprojects/groovy-test-junit6/src/main/java/groovy/junit6/plugin/ForkedJvmTestRunner.java:
##########
@@ -0,0 +1,125 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package groovy.junit6.plugin;
+
+import org.junit.platform.engine.discovery.DiscoverySelectors;
+import org.junit.platform.launcher.Launcher;
+import org.junit.platform.launcher.LauncherDiscoveryRequest;
+import org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder;
+import org.junit.platform.launcher.core.LauncherFactory;
+import org.junit.platform.launcher.listeners.SummaryGeneratingListener;
+import org.junit.platform.launcher.listeners.TestExecutionSummary;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.ObjectOutputStream;
+import java.io.OutputStream;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+
+/**
+ * Child-JVM entry point for {@link ForkedJvm}-annotated tests.
+ * <p>
+ * Invoked by {@link ForkedJvmExtension} with the qualified test class name
+ * and method name as command-line arguments. Runs exactly that one test method
+ * via the JUnit Platform {@link Launcher}, then reports outcome to the parent
+ * via the file referenced by the system property
+ * {@code groovy.junit6.forked.result}: empty file on success, serialised
+ * {@link Throwable} (with text fallback) on failure.
+ *
+ * @since 6.0.0
+ */
+public final class ForkedJvmTestRunner {
+
+    /** Set on the child JVM so {@link ForkedJvmExtension} doesn't re-fork. */
+    public static final String FORKED_FLAG = "groovy.junit6.forked";
+
+    /** Path of the result file the child writes into. */
+    public static final String RESULT_FILE_PROP = 
"groovy.junit6.forked.result";
+
+    private ForkedJvmTestRunner() {}
+
+    public static void main(String[] args) throws Exception {
+        if (args.length != 2) {
+            System.err.println("Usage: ForkedJvmTestRunner <testClass> 
<methodName>");
+            System.exit(2);
+        }
+        String className = args[0];
+        String methodName = args[1];
+        Path resultPath = Paths.get(System.getProperty(RESULT_FILE_PROP));
+
+        Class<?> testClass = Class.forName(className);
+        // Resolve a single overload by parameter-less name; ForkedJvm tests
+        // shouldn't have parameterised method signatures we can't match.
+        LauncherDiscoveryRequest req = 
LauncherDiscoveryRequestBuilder.request()
+                .selectors(DiscoverySelectors.selectMethod(testClass, 
methodName))
+                .build();
+
+        Launcher launcher = LauncherFactory.create();
+        SummaryGeneratingListener listener = new SummaryGeneratingListener();
+        launcher.registerTestExecutionListeners(listener);
+        launcher.execute(req);
+
+        TestExecutionSummary summary = listener.getSummary();
+        if (summary.getTotalFailureCount() == 0 && 
summary.getTestsFoundCount() > 0) {
+            Files.write(resultPath, new byte[0]);
+            System.exit(0);
+        }
+        if (summary.getTestsFoundCount() == 0) {
+            writeTextFallback(resultPath,
+                    "ForkedJvmTestRunner: no test discovered for "
+                            + className + "#" + methodName);
+            System.exit(3);
+        }
+        Throwable cause = summary.getFailures().get(0).getException();
+        writeFailure(resultPath, cause);
+        System.exit(1);
+    }
+
+    private static void writeFailure(Path path, Throwable t) throws 
IOException {
+        try (ByteArrayOutputStream bytes = new ByteArrayOutputStream();
+             ObjectOutputStream oos = new ObjectOutputStream(bytes)) {
+            oos.writeObject(t);
+            oos.flush();
+            Files.write(path, bytes.toByteArray());
+        } catch (Exception serializationFailed) {
+            // Some test exceptions aren't Serializable; fall back to text.
+            writeTextFallback(path, stackTraceText(t));
+        }
+    }
+
+    private static void writeTextFallback(Path path, String text) throws 
IOException {
+        // First byte 0 marks "text fallback" so parent can distinguish from
+        // a serialised Throwable (which never starts with byte 0 from
+        // ObjectOutputStream's STREAM_MAGIC = 0xACED).
+        try (OutputStream out = Files.newOutputStream(path)) {
+            out.write(0);
+            out.write(text.getBytes(java.nio.charset.StandardCharsets.UTF_8));
+        }

Review Comment:
   The protocol marker byte `0` is a magic value shared with the parent-side 
reader. Consider introducing a named constant (e.g., `TEXT_FALLBACK_PREFIX`) 
used in both `ForkedJvmTestRunner` and `ForkedJvmExtension` to avoid divergence 
and make the wire format easier to evolve.



##########
subprojects/groovy-test-junit6/src/main/java/groovy/junit6/plugin/ForkedJvmExtension.java:
##########
@@ -0,0 +1,145 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package groovy.junit6.plugin;
+
+import org.junit.jupiter.api.extension.ExtensionContext;
+import org.junit.jupiter.api.extension.InvocationInterceptor;
+import org.junit.jupiter.api.extension.ReflectiveInvocationContext;
+
+import java.io.ByteArrayInputStream;
+import java.io.File;
+import java.io.IOException;
+import java.io.ObjectInputStream;
+import java.lang.reflect.Method;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * JUnit 5 {@link InvocationInterceptor} backing the {@link ForkedJvm}
+ * annotation. When applied, the test method is skipped in the current JVM and
+ * re-run in a freshly forked JVM via {@link ForkedJvmTestRunner}, with any
+ * declared system properties and JVM args.
+ * <p>
+ * Recursion is avoided by setting the system property
+ * {@link ForkedJvmTestRunner#FORKED_FLAG} on the child; when the extension
+ * sees that flag set in the current JVM it just proceeds with the normal
+ * invocation (i.e. the child JVM actually runs the test body).
+ *
+ * @since 6.0.0
+ */
+public class ForkedJvmExtension implements InvocationInterceptor {
+
+    @Override
+    public void interceptTestMethod(Invocation<Void> invocation,
+                                    ReflectiveInvocationContext<Method> 
invocationContext,
+                                    ExtensionContext extensionContext) throws 
Throwable {
+        if 
(Boolean.parseBoolean(System.getProperty(ForkedJvmTestRunner.FORKED_FLAG))) {
+            // Already in the child JVM — run the body for real.
+            invocation.proceed();
+            return;
+        }
+
+        ForkedJvm config = findAnnotation(extensionContext);
+        if (config == null) {
+            invocation.proceed();
+            return;
+        }
+
+        // Skip in this (parent) JVM; we'll run the same method in a child.
+        invocation.skip();
+
+        Class<?> testClass = invocationContext.getTargetClass();
+        Method testMethod = invocationContext.getExecutable();
+        runInForkedJvm(testClass, testMethod, config);
+    }
+
+    private static ForkedJvm findAnnotation(ExtensionContext context) {
+        return context.getElement()
+                .map(el -> el.getAnnotation(ForkedJvm.class))
+                .orElseGet(() -> context.getTestClass()
+                        .map(c -> c.getAnnotation(ForkedJvm.class))
+                        .orElse(null));
+    }
+
+    private static void runInForkedJvm(Class<?> testClass, Method testMethod, 
ForkedJvm config) throws Throwable {
+        Path resultFile = Files.createTempFile("groovy-forked-jvm-result", 
".bin");
+        try {
+            List<String> command = buildCommand(testClass, testMethod, config, 
resultFile);
+            ProcessBuilder pb = new ProcessBuilder(command).inheritIO();
+            int exit = pb.start().waitFor();
+            propagateOutcome(exit, resultFile, testClass, testMethod);
+        } finally {
+            try { Files.deleteIfExists(resultFile); } catch (IOException 
ignore) {}
+        }
+    }
+
+    private static List<String> buildCommand(Class<?> testClass, Method 
testMethod,
+                                             ForkedJvm config, Path 
resultFile) {
+        List<String> cmd = new ArrayList<>();
+        String javaHome = System.getProperty("java.home");
+        cmd.add(javaHome + File.separator + "bin" + File.separator + "java");

Review Comment:
   The fork command uses `<java.home>/bin/java` without a platform-specific 
executable suffix. On Windows, executing a fully-qualified path without `.exe` 
can fail. Consider resolving the executable more robustly (e.g., append `.exe` 
on Windows, or locate the `java` executable via 
`ProcessHandle.info().command()` when available).
   



##########
subprojects/groovy-test-junit6/src/main/java/groovy/junit6/plugin/ForkedJvmTestRunner.java:
##########
@@ -0,0 +1,125 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package groovy.junit6.plugin;
+
+import org.junit.platform.engine.discovery.DiscoverySelectors;
+import org.junit.platform.launcher.Launcher;
+import org.junit.platform.launcher.LauncherDiscoveryRequest;
+import org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder;
+import org.junit.platform.launcher.core.LauncherFactory;
+import org.junit.platform.launcher.listeners.SummaryGeneratingListener;
+import org.junit.platform.launcher.listeners.TestExecutionSummary;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.ObjectOutputStream;
+import java.io.OutputStream;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+
+/**
+ * Child-JVM entry point for {@link ForkedJvm}-annotated tests.
+ * <p>
+ * Invoked by {@link ForkedJvmExtension} with the qualified test class name
+ * and method name as command-line arguments. Runs exactly that one test method
+ * via the JUnit Platform {@link Launcher}, then reports outcome to the parent
+ * via the file referenced by the system property
+ * {@code groovy.junit6.forked.result}: empty file on success, serialised
+ * {@link Throwable} (with text fallback) on failure.
+ *
+ * @since 6.0.0
+ */
+public final class ForkedJvmTestRunner {
+
+    /** Set on the child JVM so {@link ForkedJvmExtension} doesn't re-fork. */
+    public static final String FORKED_FLAG = "groovy.junit6.forked";
+
+    /** Path of the result file the child writes into. */
+    public static final String RESULT_FILE_PROP = 
"groovy.junit6.forked.result";
+
+    private ForkedJvmTestRunner() {}
+
+    public static void main(String[] args) throws Exception {
+        if (args.length != 2) {
+            System.err.println("Usage: ForkedJvmTestRunner <testClass> 
<methodName>");
+            System.exit(2);
+        }
+        String className = args[0];
+        String methodName = args[1];
+        Path resultPath = Paths.get(System.getProperty(RESULT_FILE_PROP));
+
+        Class<?> testClass = Class.forName(className);
+        // Resolve a single overload by parameter-less name; ForkedJvm tests
+        // shouldn't have parameterised method signatures we can't match.
+        LauncherDiscoveryRequest req = 
LauncherDiscoveryRequestBuilder.request()
+                .selectors(DiscoverySelectors.selectMethod(testClass, 
methodName))
+                .build();
+
+        Launcher launcher = LauncherFactory.create();
+        SummaryGeneratingListener listener = new SummaryGeneratingListener();
+        launcher.registerTestExecutionListeners(listener);
+        launcher.execute(req);
+
+        TestExecutionSummary summary = listener.getSummary();
+        if (summary.getTotalFailureCount() == 0 && 
summary.getTestsFoundCount() > 0) {
+            Files.write(resultPath, new byte[0]);
+            System.exit(0);
+        }

Review Comment:
   There are new semantics around how the forked child reports outcomes back to 
the parent, but the added tests don't cover the aborted/assumption case. Please 
add a test that uses `@ForkedJvm` and triggers an assumption abort in the 
child, asserting that the parent JVM reports the test as aborted (not passed).



##########
subprojects/groovy-test-junit6/src/main/java/groovy/junit6/plugin/ForkedJvmTestRunner.java:
##########
@@ -0,0 +1,125 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package groovy.junit6.plugin;
+
+import org.junit.platform.engine.discovery.DiscoverySelectors;
+import org.junit.platform.launcher.Launcher;
+import org.junit.platform.launcher.LauncherDiscoveryRequest;
+import org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder;
+import org.junit.platform.launcher.core.LauncherFactory;
+import org.junit.platform.launcher.listeners.SummaryGeneratingListener;
+import org.junit.platform.launcher.listeners.TestExecutionSummary;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.ObjectOutputStream;
+import java.io.OutputStream;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+
+/**
+ * Child-JVM entry point for {@link ForkedJvm}-annotated tests.
+ * <p>
+ * Invoked by {@link ForkedJvmExtension} with the qualified test class name
+ * and method name as command-line arguments. Runs exactly that one test method
+ * via the JUnit Platform {@link Launcher}, then reports outcome to the parent
+ * via the file referenced by the system property
+ * {@code groovy.junit6.forked.result}: empty file on success, serialised
+ * {@link Throwable} (with text fallback) on failure.
+ *
+ * @since 6.0.0
+ */
+public final class ForkedJvmTestRunner {
+
+    /** Set on the child JVM so {@link ForkedJvmExtension} doesn't re-fork. */
+    public static final String FORKED_FLAG = "groovy.junit6.forked";
+
+    /** Path of the result file the child writes into. */
+    public static final String RESULT_FILE_PROP = 
"groovy.junit6.forked.result";
+
+    private ForkedJvmTestRunner() {}
+
+    public static void main(String[] args) throws Exception {
+        if (args.length != 2) {
+            System.err.println("Usage: ForkedJvmTestRunner <testClass> 
<methodName>");
+            System.exit(2);
+        }
+        String className = args[0];
+        String methodName = args[1];
+        Path resultPath = Paths.get(System.getProperty(RESULT_FILE_PROP));
+
+        Class<?> testClass = Class.forName(className);
+        // Resolve a single overload by parameter-less name; ForkedJvm tests
+        // shouldn't have parameterised method signatures we can't match.
+        LauncherDiscoveryRequest req = 
LauncherDiscoveryRequestBuilder.request()
+                .selectors(DiscoverySelectors.selectMethod(testClass, 
methodName))
+                .build();
+
+        Launcher launcher = LauncherFactory.create();
+        SummaryGeneratingListener listener = new SummaryGeneratingListener();
+        launcher.registerTestExecutionListeners(listener);
+        launcher.execute(req);
+
+        TestExecutionSummary summary = listener.getSummary();
+        if (summary.getTotalFailureCount() == 0 && 
summary.getTestsFoundCount() > 0) {
+            Files.write(resultPath, new byte[0]);
+            System.exit(0);
+        }

Review Comment:
   The child process treats a run with zero failures as success, even if the 
test was aborted (e.g., via JUnit assumptions). In that scenario 
`getTotalFailureCount()` is 0 but the test should not be reported as a pass. 
Consider explicitly checking `summary.getTestsAbortedCount()` and propagating 
an aborted outcome to the parent (e.g., by writing a serialized 
`TestAbortedException` / abort marker and exiting non-zero so the parent can 
throw `TestAbortedException`).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to