From d3c15a75b057293f60297345065839de42554a6b Mon Sep 17 00:00:00 2001
From: Tamas Cservenak <tamas@cservenak.net>
Date: Wed, 18 Jun 2025 21:31:58 +0200
Subject: [PATCH] Fix test framework side effects (java props) (#1359)

The JvmTestClient pushed things to system properties and then did not clean up them. This went invisible, as long as user did not have user-wide extensions like smart-builder that suddenly was not filtered out anymore from tested mvnd daemon and caused havoc.

Changes:
* def client helper method emits "prev state" for properties it altered
* JvmTestClient uses this map to restore properties
* MavenConfIgnore ITs modified to still ignore takari smart builder as well...

Fixes #1357
---
 .../mvndaemon/mvnd/client/DefaultClient.java  | 12 ++++++--
 .../mvnd/it/MavenConfIgnoreExtNativeIT.java   |  2 +-
 .../mvndaemon/mvnd/junit/JvmTestClient.java   | 29 +++++++++++++------
 3 files changed, 30 insertions(+), 13 deletions(-)

diff --git a/client/src/main/java-mvnd/org/mvndaemon/mvnd/client/DefaultClient.java b/client/src/main/java-mvnd/org/mvndaemon/mvnd/client/DefaultClient.java
index fe3fbeb0..084c65e7 100644
--- a/client/src/main/java-mvnd/org/mvndaemon/mvnd/client/DefaultClient.java
+++ b/client/src/main/java-mvnd/org/mvndaemon/mvnd/client/DefaultClient.java
@@ -34,8 +34,10 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.EnumSet;
+import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Collectors;
@@ -178,7 +180,8 @@ public class DefaultClient implements Client {
         System.exit(exitCode);
     }
 
-    public static void setSystemPropertiesFromCommandLine(List<String> args) {
+    public static Map<String, String> setSystemPropertiesFromCommandLine(List<String> args) {
+        final HashMap<String, String> prevState = new HashMap<>();
         final Iterator<String> iterator = args.iterator();
         boolean defineIsEmpty = false;
         while (iterator.hasNext()) {
@@ -205,14 +208,17 @@ public class DefaultClient implements Client {
                 if (eqPos >= 0) {
                     String k = val.substring(0, eqPos);
                     String v = val.substring(eqPos + 1);
-                    System.setProperty(k, v);
+                    String old = System.setProperty(k, v);
+                    prevState.put(k, old);
                     LOGGER.trace("Setting system property {} to {}", k, v);
                 } else {
-                    System.setProperty(val, "");
+                    String old = System.setProperty(val, "");
+                    prevState.put(val, old);
                     LOGGER.trace("Setting system property {}", val);
                 }
             }
         }
+        return prevState;
     }
 
     private static boolean maybeDefineCommandLineOption(String arg) {
diff --git a/integration-tests/src/test/java/org/mvndaemon/mvnd/it/MavenConfIgnoreExtNativeIT.java b/integration-tests/src/test/java/org/mvndaemon/mvnd/it/MavenConfIgnoreExtNativeIT.java
index 2e808579..c9b0a0b1 100644
--- a/integration-tests/src/test/java/org/mvndaemon/mvnd/it/MavenConfIgnoreExtNativeIT.java
+++ b/integration-tests/src/test/java/org/mvndaemon/mvnd/it/MavenConfIgnoreExtNativeIT.java
@@ -28,7 +28,7 @@ class MavenConfIgnoreExtNativeIT extends MavenConfNativeIT {
     @Override
     protected List<String> mvndParams(String expression) {
         ArrayList<String> result = new ArrayList<>(super.mvndParams(expression));
-        result.add("-Dmvnd.coreExtensionsExclude=foo:bar");
+        result.add("-Dmvnd.coreExtensionsExclude=foo:bar,io.takari.maven:takari-smart-builder");
         return result;
     }
 }
diff --git a/integration-tests/src/test/java/org/mvndaemon/mvnd/junit/JvmTestClient.java b/integration-tests/src/test/java/org/mvndaemon/mvnd/junit/JvmTestClient.java
index 56f523fd..b18af783 100644
--- a/integration-tests/src/test/java/org/mvndaemon/mvnd/junit/JvmTestClient.java
+++ b/integration-tests/src/test/java/org/mvndaemon/mvnd/junit/JvmTestClient.java
@@ -22,6 +22,7 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Map;
 
 import org.mvndaemon.mvnd.assertj.TestClientOutput;
 import org.mvndaemon.mvnd.client.DaemonParameters;
@@ -44,16 +45,26 @@ public class JvmTestClient extends DefaultClient {
     @Override
     public ExecutionResult execute(ClientOutput output, List<String> argv) {
         setMultiModuleProjectDirectory(argv);
-        setSystemPropertiesFromCommandLine(argv);
-        argv = new ArrayList<>(argv);
-        if (parameters instanceof TestParameters && ((TestParameters) parameters).isNoTransferProgress()) {
-            argv.add("-ntp");
+        Map<String, String> prevState = setSystemPropertiesFromCommandLine(argv);
+        try {
+            argv = new ArrayList<>(argv);
+            if (parameters instanceof TestParameters && ((TestParameters) parameters).isNoTransferProgress()) {
+                argv.add("-ntp");
+            }
+            final ExecutionResult delegate = super.execute(output, augmentArgs(argv));
+            if (output instanceof TestClientOutput) {
+                return new JvmTestResult(delegate, ((TestClientOutput) output).messagesToString());
+            }
+            return delegate;
+        } finally {
+            prevState.entrySet().forEach(entry -> {
+                if (entry.getValue() == null) {
+                    System.clearProperty(entry.getKey());
+                } else {
+                    System.setProperty(entry.getKey(), entry.getValue());
+                }
+            });
         }
-        final ExecutionResult delegate = super.execute(output, augmentArgs(argv));
-        if (output instanceof TestClientOutput) {
-            return new JvmTestResult(delegate, ((TestClientOutput) output).messagesToString());
-        }
-        return delegate;
     }
 
     private void setMultiModuleProjectDirectory(List<String> args) {