This is an automated email from the ASF dual-hosted git repository.

hansva pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/hop.git


The following commit(s) were added to refs/heads/main by this push:
     new 1ebee033a3 Fix hop-conf variable option handling (#7856)
1ebee033a3 is described below

commit 1ebee033a3767f650eab05a54fe03af9ce4628c3
Author: Gabriel Dutra <[email protected]>
AuthorDate: Sat Sep 26 02:50:27 2026 -0700

    Fix hop-conf variable option handling (#7856)
    
    * Report hop-conf variable changes
    
    * rebase and fix errors
    
    ---------
    
    Co-authored-by: Hans Van Akelyen <[email protected]>
---
 .../apache/hop/config/SetHopConfigVariables.java   |   6 +-
 .../hop/config/SetHopConfigVariablesTest.java      | 130 +++++++++++++++++++++
 2 files changed, 135 insertions(+), 1 deletion(-)

diff --git 
a/engine/src/main/java/org/apache/hop/config/SetHopConfigVariables.java 
b/engine/src/main/java/org/apache/hop/config/SetHopConfigVariables.java
index e5317d9265..21db84131f 100644
--- a/engine/src/main/java/org/apache/hop/config/SetHopConfigVariables.java
+++ b/engine/src/main/java/org/apache/hop/config/SetHopConfigVariables.java
@@ -78,11 +78,15 @@ public class SetHopConfigVariables implements 
IConfigOptions {
           describedVariable.setDescription(description);
         }
         HopConfig.getInstance().setDescribedVariable(describedVariable);
+        changed = true;
       } else {
         throw new HopException(
             "Please set a variable description in the format 
'VARIABLE_NAME=DESCRIPTION'");
       }
     }
-    return false;
+    if (changed) {
+      HopConfig.getInstance().saveToFile();
+    }
+    return changed;
   }
 }
diff --git 
a/engine/src/test/java/org/apache/hop/config/SetHopConfigVariablesTest.java 
b/engine/src/test/java/org/apache/hop/config/SetHopConfigVariablesTest.java
new file mode 100644
index 0000000000..d6ca44e273
--- /dev/null
+++ b/engine/src/test/java/org/apache/hop/config/SetHopConfigVariablesTest.java
@@ -0,0 +1,130 @@
+/*
+ * 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 org.apache.hop.config;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.List;
+import java.util.Map;
+import org.apache.hop.core.config.ConfigFileSerializer;
+import org.apache.hop.core.config.HopConfig;
+import org.apache.hop.core.config.IHopConfigSerializer;
+import org.apache.hop.core.config.plugin.ConfigFile;
+import org.apache.hop.core.exception.HopException;
+import org.apache.hop.junit.rules.RestoreHopEnvironmentExtension;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.api.io.TempDir;
+import picocli.CommandLine;
+
+@ExtendWith(RestoreHopEnvironmentExtension.class)
+class SetHopConfigVariablesTest {
+
+  private static final String VARIABLE_NAME = "HOP_SET_CONFIG_VARIABLES_TEST";
+
+  @TempDir Path tempDir;
+
+  private Path configFile;
+  private String originalConfigFilename;
+  private IHopConfigSerializer originalSerializer;
+
+  @BeforeEach
+  void redirectConfigFile() throws Exception {
+    // Never write to the developer's real hop-config.json
+    //
+    configFile = tempDir.resolve("hop-config.json");
+    Files.writeString(configFile, "{}");
+
+    HopConfig hopConfig = HopConfig.getInstance();
+    originalConfigFilename = hopConfig.getConfigFilename();
+    originalSerializer = hopConfig.getSerializer();
+    hopConfig.setConfigFilename(configFile.toString());
+    hopConfig.setSerializer(new ConfigFileSerializer());
+  }
+
+  @AfterEach
+  void restoreConfigFile() {
+    HopConfig hopConfig = HopConfig.getInstance();
+    hopConfig.getDescribedVariables().removeIf(v -> 
VARIABLE_NAME.equals(v.getName()));
+    hopConfig.setConfigFilename(originalConfigFilename);
+    hopConfig.setSerializer(originalSerializer);
+  }
+
+  @Test
+  void setVariableIsReportedAndSaved() throws Exception {
+    assertTrue(handle("-sv", VARIABLE_NAME + "=value"));
+
+    assertEquals("value", 
HopConfig.getInstance().findDescribedVariableValue(VARIABLE_NAME));
+    Map<String, Object> saved = savedVariable();
+    assertEquals("value", saved.get("value"));
+  }
+
+  @Test
+  void describeVariableIsReportedAndSaved() throws Exception {
+    assertTrue(handle("-dv", VARIABLE_NAME + "=description"));
+
+    Map<String, Object> saved = savedVariable();
+    assertEquals("description", saved.get("description"));
+  }
+
+  @Test
+  void describeKeepsExistingValue() throws Exception {
+    handle("-sv", VARIABLE_NAME + "=value");
+    handle("-dv", VARIABLE_NAME + "=description");
+
+    Map<String, Object> saved = savedVariable();
+    assertEquals("value", saved.get("value"));
+    assertEquals("description", saved.get("description"));
+  }
+
+  @Test
+  void noOptionIsNotAnAction() throws Exception {
+    assertFalse(handle());
+    assertEquals("{}", Files.readString(configFile));
+  }
+
+  @Test
+  void invalidFormatIsRejected() {
+    assertThrows(HopException.class, () -> handle("-sv", VARIABLE_NAME));
+  }
+
+  private boolean handle(String... args) throws HopException {
+    SetHopConfigVariables configVariables = new SetHopConfigVariables();
+    new CommandLine(configVariables).parseArgs(args);
+    return configVariables.handleOption(null, null, null);
+  }
+
+  @SuppressWarnings("unchecked")
+  private Map<String, Object> savedVariable() throws HopException {
+    Map<String, Object> config = new 
ConfigFileSerializer().readFromFile(configFile.toString());
+    List<Map<String, Object>> variables =
+        (List<Map<String, Object>>) config.get(ConfigFile.HOP_VARIABLES_KEY);
+    assertNotNull(variables, "No variables were written to " + configFile);
+    return variables.stream()
+        .filter(v -> VARIABLE_NAME.equals(v.get("name")))
+        .findFirst()
+        .orElseThrow(() -> new AssertionError(VARIABLE_NAME + " was not 
written"));
+  }
+}

Reply via email to